eventloop: delay freeing pollables until the current iteration finishes

mowgli_pollable_destroy would free the pollable, however if called from
within a handler function invoked by the event loop, the event loop
might still have a pointer to it waiting to be processed, causing a
use-after-free.

Instead, keep track of whether we are currently processing events (i.e.
calling a pollops function that will process events); if we are doing
so, mark the pollable as removed and add it to a list of to-be-freed
pollables.

Individual implementations check the removed flag and ignore pollables
with it set; the general-case abstraction is responsible for keeping
track of whether we are currently in an event processing function and
cleaning up the list of pollables after each iteration.

For users of libmowgli, there is no change to API nor ABI as long as
programs use mowgli_{eventloop,pollable}_create to allocate the involved
structures.

diff --git a/src/libmowgli/eventloop/epoll_pollops.c b/src/libmowgli/eventloop/epoll_pollops.c
index d8e5320904f5d444df9cdd936c4788c674471312..01e3b1e0b785888770d4ec95ec2cbc0d79371f58 100644 (file)
--- a/src/libmowgli/eventloop/epoll_pollops.c
+++ b/src/libmowgli/eventloop/epoll_pollops.c
@@ -187,10 +187,10 @@ mowgli_epoll_eventloop_select(mowgli_eventloop_t *eventloop, int delay)
        {
                mowgli_eventloop_pollable_t *pollable = priv->pfd[i].data.ptr;
 
-               if (priv->pfd[i].events & (EPOLLIN | EPOLLHUP | EPOLLERR))
+               if (priv->pfd[i].events & (EPOLLIN | EPOLLHUP | EPOLLERR) && !pollable->removed)
                        mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_READ);
 
-               if (priv->pfd[i].events & (EPOLLOUT | EPOLLHUP | EPOLLERR))
+               if (priv->pfd[i].events & (EPOLLOUT | EPOLLHUP | EPOLLERR) && !pollable->removed)
                        mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_WRITE);
        }
 }

diff --git a/src/libmowgli/eventloop/eventloop.c b/src/libmowgli/eventloop/eventloop.c
index 22e9693274302fa3a42a0b0a68ffb5b09677c357..7bcefe19903628d923b405214b81a0b4149e8f68 100644 (file)
--- a/src/libmowgli/eventloop/eventloop.c
+++ b/src/libmowgli/eventloop/eventloop.c
@@ -81,6 +81,18 @@ mowgli_eventloop_destroy(mowgli_eventloop_t *eventloop)
        mowgli_heap_free(eventloop_heap, eventloop);
 }
 
+static void
+mowgli_eventloop_reap_pollables(mowgli_eventloop_t *eventloop)
+{
+       mowgli_node_t *n, *tn;
+       MOWGLI_ITER_FOREACH_SAFE(n, tn, eventloop->destroyed_pollable_list.head)
+       {
+               mowgli_pollable_free(n->data);
+               mowgli_node_delete(n, &eventloop->destroyed_pollable_list);
+               mowgli_node_free(n);
+       }
+}
+
 void
 mowgli_eventloop_run(mowgli_eventloop_t *eventloop)
 {
@@ -90,11 +102,16 @@ mowgli_eventloop_run(mowgli_eventloop_t *eventloop)
 
        eventloop->death_requested = false;
 
+       eventloop->processing_events = true;
+
        while (!eventloop->death_requested)
        {
                eventloop->eventloop_ops->run_once(eventloop);
+               mowgli_eventloop_reap_pollables(eventloop);
        }
 
+       eventloop->processing_events = false;
+
        mowgli_mutex_unlock(&eventloop->mutex);
 }
 
@@ -105,7 +122,11 @@ mowgli_eventloop_run_once(mowgli_eventloop_t *eventloop)
 
        mowgli_mutex_lock(&eventloop->mutex);
 
+       eventloop->processing_events = true;
        eventloop->eventloop_ops->run_once(eventloop);
+       eventloop->processing_events = false;
+
+       mowgli_eventloop_reap_pollables(eventloop);
 
        mowgli_mutex_unlock(&eventloop->mutex);
 }
@@ -117,11 +138,17 @@ mowgli_eventloop_timeout_once(mowgli_eventloop_t *eventloop, int timeout)
 
        mowgli_mutex_lock(&eventloop->mutex);
 
+       eventloop->processing_events = true;
+
        if (timeout >= 0)
                eventloop->eventloop_ops->timeout_once(eventloop, timeout);
        else
                eventloop->eventloop_ops->run_once(eventloop);
 
+       eventloop->processing_events = false;
+
+       mowgli_eventloop_reap_pollables(eventloop);
+
        mowgli_mutex_unlock(&eventloop->mutex);
 }
 

diff --git a/src/libmowgli/eventloop/eventloop.h b/src/libmowgli/eventloop/eventloop.h
index f57dfff21eb58925101942b637c66a80a709de02..5e5bf2361d285a2ae5195852916962e376d5624a 100644 (file)
--- a/src/libmowgli/eventloop/eventloop.h
+++ b/src/libmowgli/eventloop/eventloop.h
@@ -122,6 +122,8 @@ struct _mowgli_pollable
        mowgli_node_t node;
 
        mowgli_eventloop_t *eventloop;
+
+       bool removed;
 };
 
 typedef struct
@@ -153,6 +155,9 @@ struct _mowgli_eventloop
        void *data;
 
        time_t epochbias;
+
+       mowgli_list_t destroyed_pollable_list;
+       bool processing_events;
 };
 
 typedef void mowgli_event_dispatch_func_t (void *userdata);
@@ -358,6 +363,7 @@ extern mowgli_eventloop_timer_t *mowgli_timer_find(mowgli_eventloop_t *eventloop
 
 /* pollable.c */
 extern mowgli_eventloop_pollable_t *mowgli_pollable_create(mowgli_eventloop_t *eventloop, mowgli_descriptor_t fd, void *userdata);
+extern void mowgli_pollable_free(mowgli_eventloop_pollable_t *pollable);
 extern void mowgli_pollable_destroy(mowgli_eventloop_t *eventloop, mowgli_eventloop_pollable_t *pollable);
 extern void mowgli_pollable_setselect(mowgli_eventloop_t *eventloop, mowgli_eventloop_pollable_t *pollable, mowgli_eventloop_io_dir_t dir, mowgli_eventloop_io_cb_t *event_function);
 extern void mowgli_pollable_set_nonblocking(mowgli_eventloop_pollable_t *pollable, bool nonblocking);

diff --git a/src/libmowgli/eventloop/kqueue_pollops.c b/src/libmowgli/eventloop/kqueue_pollops.c
index 4f2437d4f107ae13b6256567058d8c98c63a1d6d..ede3860184667a0aa1daa4e593a7419ece5888c1 100644 (file)
--- a/src/libmowgli/eventloop/kqueue_pollops.c
+++ b/src/libmowgli/eventloop/kqueue_pollops.c
@@ -171,10 +171,10 @@ mowgli_kqueue_eventloop_select(mowgli_eventloop_t *eventloop, int delay)
        {
                mowgli_eventloop_pollable_t *pollable = priv->events[i].udata;
 
-               if (priv->events[i].filter == EVFILT_READ)
+               if (priv->events[i].filter == EVFILT_READ && !pollable->removed)
                        mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_READ);
 
-               if (priv->events[i].filter == EVFILT_WRITE)
+               if (priv->events[i].filter == EVFILT_WRITE && !pollable->removed)
                        mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_WRITE);
 
                /* XXX Perhaps we need to recheck read_function and

diff --git a/src/libmowgli/eventloop/poll_pollops.c b/src/libmowgli/eventloop/poll_pollops.c
index ac465f6652dadcc22de10e2574f7de4218205069..8392fae2c9580f7369aca4ca669dffd803780bd4 100644 (file)
--- a/src/libmowgli/eventloop/poll_pollops.c
+++ b/src/libmowgli/eventloop/poll_pollops.c
@@ -186,7 +186,6 @@ mowgli_poll_eventloop_select(mowgli_eventloop_t *eventloop, int time)
        {
                mowgli_eventloop_synchronize(eventloop);
 
-               /* iterate twice so we don't touch freed memory if a pollable is destroyed */
                MOWGLI_ITER_FOREACH_SAFE(n, tn, priv->pollable_list.head)
                {
                        pollable = n->data;
@@ -195,7 +194,7 @@ mowgli_poll_eventloop_select(mowgli_eventloop_t *eventloop, int time)
                        if ((slot == -1) || (priv->pollfds[slot].revents == 0))
                                continue;
 
-                       if (priv->pollfds[slot].revents & (POLLRDNORM | POLLIN | POLLHUP | POLLERR) && pollable->read_function)
+                       if (priv->pollfds[slot].revents & (POLLRDNORM | POLLIN | POLLHUP | POLLERR) && pollable->read_function && !pollable->removed)
                        {
 # ifdef DEBUG
                                mowgli_log("run %p(%p, %p, MOWGLI_EVENTLOOP_IO_READ, %p)\n", pollable->read_function, eventloop, pollable, pollable->userdata);
@@ -204,17 +203,8 @@ mowgli_poll_eventloop_select(mowgli_eventloop_t *eventloop, int time)
                                priv->pollfds[slot].events &= ~(POLLRDNORM | POLLIN);
                                mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_READ);
                        }
-               }
-
-               MOWGLI_ITER_FOREACH_SAFE(n, tn, priv->pollable_list.head)
-               {
-                       pollable = n->data;
-                       slot = pollable->slot;
-
-                       if ((slot == -1) || (priv->pollfds[slot].revents == 0))
-                               continue;
 
-                       if (priv->pollfds[slot].revents & (POLLWRNORM | POLLOUT | POLLHUP | POLLERR) && pollable->write_function)
+                       if (priv->pollfds[slot].revents & (POLLWRNORM | POLLOUT | POLLHUP | POLLERR) && pollable->write_function && !pollable->removed)
                        {
 # ifdef DEBUG
                                mowgli_log("run %p(%p, %p, MOWGLI_EVENTLOOP_IO_WRITE, %p)\n", pollable->write_function, eventloop, pollable, pollable->userdata);

diff --git a/src/libmowgli/eventloop/pollable.c b/src/libmowgli/eventloop/pollable.c
index 5f9526ecaa13cbb10be7a30fdb80dd62dbf21886..5b4910375ed4846449a540d78c9d6ac3ed4d2bc9 100644 (file)
--- a/src/libmowgli/eventloop/pollable.c
+++ b/src/libmowgli/eventloop/pollable.c
@@ -42,16 +42,36 @@ mowgli_pollable_create(mowgli_eventloop_t *eventloop, mowgli_descriptor_t fd, vo
        return pollable;
 }
 
+void
+mowgli_pollable_free(mowgli_eventloop_pollable_t *pollable)
+{
+       mowgli_heap_free(pollable_heap, pollable);
+}
+
 void
 mowgli_pollable_destroy(mowgli_eventloop_t *eventloop, mowgli_eventloop_pollable_t *pollable)
 {
        return_if_fail(eventloop != NULL);
        return_if_fail(pollable != NULL);
+       return_if_fail(!pollable->removed);
 
        /* unregister any interest in the pollable. */
        eventloop->eventloop_ops->destroy(eventloop, pollable);
 
-       mowgli_heap_free(pollable_heap, pollable);
+       /* we cannot safely free a pollable from within the event loop
+        * as the event processing code might still hold pointers to it;
+        * only mark it as removed in that case and have the event loop
+        * handle cleanup as needed
+        */
+       if (eventloop->processing_events)
+       {
+               pollable->removed = true;
+               mowgli_node_add(pollable, mowgli_node_create(), &eventloop->destroyed_pollable_list);
+       }
+       else
+       {
+               mowgli_pollable_free(pollable);
+       }
 }
 
 void

diff --git a/src/libmowgli/eventloop/ports_pollops.c b/src/libmowgli/eventloop/ports_pollops.c
index bfb840ed35f3205fc0e7dd44fa28b51385eda13c..ef2f9f8e9602b4ab1cc14d38b0a2aef611577567 100644 (file)
--- a/src/libmowgli/eventloop/ports_pollops.c
+++ b/src/libmowgli/eventloop/ports_pollops.c
@@ -177,10 +177,10 @@ mowgli_ports_eventloop_select(mowgli_eventloop_t *eventloop, int delay)
                if (priv->pfd[i].portev_source != PORT_SOURCE_FD)
                        continue;
 
-               if (priv->pfd[i].portev_events & (POLLIN | POLLHUP | POLLERR))
+               if (priv->pfd[i].portev_events & (POLLIN | POLLHUP | POLLERR) && !pollable->removed)
                        mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_READ);
 
-               if (priv->pfd[i].portev_events & (POLLOUT | POLLHUP | POLLERR))
+               if (priv->pfd[i].portev_events & (POLLOUT | POLLHUP | POLLERR) && !pollable->removed)
                        mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_WRITE);
        }
 }

diff --git a/src/libmowgli/eventloop/qnx_pollops.c b/src/libmowgli/eventloop/qnx_pollops.c
index 76c83722fb42c3cf05c6c0e553709da6d1ef7a67..4db4d6339157b6b028ed0c3d422e320233cf7c2c 100644 (file)
--- a/src/libmowgli/eventloop/qnx_pollops.c
+++ b/src/libmowgli/eventloop/qnx_pollops.c
@@ -96,10 +96,10 @@ mowgli_qnx_eventloop_event_cb(select_context_t *ctp, mowgli_descriptor_t fd, uns
 
        return_if_fail(eventloop != NULL);
 
-       if (flags & (SELECT_FLAG_READ | SELECT_FLAG_EXCEPT))
+       if (flags & (SELECT_FLAG_READ | SELECT_FLAG_EXCEPT) && !pollable->removed)
                mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_READ);
 
-       if (flags & (SELECT_FLAG_WRITE | SELECT_FLAG_EXCEPT))
+       if (flags & (SELECT_FLAG_WRITE | SELECT_FLAG_EXCEPT) && !pollable->removed)
                mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_WRITE);
 }
 

diff --git a/src/libmowgli/eventloop/select_pollops.c b/src/libmowgli/eventloop/select_pollops.c
index 0be05222ef3affd0a62a5ea579e148c2758ea6f3..76bb337d66f1bc973f17ae15d8f9208687d0835b 100644 (file)
--- a/src/libmowgli/eventloop/select_pollops.c
+++ b/src/libmowgli/eventloop/select_pollops.c
@@ -166,12 +166,11 @@ mowgli_select_eventloop_select(mowgli_eventloop_t *eventloop, int delay)
        {
                mowgli_eventloop_synchronize(eventloop);
 
-               /* iterate twice so we don't touch freed memory if a pollable is destroyed */
                MOWGLI_ITER_FOREACH_SAFE(n, tn, priv->pollable_list.head)
                {
                        pollable = n->data;
 
-                       if ((FD_ISSET(pollable->fd, &rfds) || FD_ISSET(pollable->fd, &efds)))
+                       if (!pollable->removed && (FD_ISSET(pollable->fd, &rfds) || FD_ISSET(pollable->fd, &efds)))
                        {
 # ifdef DEBUG
                                mowgli_log("run %p(%p, %p, MOWGLI_EVENTLOOP_IO_READ, %p)\n", pollable->read_function, eventloop, pollable, pollable->userdata);
@@ -179,13 +178,8 @@ mowgli_select_eventloop_select(mowgli_eventloop_t *eventloop, int delay)
 
                                mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_READ);
                        }
-               }
-
-               MOWGLI_ITER_FOREACH_SAFE(n, tn, priv->pollable_list.head)
-               {
-                       pollable = n->data;
 
-                       if ((FD_ISSET(pollable->fd, &wfds) || FD_ISSET(pollable->fd, &efds)))
+                       if (!pollable->removed && (FD_ISSET(pollable->fd, &wfds) || FD_ISSET(pollable->fd, &efds)))
                        {
 # ifdef DEBUG
                                mowgli_log("run %p(%p, %p, MOWGLI_EVENTLOOP_IO_WRITE, %p)\n", pollable->write_function, eventloop, pollable, pollable->userdata);

diff --git a/src/libmowgli/eventloop/windows_pollops.c b/src/libmowgli/eventloop/windows_pollops.c
index e5c441c37e70ecce9f2c940fa36edbe3416a2c6e..35c915241686ceec7bb2e169b5adb68419e15718 100644 (file)
--- a/src/libmowgli/eventloop/windows_pollops.c
+++ b/src/libmowgli/eventloop/windows_pollops.c
@@ -262,10 +262,10 @@ mowgli_winsock_eventloop_select(mowgli_eventloop_t *eventloop, int delay)
 
                        WSAEnumNetworkEvents(pollable->fd, priv->pfd[pollable->slot], &events);
 
-                       if (events.lNetworkEvents & (FD_READ | FD_CLOSE | FD_ACCEPT | FD_OOB))
+                       if (events.lNetworkEvents & (FD_READ | FD_CLOSE | FD_ACCEPT | FD_OOB) && !pollable->removed)
                                mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_READ);
 
-                       if (events.lNetworkEvents & (FD_WRITE | FD_CONNECT | FD_CLOSE))
+                       if (events.lNetworkEvents & (FD_WRITE | FD_CONNECT | FD_CLOSE) && !pollable->removed)
                                mowgli_pollable_trigger(eventloop, pollable, MOWGLI_EVENTLOOP_IO_WRITE);
                }
        }