File 0001-revert-loop-remove-destroy-list.patch of Package pipewire

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-revert-loop-remove-destroy-list.patch of Package pipewire (Revision 9a908e9cf0675fd5bfb7d901b0fecce7)

Currently displaying revision 9a908e9cf0675fd5bfb7d901b0fecce7 , Show latest

From 16f63a3c8fa227625bade5a9edea22354b347d18 Mon Sep 17 00:00:00 2001
From: Barnabás Pőcze <pobrn@protonmail.com>
Date: Fri, 18 Feb 2022 18:36:36 +0100
Subject: [PATCH] Revert "loop: remove destroy list"

This reverts commit c474846c42967c44db069a23b76a29da6f496f33.
In addition, `s->loop` is also checked before dispatching a source.

The destroy list is needed in the presence of threads. The
issue is that a source may be destroyed between `epoll_wait()`
returning and thread loop lock being acquired. If this
source is active, then a use-after-free will be triggered
when the thread loop acquires the lock and starts dispatching
the sources.

thread 1                       thread 2
 ----------                     ----------
                                loop_iterate
                                  spa_loop_control_hook_before
                                    // release lock

pw_thread_loop_lock

spa_system_pollfd_wait
                                    // assume it returns with source A

pw_loop_destroy_source(..., A)
  // frees storage of A

pw_thread_loop_unlock
                                  spa_loop_control_hook_after
                                    // acquire the lock

for (...) {
                                    struct spa_source *s = ep[i].data;
                                    s->rmask = ep[i].events;
                                      // use-after-free if `s` refers to
                                      // the previously freed `A`

Fixes #2147
---
 spa/plugins/support/loop.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/spa/plugins/support/loop.c b/spa/plugins/support/loop.c
index 0588ce770..04739eb2a 100644
--- a/spa/plugins/support/loop.c
+++ b/spa/plugins/support/loop.c
@@ -75,6 +75,7 @@ struct impl {
         struct spa_system *system;
 
 	struct spa_list source_list;
+	struct spa_list destroy_list;
 	struct spa_hook_list hooks_list;
 
 	int poll_fd;
@@ -325,6 +326,14 @@ static void loop_leave(void *object)
 		impl->thread = 0;
 }
 
+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);
+	spa_list_init(&impl->destroy_list);
+}
+
 static int loop_iterate(void *object, int timeout)
 {
 	struct impl *impl = object;
@@ -354,11 +363,14 @@ static int loop_iterate(void *object, int timeout)
 	}
 	for (i = 0; i < nfds; i++) {
 		struct spa_source *s = ep[i].data;
-		if (SPA_LIKELY(s && s->rmask)) {
+		if (SPA_LIKELY(s && s->rmask && s->loop)) {
 			s->priv = NULL;
 			s->func(s);
 		}
 	}
+	if (SPA_UNLIKELY(!spa_list_is_empty(&impl->destroy_list)))
+		process_destroy(impl);
+
 	return nfds;
 }
 
@@ -712,7 +724,7 @@ static void loop_destroy_source(void *object, struct spa_source *source)
 		spa_system_close(impl->impl->system, source->fd);
 		source->fd = -1;
 	}
-	free(source);
+	spa_list_insert(&impl->impl->destroy_list, &impl->link);
 }
 
 static const struct spa_loop_methods impl_loop = {
@@ -783,6 +795,8 @@ 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);
 
@@ -844,6 +858,7 @@ impl_init(const struct spa_handle_factory *factory,
 	impl->poll_fd = res;
 
 	spa_list_init(&impl->source_list);
+	spa_list_init(&impl->destroy_list);
 	spa_hook_list_init(&impl->hooks_list);
 
 	impl->buffer_data = SPA_PTR_ALIGN(impl->buffer_mem, MAX_ALIGN, uint8_t);
-- 
GitLab

Places

File 0001-revert-loop-remove-destroy-list.patch of Package pipewire (Revision 9a908e9cf0675fd5bfb7d901b0fecce7)

Places