[libvirt] [PATCH] virEventPollDispatchHandles: Honour array boundaries

[Michal Privoznik]

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 at 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