From 2ebb27909ba460be6b58e24750e1cd55b12729df Mon Sep 17 00:00:00 2001 From: Jonathan Rajotte Date: Tue, 17 May 2022 14:09:42 -0400 Subject: [PATCH] Fix: statedump: invalid read during iter_end Scenario ========= Using a modified doc/examples/easy-ust/sample.c and dummy shared objects: int main(int argc, char **argv) { int i = 0; void *handle_cat; void *handle_dog; void (*func_print_name_cat)(const char*); void (*func_print_name_dog)(const char*); handle_dog = dlopen("./libdog.so", RTLD_NOW); handle_cat = dlopen("./libcat.so", RTLD_NOW); *(void**)(&func_print_name_dog) = dlsym(handle_dog, "print_name"); *(void**)(&func_print_name_cat) = dlsym(handle_cat, "print_name"); for (i = 0; i < 5; i++) { tracepoint(sample_component, message, "Hello World"); usleep(1); } printf("Run `lttng regenerate statedump. Press enter \n"); getchar(); dlclose(handle_dog); printf("Run `lttng regenerate statedump. Press enter \n"); getchar(); dlclose(handle_cat); return 0; } On lttng side: lttng create lttng enable-event -u -a lttng start valgrind sample Issue `lttng regenerate statedump` as the app suggest. The second `lttng regenerate statedump` results in: ==934747== Invalid read of size 8 ==934747== at 0x48BA90F: iter_end (lttng-ust-statedump.c:439) ==934747== by 0x48BAD73: lttng_ust_dl_update (lttng-ust-statedump.c:586) ==934747== by 0x48BADC0: do_baddr_statedump (lttng-ust-statedump.c:599) ==934747== by 0x48BAE62: do_lttng_ust_statedump (lttng-ust-statedump.c:633) ==934747== by 0x489F820: lttng_handle_pending_statedump (lttng-events.c:969) ==934747== by 0x488C000: handle_pending_statedump (lttng-ust-comm.c:717) ==934747== by 0x488DCF7: handle_message (lttng-ust-comm.c:1110) ==934747== by 0x48905EA: ust_listener_thread (lttng-ust-comm.c:1756) ==934747== by 0x4B62608: start_thread (pthread_create.c:477) ==934747== by 0x4A4D162: clone (clone.S:95) ==934747== Address 0x4c4ea88 is 4,152 bytes inside a block of size 4,176 free'd ==934747== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==934747== by 0x48B9588: free_dl_node (lttng-ust-statedump.c:123) ==934747== by 0x48BA90A: iter_end (lttng-ust-statedump.c:450) ==934747== by 0x48BAD73: lttng_ust_dl_update (lttng-ust-statedump.c:586) ==934747== by 0x48BADC0: do_baddr_statedump (lttng-ust-statedump.c:599) ==934747== by 0x48BAE62: do_lttng_ust_statedump (lttng-ust-statedump.c:633) ==934747== by 0x489F820: lttng_handle_pending_statedump (lttng-events.c:969) ==934747== by 0x488C000: handle_pending_statedump (lttng-ust-comm.c:717) ==934747== by 0x488DCF7: handle_message (lttng-ust-comm.c:1110) ==934747== by 0x48905EA: ust_listener_thread (lttng-ust-comm.c:1756) ==934747== by 0x4B62608: start_thread (pthread_create.c:477) ==934747== by 0x4A4D162: clone (clone.S:95) ==934747== Block was alloc'd at ==934747== at 0x483DD99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==934747== by 0x48B936A: zmalloc (helper.h:27) ==934747== by 0x48B936A: alloc_dl_node (lttng-ust-statedump.c:85) ==934747== by 0x48B98F7: find_or_create_dl_node (lttng-ust-statedump.c:184) ==934747== by 0x48BA205: extract_baddr (lttng-ust-statedump.c:339) ==934747== by 0x48BABC6: extract_bin_info_events (lttng-ust-statedump.c:528) ==934747== by 0x4A8D2F4: dl_iterate_phdr (dl-iteratephdr.c:75) ==934747== by 0x48BAD4C: lttng_ust_dl_update (lttng-ust-statedump.c:583) ==934747== by 0x48BADC0: do_baddr_statedump (lttng-ust-statedump.c:599) ==934747== by 0x48BAE62: do_lttng_ust_statedump (lttng-ust-statedump.c:633) ==934747== by 0x489F820: lttng_handle_pending_statedump (lttng-events.c:969) ==934747== by 0x488C000: handle_pending_statedump (lttng-ust-comm.c:717) ==934747== by 0x488DCF7: handle_message (lttng-ust-comm.c:1110) ==934747== Cause ========= Nodes can be removed during the `cds_hlist_for_each_entry_2` iteration which is not meant to be used when items are removed within the traversal. Solution ========= Use `cds_hlist_for_each_entry_safe_2`. Change-Id: Ibf3d94a4d6f7abac19ed9740eeacfbcb1bdf1f4f Signed-off-by: Jonathan Rajotte Signed-off-by: Mathieu Desnoyers --- src/lib/lttng-ust/lttng-ust-statedump.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/lttng-ust/lttng-ust-statedump.c b/src/lib/lttng-ust/lttng-ust-statedump.c index 64294706..309a98fa 100644 --- a/src/lib/lttng-ust/lttng-ust-statedump.c +++ b/src/lib/lttng-ust/lttng-ust-statedump.c @@ -423,10 +423,10 @@ void iter_end(struct dl_iterate_data *data, void *ip) */ for (i = 0; i < UST_DL_STATE_TABLE_SIZE; i++) { struct cds_hlist_head *head; - struct lttng_ust_dl_node *e; + struct lttng_ust_dl_node *e, *tmp; head = &dl_state_table[i]; - cds_hlist_for_each_entry_2(e, head, node) { + cds_hlist_for_each_entry_safe_2(e, tmp, head, node) { if (e->marked) { if (!e->traced) { trace_lib_load(&e->bin_data, ip); -- 2.34.1