[libvirt] [PATCH] event: fix two event-handling bugs

[Eric Blake]

Regression introduced in commit e6b68d7.

Prior to that point, handlesAlloc was always a multiple of
EVENT_ALLOC_EXTENT, and was an integer (so even if the
subtraction wrapped, a negative value was less than the
count and did not try to free the handles array).  But after
that point, VIR_RESIZE_N made handlesAlloc grow geometrically,
so handlesAlloc could be 49 when handlesCount was 47, while
still freeing off only 10 at a time, and eventually reach
handlesAlloc 7 and handlesCount 1.  Since (size_t)(7 - 10) is
indeed greater than 1, this then tried to free 10 elements,
which had the awful effect of nuking the handles array while
there were still live handles.

Which leads to crashes like this:
https://bugzilla.redhat.com/show_bug.cgi?id=670848

* daemon/event.c (virEventDispatchHandles): Cache watch while the
lock is still held, as eventLoop.handles might be relocated
outside the lock.
(virEventCleanupHandles): Avoid integer wrap-around causing us to
delete the entire handles array while handles are still active.
---

Thanks to Dave Allan for letting me use his faqemu setup for testing
this.  Basically, starting 60 faqemu followed by stopping them all
was a reliable way to trigger the handle cleanup integer wraparound.

 daemon/event.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/daemon/event.c b/daemon/event.c
index 89ca9f0..9cad466 100644
--- a/daemon/event.c
+++ b/daemon/event.c
@@ -1,7 +1,7 @@
 /*
  * event.c: event loop for monitoring file handles
  *
- * Copyright (C) 2007, 2010 Red Hat, Inc.
+ * Copyright (C) 2007, 2010-2011 Red Hat, Inc.
  * Copyright (C) 2007 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -435,6 +435,7 @@ static int virEventDispatchTimeouts(void) {
  */
 static int virEventDispatchHandles(int nfds, struct pollfd *fds) {
     int i, n;
+    int watch;
     DEBUG("Dispatch %d", nfds);

     /* NB, use nfds not eventLoop.handlesCount, because new
@@ -463,9 +464,9 @@ static int virEventDispatchHandles(int nfds, struct pollfd *fds) {
             EVENT_DEBUG("Dispatch n=%d f=%d w=%d e=%d %p", i,
                         fds[n].fd, eventLoop.handles[i].watch,
                         fds[n].revents, eventLoop.handles[i].opaque);
+            watch = eventLoop.handles[i].watch;
             virMutexUnlock(&eventLoop.lock);
-            (cb)(eventLoop.handles[i].watch,
-                 fds[n].fd, hEvents, opaque);
+            (cb)(watch, fds[n].fd, hEvents, opaque);
             virMutexLock(&eventLoop.lock);
         }
     }
@@ -542,9 +543,10 @@ static int virEventCleanupHandles(void) {
     }

     /* Release some memory if we've got a big chunk free */
-    if ((eventLoop.handlesAlloc - EVENT_ALLOC_EXTENT) > eventLoop.handlesCount) {
+    if (eventLoop.handlesAlloc - eventLoop.handlesCount > EVENT_ALLOC_EXTENT) {
         EVENT_DEBUG("Releasing %zu out of %zu handles slots used, releasing %d",
-                   eventLoop.handlesCount, eventLoop.handlesAlloc, EVENT_ALLOC_EXTENT);
+                    eventLoop.handlesCount, eventLoop.handlesAlloc,
+                    EVENT_ALLOC_EXTENT);
         VIR_SHRINK_N(eventLoop.handles, eventLoop.handlesAlloc,
                      EVENT_ALLOC_EXTENT);
     }
-- 
1.7.3.4