pipewire: Backport crash fix 78/27278/2
authorScott Murray <scott.murray@konsulko.com>
Wed, 23 Mar 2022 15:27:06 +0000 (11:27 -0400)
committerJan-Simon Moeller <jsmoeller@linuxfoundation.org>
Thu, 24 Mar 2022 15:15:24 +0000 (15:15 +0000)
Backport revert done post-0.3.47 to fix crashes seen in MPD.
See upstream discussion here:

https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/2147

Bug-AGL: SPEC-4294

Signed-off-by: Scott Murray <scott.murray@konsulko.com>
Change-Id: I48bf5bc632ad50daa43a3e8125ec8885f7f3a537
Reviewed-on: https://gerrit.automotivelinux.org/gerrit/c/AGL/meta-agl/+/27278
Tested-by: Jenkins Job builder account
ci-image-build: Jenkins Job builder account
ci-image-boot-test: Jenkins Job builder account
Reviewed-by: Jan-Simon Moeller <jsmoeller@linuxfoundation.org>
meta-pipewire/recipes-multimedia/pipewire/pipewire/0013-Revert-loop-remove-destroy-list.patch [new file with mode: 0644]
meta-pipewire/recipes-multimedia/pipewire/pipewire_0.3.47.bbappend

diff --git a/meta-pipewire/recipes-multimedia/pipewire/pipewire/0013-Revert-loop-remove-destroy-list.patch b/meta-pipewire/recipes-multimedia/pipewire/pipewire/0013-Revert-loop-remove-destroy-list.patch
new file mode 100644 (file)
index 0000000..8a988b0
--- /dev/null
@@ -0,0 +1,120 @@
+From 16f63a3c8fa227625bade5a9edea22354b347d18 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= <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
+
+Upstream-Status: Backport [https://gitlab.freedesktop.org/pipewire/pipewire/-/commit/16f63a3c]
+Signed-off-by: Scott Murray <scott.murray@konsulko.com>
+
+---
+ 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);
+-- 
+2.35.1
+
index 8c8467b..5c0d065 100644 (file)
@@ -19,6 +19,7 @@ SRC_URI += "\
     file://0010-Revert-spa-improve-the-AEC-interface.patch \
     file://0011-Revert-module-echo-cancel-Move-backends-to-dynamic-l.patch \
     file://0012-Miscellanous-changes-to-account-for-lower-version-of.patch \
+    file://0013-Revert-loop-remove-destroy-list.patch \
 "
 
 do_install:append() {