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.

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);
 	}
 }

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);
 }
 

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);

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

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);

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

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);
 	}
 }

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);
 }
 

src/libmowgli/eventloop/select_pollops.c

@@ -166,26 +166,20 @@ 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);
 # endif
 
 				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);

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);
 		}
 	}