[PATCH] Reference event sources signalled by epoll before timeout handling.

James Hunt james.hunt at ubuntu.com
Tue Apr 17 09:37:04 PDT 2012


If ply_event_loop_handle_timeouts() is called before the events returned
by epoll_wait() are referenced, a timeout handler can free an event
source leading to undefined behaviour (and frequently SIGSEGV crashes)
once ply_event_loop_handle_timeouts() has finished since
ply_event_loop_process_pending_events() continues to try and process the
now invalid event sources. Thanks to cjwatson for a simpler solution to
my original fix.

Save errno values to ensure logic is robust in case handler makes
system call.

Signed-off-by: James Hunt <james.hunt at ubuntu.com>
---
 src/libply/ply-event-loop.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/src/libply/ply-event-loop.c b/src/libply/ply-event-loop.c
index acfdc5d..a1abdb6 100644
--- a/src/libply/ply-event-loop.c
+++ b/src/libply/ply-event-loop.c
@@ -1255,6 +1255,7 @@ void
 ply_event_loop_process_pending_events (ply_event_loop_t *loop)
 {
   int number_of_received_events, i;
+  int saved_errno;
   static struct epoll_event events[PLY_EVENT_LOOP_NUM_EVENT_HANDLERS];
 
   assert (loop != NULL);
@@ -1277,30 +1278,32 @@ ply_event_loop_process_pending_events (ply_event_loop_t *loop)
      number_of_received_events = epoll_wait (loop->epoll_fd, events,
                                              PLY_EVENT_LOOP_NUM_EVENT_HANDLERS,
                                              timeout);
-
-     ply_event_loop_handle_timeouts (loop);
+     saved_errno = errno;
 
      if (number_of_received_events < 0)
        {
-         if (errno != EINTR && errno != EAGAIN)
+         if (saved_errno != EINTR && saved_errno != EAGAIN)
            {
              ply_event_loop_exit (loop, 255);
              return;
            }
        }
-    }
-  while ((number_of_received_events < 0) && ((errno == EINTR) || (errno == EAGAIN)));
 
-  /* first reference all sources, so they stay alive for the duration of this
-   * iteration of the loop
-   */
-  for (i = 0; i < number_of_received_events; i++)
-    {
-      ply_event_source_t *source;
-      source = (ply_event_source_t *) (events[i].data.ptr);
+     /* Reference all sources, so they stay alive for the duration of this
+      * iteration of the loop
+      */
+     for (i = 0; i < number_of_received_events; i++)
+       {
+         ply_event_source_t *source;
+         source = (ply_event_source_t *) (events[i].data.ptr);
+
+         ply_event_source_take_reference (source);
+       }
+
+     ply_event_loop_handle_timeouts (loop);
 
-      ply_event_source_take_reference (source);
     }
+  while ((number_of_received_events < 0) && ((saved_errno == EINTR) || (saved_errno == EAGAIN)));
 
   /* Then process the incoming events
    */
-- 
1.7.9.5


--------------060300060303030109010908--


More information about the plymouth mailing list