From c8c2526f87e6631700e81f449138f0e24fb15a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 9 Jun 2022 02:48:54 +0200 Subject: [PATCH] pulse-server: destroy modules first The `impl::servers` list contains all servers, and they are all destroyed in `impl_clear()`. However, by that time, modules were not freed previously, so if there were any instances of *module-protocol-native-tcp* loaded, then the unload() method of those would call `server_free()` on already freed servers, resulting in use-after-frees. Fix that by unloading modules before destroying the servers. ==451490==ERROR: AddressSanitizer: heap-use-after-free on address 0x612000006050 ... READ of size 8 at 0x612000006050 thread T0 #0 0x7f45edb19a0c in server_free ../src/modules/module-protocol-pulse/server.c:1022 #1 0x7f45edb46c7d in module_native_protocol_tcp_unload ../src/modules/module-protocol-pulse/modules/module-native-protocol-tcp.c:66 #2 0x7f45eda893c7 in module_unload ../src/modules/module-protocol-pulse/module.c:128 #3 0x7f45edaf7269 in impl_unload_module ../src/modules/module-protocol-pulse/pulse-server.c:5336 #4 0x7f45edaa1583 in pw_map_for_each ../src/pipewire/map.h:238 #5 0x7f45edaf79c5 in impl_clear ../src/modules/module-protocol-pulse/pulse-server.c:5358 ... 0x612000006050 is located 16 bytes inside of 264-byte region [0x612000006040,0x612000006148) freed by thread T0 here: #0 0x7f45f30be672 in __interceptor_free /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:52 #1 0x7f45edb1a48a in server_free ../src/modules/module-protocol-pulse/server.c:1040 #2 0x7f45edaf730b in impl_clear ../src/modules/module-protocol-pulse/pulse-server.c:5347 ... Fixes: f181210b2996 ("pulse-server: properly unload modules") --- src/modules/module-protocol-pulse/pulse-server.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/module-protocol-pulse/pulse-server.c b/src/modules/module-protocol-pulse/pulse-server.c index 8194e2791..7dead6055 100644 --- a/src/modules/module-protocol-pulse/pulse-server.c +++ b/src/modules/module-protocol-pulse/pulse-server.c @@ -5343,6 +5343,9 @@ static void impl_clear(struct impl *impl) struct server *s; struct client *c; + pw_map_for_each(&impl->modules, impl_unload_module, impl); + pw_map_clear(&impl->modules); + spa_list_consume(s, &impl->servers, link) server_free(s); @@ -5355,9 +5358,6 @@ static void impl_clear(struct impl *impl) pw_map_for_each(&impl->samples, impl_free_sample, impl); pw_map_clear(&impl->samples); - pw_map_for_each(&impl->modules, impl_unload_module, impl); - pw_map_clear(&impl->modules); - #ifdef HAVE_DBUS if (impl->dbus_name) { dbus_release_name(impl->dbus_name);