Skip to content

Commit

Permalink
pulse-server: destroy modules first
Browse files Browse the repository at this point in the history
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: f181210 ("pulse-server: properly unload modules")
  • Loading branch information
pobrn committed Jun 9, 2022
1 parent b99c712 commit c8c2526
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/modules/module-protocol-pulse/pulse-server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down

0 comments on commit c8c2526

Please sign in to comment.