From ab8fc98ec85cd8784afb07887bc38846b900ee63 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Thu, 14 Apr 2022 23:21:27 -0400 Subject: [PATCH] Fix: action error query: leak of action path MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit ==1429021==ERROR: LeakSanitizer: detected memory leaks Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x7fe305f031b2 in __interceptor_realloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:164 #1 0x559f1b022238 in lttng_dynamic_buffer_set_capacity(lttng_dynamic_buffer*, unsigned long) /home/jgalar/EfficiOS/src/lttng-tools/src/common/dynamic-buffer.cpp:159 #2 0x559f1b021d9f in lttng_dynamic_buffer_append(lttng_dynamic_buffer*, void const*, unsigned long) /home/jgalar/EfficiOS/src/lttng-tools/src/common/dynamic-buffer.cpp:52 #3 0x559f1b02144a in lttng_dynamic_array_add_element(lttng_dynamic_array*, void const*) /home/jgalar/EfficiOS/src/lttng-tools/src/common/dynamic-array.cpp:58 #4 0x559f1b07d07b in lttng_action_path_copy(lttng_action_path const*, lttng_action_path*) actions/path.cpp:116 #5 0x559f1b02383f in lttng_error_query_action_create /home/jgalar/EfficiOS/src/lttng-tools/src/common/error-query.cpp:232 #6 0x559f1b02760e in lttng_error_query_create_from_payload(lttng_payload_view*, lttng_error_query**) /home/jgalar/EfficiOS/src/lttng-tools/src/common/error-query.cpp:911 #7 0x559f1af5c361 in receive_lttng_error_query /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:740 #8 0x559f1af64eba in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2336 #9 0x559f1af67378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624 #10 0x559f1af50642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68 #11 0x7fe3055225c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1) Signed-off-by: Jérémie Galarneau Change-Id: I7a6f7d2a9746124581eebf30877466f16db67a6b --- include/lttng/action/path-internal.h | 3 +- src/common/actions/path.c | 29 +++++++----------- src/common/error-query.c | 46 +++++++++++++++++++++++----- 3 files changed, 50 insertions(+), 28 deletions(-) diff --git a/include/lttng/action/path-internal.h b/include/lttng/action/path-internal.h index 608f144c2..9f7eaaaa1 100644 --- a/include/lttng/action/path-internal.h +++ b/include/lttng/action/path-internal.h @@ -23,10 +23,9 @@ struct lttng_action_path { struct lttng_dynamic_array indexes; }; -/* Assumes that 'dst' is uninitialized. */ LTTNG_HIDDEN int lttng_action_path_copy(const struct lttng_action_path *src, - struct lttng_action_path *dst); + struct lttng_action_path **dst); LTTNG_HIDDEN ssize_t lttng_action_path_create_from_payload( diff --git a/src/common/actions/path.c b/src/common/actions/path.c index 4ee649928..250b5e905 100644 --- a/src/common/actions/path.c +++ b/src/common/actions/path.c @@ -97,32 +97,25 @@ end: LTTNG_HIDDEN int lttng_action_path_copy(const struct lttng_action_path *src, - struct lttng_action_path *dst) + struct lttng_action_path **dst) { int ret; - size_t i, src_count; + struct lttng_action_path *new_path; assert(src); assert(dst); - lttng_dynamic_array_init(&dst->indexes, sizeof(uint64_t), NULL); - src_count = lttng_dynamic_array_get_count(&src->indexes); - - for (i = 0; i < src_count; i++) { - const void *index = lttng_dynamic_array_get_element( - &src->indexes, i); - - ret = lttng_dynamic_array_add_element(&dst->indexes, index); - if (ret) { - goto error; - } + new_path = lttng_action_path_create( + (uint64_t *) lttng_dynamic_array_get_element( + &src->indexes, 0), + lttng_dynamic_array_get_count(&src->indexes)); + if (!new_path) { + ret = -1; + } else { + ret = 0; + *dst = new_path; } - ret = 0; - goto end; -error: - lttng_dynamic_array_reset(&dst->indexes); -end: return ret; } diff --git a/src/common/error-query.c b/src/common/error-query.c index 2eb1c9717..298809711 100644 --- a/src/common/error-query.c +++ b/src/common/error-query.c @@ -47,7 +47,7 @@ struct lttng_error_query_action { struct lttng_error_query parent; /* Mutable only because of the reference count. */ struct lttng_trigger *trigger; - struct lttng_action_path action_path; + struct lttng_action_path *action_path; }; struct lttng_error_query_result { @@ -246,15 +246,45 @@ end: void lttng_error_query_destroy(struct lttng_error_query *query) { - struct lttng_error_query_trigger *trigger_query; - if (!query) { return; } - trigger_query = container_of(query, typeof(*trigger_query), parent); - lttng_trigger_put(trigger_query->trigger); - free(trigger_query); + switch (query->target_type) { + case LTTNG_ERROR_QUERY_TARGET_TYPE_TRIGGER: + { + struct lttng_error_query_trigger *trigger_query = + container_of(query, typeof(*trigger_query), + parent); + + lttng_trigger_put(trigger_query->trigger); + free(trigger_query); + break; + } + case LTTNG_ERROR_QUERY_TARGET_TYPE_CONDITION: + { + struct lttng_error_query_condition *condition_query = + container_of(query, typeof(*condition_query), + parent); + + lttng_trigger_put(condition_query->trigger); + free(condition_query); + break; + } + case LTTNG_ERROR_QUERY_TARGET_TYPE_ACTION: + { + struct lttng_error_query_action *action_query = + container_of(query, typeof(*action_query), + parent); + + lttng_trigger_put(action_query->trigger); + lttng_action_path_destroy(action_query->action_path); + free(action_query); + break; + } + default: + abort(); + } } static @@ -692,7 +722,7 @@ int lttng_error_query_action_serialize(const struct lttng_error_query *query, goto end; } - ret = lttng_action_path_serialize(&query_action->action_path, payload); + ret = lttng_action_path_serialize(query_action->action_path, payload); if (ret) { goto end; } @@ -747,7 +777,7 @@ struct lttng_action *lttng_error_query_action_borrow_action_target( container_of(query, typeof(*query_action), parent); return get_trigger_action_from_path( - trigger, &query_action->action_path); + trigger, query_action->action_path); } LTTNG_HIDDEN -- 2.34.1