use-after-free when node is destroyed while async param set is in progress
See:
[I][206213][51607.320387] pw.node | [ impl-node.c: 408 node_update_state()] (Mutter-78) creating -> suspended
[I][206213][51607.323619] pw.context | [ context.c: 1194 pw_context_recalc_graph()] 0x61a000000c80: busy:0 reason:node activate
[I][206213][51607.355669] pw.node | [ impl-node.c: 2034 pw_impl_node_destroy()] (Mutter-78) destroy
[I][206213][51607.355819] pw.context | [ context.c: 1194 pw_context_recalc_graph()] 0x61a000000c80: busy:0 reason:active node destroy
=================================================================
==206213==ERROR: AddressSanitizer: heap-use-after-free on address 0x617000047ce0 at pc 0x7fb5978c2560 bp 0x7ffe99d16880 sp 0x7ffe99d16870
WRITE of size 8 at 0x617000047ce0 thread T0
#0 0x7fb5978c255f in spa_list_remove ../spa/include/spa/utils/list.h:61
#1 0x7fb5978c290c in spa_hook_remove ../spa/include/spa/utils/hook.h:385
#2 0x7fb5978dee81 in remove_busy_resource ../src/pipewire/impl-node.c:534
#3 0x7fb5978e0965 in resource_destroy ../src/pipewire/impl-node.c:614
#4 0x7fb59799fcd5 in pw_resource_destroy ../src/pipewire/resource.c:325
#5 0x7fb59785422f in pw_global_destroy ../src/pipewire/global.c:399
#6 0x7fb59777423a in registry_destroy ../src/pipewire/impl-core.c:107
#7 0x7fb59133efb8 in registry_demarshal_destroy ../src/modules/module-protocol-native/protocol-native.c:810
#8 0x7fb591306f42 in process_messages ../src/modules/module-protocol-native.c:383pw_context_recalc_graph()] 0x61a000000c80: busy:0 reason:active node destroy
#9 0x7fb59130883f in connection_data ../src/modules/module-protocol-native.c:454
#10 0x7fb5930cc18c in source_io_func ../spa/plugins/support/loop.c:512
#11 0x7fb5930cbbdb in loop_iterate ../spa/plugins/support/loop.c:496
#12 0x7fb5978a7de4 in pw_main_loop_run ../src/pipewire/main-loop.c:128
#13 0x56112f689722 in main ../src/daemon/pipewire.c:111
#14 0x7fb598245ccf (/usr/lib/libc.so.6+0x27ccf) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
#15 0x7fb598245d89 in __libc_start_main (/usr/lib/libc.so.6+0x27d89) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
#16 0x56112f6882a4 in _start (/home/pb/temp/src/pipewire/build/src/daemon/pipewire+0x42a4) (BuildId: 2ed9678cd6b377f0dee04014fb5e5001474e37a3)
0x617000047ce0 is located 96 bytes inside of 704-byte region [0x617000047c80,0x617000047f40)
freed by thread T0 here:
#0 0x7fb5984dfdb2 in __interceptor_free /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:52
#1 0x7fb59077c6b4 in node_free ../src/modules/module-client-node/client-node.c:1369
#2 0x7fb59790dbd9 in pw_impl_node_destroy ../src/pipewire/impl-node.c:2082
#3 0x7fb5978e1ae1 in global_destroy ../src/pipewire/impl-node.c:677
#4 0x7fb597854028 in pw_global_destroy ../src/pipewire/global.c:396
#5 0x7fb59777423a in registry_destroy ../src/pipewire/impl-core.c:107
#6 0x7fb59133efb8 in registry_demarshal_destroy ../src/modules/module-protocol-native/protocol-native.c:810
#7 0x7fb591306f42 in process_messages ../src/modules/module-protocol-native.c:383
#8 0x7fb59130883f in connection_data ../src/modules/module-protocol-native.c:454
#9 0x7fb5930cc18c in source_io_func ../spa/plugins/support/loop.c:512
#10 0x7fb5930cbbdb in loop_iterate ../spa/plugins/support/loop.c:496
#11 0x7fb5978a7de4 in pw_main_loop_run ../src/pipewire/main-loop.c:128
#12 0x56112f689722 in main ../src/daemon/pipewire.c:111
#13 0x7fb598245ccf (/usr/lib/libc.so.6+0x27ccf) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
previously allocated by thread T0 here:
#0 0x7fb5984e0cc1 in __interceptor_calloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77
#1 0x7fb5907811e0 in pw_impl_client_node_new ../src/modules/module-client-node/client-node.c:1657
#2 0x7fb59072e31b in create_object ../src/modules/module-client-node.c:79
#3 0x7fb59791b006 in pw_impl_factory_create_object ../src/pipewire/impl-factory.c:254
#4 0x7fb597778632 in core_create_object ../src/pipewire/impl-core.c:328
#5 0x7fb59133c80a in core_method_demarshal_create_object ../src/modules/module-protocol-native/protocol-native.c:706
#6 0x7fb591306f42 in process_messages ../src/modules/module-protocol-native.c:383
#7 0x7fb59130883f in connection_data ../src/modules/module-protocol-native.c:454
#8 0x7fb5930cc18c in source_io_func ../spa/plugins/support/loop.c:512
#9 0x7fb5930cbbdb in loop_iterate ../spa/plugins/support/loop.c:496
#10 0x7fb5978a7de4 in pw_main_loop_run ../src/pipewire/main-loop.c:128
#11 0x56112f689722 in main ../src/daemon/pipewire.c:111
#12 0x7fb598245ccf (/usr/lib/libc.so.6+0x27ccf) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
SUMMARY: AddressSanitizer: heap-use-after-free ../spa/include/spa/utils/list.h:61 in spa_list_remove
Shadow bytes around the buggy address:
0x617000047a00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x617000047a80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x617000047b00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x617000047b80: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x617000047c00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x617000047c80: fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd
0x617000047d00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x617000047d80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x617000047e00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x617000047e80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x617000047f00: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==206213==ABORTING
Félbeszakítva (core készült)
This is because the pw_impl_node
and the contained spa_node
objects are destroyed before the impl-node.c:resource_data
objects that refer to them. This is especially problematic if an async param set is in progress (impl-node.c:node_set_param()
) because in that case the resource_data
will be on a spa_hook_list
that has already been destroyed (in this case: in client-node.c:node_free()
). So when the resource is eventually destroyed, impl-node.c:remove_busy_resource()
will call spa_hook_remove()
, which leads to the use-after-free seen above.
I believe something like the following fixes this issue but I think there should be a better solution.
diff --git a/src/pipewire/impl-node.c b/src/pipewire/impl-node.c
index 6bfa26b16..a8bacfc24 100644
--- a/src/pipewire/impl-node.c
+++ b/src/pipewire/impl-node.c
@@ -39,6 +39,7 @@ struct impl {
struct spa_list param_list;
struct spa_list pending_list;
+ struct spa_list resources;
unsigned int cache_params:1;
unsigned int pending_play:1;
@@ -49,6 +50,7 @@ struct impl {
#define pw_node_resource_param(r,...) pw_node_resource(r,param,0,__VA_ARGS__)
struct resource_data {
+ struct spa_list link;
struct pw_impl_node *node;
struct pw_resource *resource;
@@ -608,12 +610,17 @@ static const struct pw_node_methods node_methods = {
.send_command = node_send_command
};
-static void resource_destroy(void *data)
+static void resource_data_clear(struct resource_data *d)
{
- struct resource_data *d = data;
remove_busy_resource(d);
spa_hook_remove(&d->resource_listener);
spa_hook_remove(&d->object_listener);
+ spa_list_remove(&d->link);
+}
+
+static void resource_destroy(void *data)
+{
+ resource_data_clear(data);
}
static void resource_pong(void *data, int seq)
@@ -655,6 +662,8 @@ global_bind(void *object, struct pw_impl_client *client, uint32_t permissions,
&data->object_listener,
&node_methods, data);
+ spa_list_append(&SPA_CONTAINER_OF(this, struct impl, this)->resources, &data->link);
+
pw_log_debug("%p: bound to %d", this, resource->id);
pw_global_add_resource(global, resource);
@@ -1347,6 +1356,7 @@ struct pw_impl_node *pw_context_create_node(struct pw_context *context,
spa_list_init(&impl->param_list);
spa_list_init(&impl->pending_list);
+ spa_list_init(&impl->resources);
this = &impl->this;
this->context = context;
@@ -2069,6 +2079,10 @@ void pw_impl_node_destroy(struct pw_impl_node *node)
spa_list_consume(port, &node->output_ports, link)
pw_impl_port_destroy(port);
+ struct resource_data *d;
+ spa_list_consume(d, &impl->resources, link)
+ resource_data_clear(d);
+
if (node->global) {
spa_hook_remove(&node->global_listener);
pw_global_destroy(node->global);