From 970b5906a8472864ad770073c2a2b41e898a8fad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Mon, 16 May 2022 19:13:17 +0200 Subject: [PATCH] pipewire: module-spa-node-factory: remove resource listener when node is destroyed Previously, the resource listener was not removed when the `node_data` object was freed, which could lead to a use-after-free when the resource emitted an event later. ==2787072==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d000016728 at pc 0x7ffff7175b52 bp 0x7fffffffb930 sp 0x7fffffffb920 WRITE of size 8 at 0x61d000016728 thread T0 #0 0x7ffff7175b51 in spa_list_remove ../spa/include/spa/utils/list.h:77 #1 0x7ffff717cb5a in pw_resource_destroy ../src/pipewire/resource.c:335 #2 0x7ffff7051c56 in pw_global_destroy ../src/pipewire/global.c:417 #3 0x7ffff6f82a68 in registry_destroy ../src/pipewire/impl-core.c:130 #4 0x7ffff3a5f349 in registry_demarshal_destroy ../src/modules/module-protocol-native/protocol-native.c:784 #5 0x7ffff3a2c9ed in process_messages ../src/modules/module-protocol-native.c:352 #6 0x7ffff3a2e2ea in connection_data ../src/modules/module-protocol-native.c:423 #7 0x7ffff3e09402 in source_io_func ../spa/plugins/support/loop.c:427 #8 0x7ffff3e0851d in loop_iterate ../spa/plugins/support/loop.c:409 #9 0x7ffff709c21d in pw_main_loop_run ../src/pipewire/main-loop.c:148 #10 0x555555559722 in main ../src/daemon/pipewire.c:131 #11 0x7ffff62a528f (/usr/lib/libc.so.6+0x2928f) #12 0x7ffff62a5349 in __libc_start_main (/usr/lib/libc.so.6+0x29349) #13 0x5555555582a4 in _start (./src/daemon/pipewire+0x42a4) 0x61d000016728 is located 2216 bytes inside of 2264-byte region [0x61d000015e80,0x61d000016758) freed by thread T0 here: #0 0x7ffff798c672 in __interceptor_free /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:52 #1 0x7ffff70f9bc3 in pw_impl_node_destroy ../src/pipewire/impl-node.c:1880 #2 0x7ffff70d1d57 in global_destroy ../src/pipewire/impl-node.c:638 #3 0x7ffff7051a4f in pw_global_destroy ../src/pipewire/global.c:414 #4 0x7ffff6f82a68 in registry_destroy ../src/pipewire/impl-core.c:130 #5 0x7ffff3a5f349 in registry_demarshal_destroy ../src/modules/module-protocol-native/protocol-native.c:784 #6 0x7ffff3a2c9ed in process_messages ../src/modules/module-protocol-native.c:352 #7 0x7ffff3a2e2ea in connection_data ../src/modules/module-protocol-native.c:423 #8 0x7ffff3e09402 in source_io_func ../spa/plugins/support/loop.c:427 #9 0x7ffff3e0851d in loop_iterate ../spa/plugins/support/loop.c:409 #10 0x7ffff709c21d in pw_main_loop_run ../src/pipewire/main-loop.c:148 #11 0x555555559722 in main ../src/daemon/pipewire.c:131 #12 0x7ffff62a528f (/usr/lib/libc.so.6+0x2928f) previously allocated by thread T0 here: #0 0x7ffff798d411 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x7ffff70e5bb7 in pw_context_create_node ../src/pipewire/impl-node.c:1192 #2 0x7ffff28c748e in pw_spa_node_new ../src/modules/spa/spa-node.c:112 #3 0x7ffff28c9a9f in pw_spa_node_load ../src/modules/spa/spa-node.c:276 #4 0x7ffff28c1618 in create_object ../src/modules/spa/module-node-factory.c:134 #5 0x7ffff7106c4e in pw_impl_factory_create_object ../src/pipewire/impl-factory.c:273 #6 0x7ffff6f86dd7 in core_create_object ../src/pipewire/impl-core.c:349 #7 0x7ffff3a5cba9 in core_method_demarshal_create_object ../src/modules/module-protocol-native/protocol-native.c:680 #8 0x7ffff3a2c9ed in process_messages ../src/modules/module-protocol-native.c:352 #9 0x7ffff3a2e2ea in connection_data ../src/modules/module-protocol-native.c:423 #10 0x7ffff3e09402 in source_io_func ../spa/plugins/support/loop.c:427 #11 0x7ffff3e0851d in loop_iterate ../spa/plugins/support/loop.c:409 #12 0x7ffff709c21d in pw_main_loop_run ../src/pipewire/main-loop.c:148 #13 0x555555559722 in main ../src/daemon/pipewire.c:131 #14 0x7ffff62a528f (/usr/lib/libc.so.6+0x2928f) SUMMARY: AddressSanitizer: heap-use-after-free ../spa/include/spa/utils/list.h:77 in spa_list_remove --- src/modules/spa/module-node-factory.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/modules/spa/module-node-factory.c b/src/modules/spa/module-node-factory.c index 9c66a62f7..7fb2a42b1 100644 --- a/src/modules/spa/module-node-factory.c +++ b/src/modules/spa/module-node-factory.c @@ -66,6 +66,7 @@ struct node_data { struct spa_list link; struct pw_impl_node *node; struct spa_hook node_listener; + struct pw_resource *resource; struct spa_hook resource_listener; unsigned int linger:1; }; @@ -75,6 +76,7 @@ static void resource_destroy(void *data) struct node_data *nd = data; pw_log_debug("node %p", nd); spa_hook_remove(&nd->resource_listener); + nd->resource = NULL; if (nd->node && !nd->linger) pw_impl_node_destroy(nd->node); } @@ -91,6 +93,11 @@ static void node_destroy(void *data) spa_list_remove(&nd->link); spa_hook_remove(&nd->node_listener); nd->node = NULL; + + if (nd->resource) { + spa_hook_remove(&nd->resource_listener); + nd->resource = NULL; + } } static const struct pw_impl_node_events node_events = { @@ -148,17 +155,15 @@ static void *create_object(void *_data, pw_impl_node_add_listener(node, &nd->node_listener, &node_events, nd); if (client) { - struct pw_resource *bound_resource; - res = pw_global_bind(pw_impl_node_get_global(node), client, PW_PERM_ALL, version, new_id); if (res < 0) goto error_bind; - if ((bound_resource = pw_impl_client_find_resource(client, new_id)) == NULL) + if ((nd->resource = pw_impl_client_find_resource(client, new_id)) == NULL) goto error_bind; - pw_resource_add_listener(bound_resource, &nd->resource_listener, &resource_events, nd); + pw_resource_add_listener(nd->resource, &nd->resource_listener, &resource_events, nd); } return node;