When dispatching events from the event loop, the array of registered handles is searched to see what handles happened an event on. However, the array is searched in weird way: the check for the array boundaries is at the end, so we may touch the elements after the end of the array: ==10434== Invalid read of size 4 ==10434== at 0x52D06B6: virEventPollDispatchHandles (vireventpoll.c:486) ==10434== by 0x52D10E4: virEventPollRunOnce (vireventpoll.c:660) ==10434== by 0x52CF207: virEventRunDefaultImpl (virevent.c:308) ==10434== by 0x1639D1: virNetServerRun (virnetserver.c:1139) ==10434== by 0x1220DC: main (libvirtd.c:1507) ==10434== Address 0xc11ff04 is 4 bytes after a block of size 960 alloc'd ==10434== at 0x4C2CA5E: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==10434== by 0x52AD378: virReallocN (viralloc.c:245) ==10434== by 0x52AD46E: virExpandN (viralloc.c:294) ==10434== by 0x52AD5B1: virResizeN (viralloc.c:352) ==10434== by 0x52CF2EC: virEventPollAddHandle (vireventpoll.c:116) ==10434== by 0x52CEF5B: virEventAddHandle (virevent.c:78) ==10434== by 0x11F69A90: nodeStateInitialize (node_device_udev.c:1797) ==10434== by 0x53C3C89: virStateInitialize (libvirt.c:743) ==10434== by 0x120563: daemonRunStateInit (libvirtd.c:919) ==10434== by 0x5317719: virThreadHelper (virthread.c:197) ==10434== by 0x8376F39: start_thread (in /lib64/libpthread-2.17.so) ==10434== by 0x8A7F9FC: clone (in /lib64/libc-2.17.so) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vireventpoll.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c index 528b24c..13f40dc 100644 --- a/src/util/vireventpoll.c +++ b/src/util/vireventpoll.c @@ -477,46 +477,46 @@ static int virEventPollDispatchTimeouts(void) static int virEventPollDispatchHandles(int nfds, struct pollfd *fds) { size_t i, n; VIR_DEBUG("Dispatch %d", nfds); /* NB, use nfds not eventLoop.handlesCount, because new * fds might be added on end of list, and they're not * in the fds array we've got */ for (i = 0, n = 0; n < nfds && i < eventLoop.handlesCount; n++) { - while ((eventLoop.handles[i].fd != fds[n].fd || - eventLoop.handles[i].events == 0) && - i < eventLoop.handlesCount) { + while (i < eventLoop.handlesCount && + (eventLoop.handles[i].fd != fds[n].fd || + eventLoop.handles[i].events == 0)) { i++; } if (i == eventLoop.handlesCount) break; VIR_DEBUG("i=%zu w=%d", i, eventLoop.handles[i].watch); if (eventLoop.handles[i].deleted) { EVENT_DEBUG("Skip deleted n=%zu w=%d f=%d", i, eventLoop.handles[i].watch, eventLoop.handles[i].fd); continue; } if (fds[n].revents) { virEventHandleCallback cb = eventLoop.handles[i].cb; int watch = eventLoop.handles[i].watch; void *opaque = eventLoop.handles[i].opaque; int hEvents = virEventPollFromNativeEvents(fds[n].revents); PROBE(EVENT_POLL_DISPATCH_HANDLE, "watch=%d events=%d", watch, hEvents); virMutexUnlock(&eventLoop.lock); (cb)(watch, fds[n].fd, hEvents, opaque); virMutexLock(&eventLoop.lock); } } return 0; } /* Used post dispatch to actually remove any timers that * were previously marked as deleted. This asynchronous * cleanup is needed to make dispatch re-entrant safe. */ -- 1.8.5.5