spa: support: loop: fix use-after-free when loop is reentered

The core of the issue is the following: what happens if an
active source is destroyed before it could be dispatched?

For loop-managed sources (`struct source_impl`) this was addressed
by storing all destroyed sources in a list, and only freeing them
after dispatching has been finished. (0eb73f0f06)
This approach works for both strictly single-threaded
and `pw_thread_loop` loops assuming the loop is not
reentered.

However, if the loop is reentered, there can still be issues.
Assume that in one iteration sources A and B are active,
and returned from the system call, and source B is destroyed
before the loop starts dispatching. Consider what happens when
"A" is dispatched first, and it reenters the loop with timeout 0.
Imagine there are no new events, so `loop_iterate()` will immediately
return, but it will first destroy everything in the destroy list
(this is done at the end of `loop_iterate()`).
And herein lies the problem. In the previous iteration,
there exists a `spa_poll_event` object which points to source "B",
but that has just been destroyed at the end of the recursive
iteration. This will trigger a use-after-free once the previous
iteration inspects it.

Fix that by processing the destroy list right after first
processing the returned `spa_poll_event` objects, and
"detach" the source from the loop and its iterations
in `process_destroy()` before the source is destroyed.

See #2114 #2147

spa/plugins/support/loop.c

@@ -132,23 +132,35 @@ static int loop_update_source(void *object, struct spa_source *source)
 	return spa_system_pollfd_mod(impl->system, impl->poll_fd, source->fd, source->mask, source);
 }
 
-static int loop_remove_source(void *object, struct spa_source *source)
+static void detach_source(struct spa_source *source)
 {
-	struct impl *impl = object;
 	struct spa_poll_event *e;
 
-	spa_assert(source->loop == &impl->loop);
+	source->loop = NULL;
+	source->rmask = 0;
 
 	if ((e = source->priv)) {
 		/* active in an iteration of the loop, remove it from there */
 		e->data = NULL;
 		source->priv = NULL;
 	}
-	source->loop = NULL;
-	source->rmask = 0;
+}
+
+static int remove_from_poll(struct impl *impl, struct spa_source *source)
+{
+	spa_assert(source->loop == &impl->loop);
+
 	return spa_system_pollfd_del(impl->system, impl->poll_fd, source->fd);
 }
 
+static int loop_remove_source(void *object, struct spa_source *source)
+{
+	int res = remove_from_poll(object, source);
+	detach_source(source);
+
+	return res;
+}
+
 static void flush_items(struct impl *impl)
 {
 	uint32_t index;
@@ -337,11 +349,19 @@ static void loop_leave(void *object)
 		impl->thread = 0;
 }
 
+static inline void free_source(struct source_impl *s)
+{
+	detach_source(&s->source);
+	free(s);
+}
+
 static inline void process_destroy(struct impl *impl)
 {
 	struct source_impl *source, *tmp;
+
 	spa_list_for_each_safe(source, tmp, &impl->destroy_list, link)
-		free(source);
+		free_source(source);
+
 	spa_list_init(&impl->destroy_list);
 }
 
@@ -371,11 +391,16 @@ static int loop_iterate(void *object, int timeout)
 			e->data = NULL;
 		s->priv = &ep[i];
 	}
+
+	if (SPA_UNLIKELY(!spa_list_is_empty(&impl->destroy_list)))
+		process_destroy(impl);
+
 	for (i = 0; i < nfds; i++) {
 		struct spa_source *s = ep[i].data;
-		if (SPA_LIKELY(s && s->rmask && s->loop))
+		if (SPA_LIKELY(s && s->rmask))
 			s->func(s);
 	}
+
 	for (i = 0; i < nfds; i++) {
 		struct spa_source *s = ep[i].data;
 		if (SPA_LIKELY(s)) {
@@ -383,8 +408,6 @@ static int loop_iterate(void *object, int timeout)
 			s->priv = NULL;
 		}
 	}
-	if (SPA_UNLIKELY(!spa_list_is_empty(&impl->destroy_list)))
-		process_destroy(impl);
 
 	return nfds;
 }
@@ -748,13 +771,17 @@ static void loop_destroy_source(void *object, struct spa_source *source)
 
 	if (s->fallback)
 		loop_destroy_source(s->impl, s->fallback);
-	else if (source->loop)
-		loop_remove_source(s->impl, source);
+	else
+		remove_from_poll(s->impl, source);
 
 	if (source->fd != -1 && s->close) {
 		spa_system_close(s->impl->system, source->fd);
 		source->fd = -1;
 	}
+
+	if (!s->impl->polling)
+		free_source(s);
+	else
 		spa_list_insert(&s->impl->destroy_list, &s->link);
 }
 
@@ -826,8 +853,6 @@ static int impl_clear(struct spa_handle *handle)
 	spa_list_consume(source, &impl->source_list, link)
 		loop_destroy_source(impl, &source->source);
 
-	process_destroy(impl);
-
 	spa_system_close(impl->system, impl->ack_fd);
 	spa_system_close(impl->system, impl->poll_fd);