From a7d0c51c52357e361b9834d04d39a40b16edb46f Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Fri, 15 Apr 2022 01:30:50 -0400 Subject: [PATCH] Fix: sessiond: rotation trigger leak MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit ==1801304==ERROR: LeakSanitizer: detected memory leaks Direct leak of 224 byte(s) in 2 object(s) allocated from: #0 0x7fe0f4e73fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x559fbeb64175 in zmalloc_internal ../../src/common/macros.hpp:60 #2 0x559fbeb6a291 in lttng_trigger* zmalloc() ../../src/common/macros.hpp:89 #3 0x559fbeb64aa6 in lttng_trigger_create /home/jgalar/EfficiOS/src/lttng-tools/src/common/trigger.cpp:58 #4 0x559fbe9dc417 in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:87 #5 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993 #6 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246 #7 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624 #8 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68 #9 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1) Indirect leak of 208 byte(s) in 2 object(s) allocated from: #0 0x7fe0f4e73fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x559fbeb16e21 in zmalloc_internal ../../src/common/macros.hpp:60 #2 0x559fbeb16e31 in lttng_action_notify* zmalloc() ../../src/common/macros.hpp:89 #3 0x559fbeb168a0 in lttng_action_notify_create actions/notify.cpp:135 #4 0x559fbe9dc34b in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:80 #5 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993 #6 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246 #7 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624 #8 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68 #9 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1) Indirect leak of 160 byte(s) in 2 object(s) allocated from: #0 0x7fe0f4e73fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x559fbeb3d7a1 in zmalloc_internal ../../src/common/macros.hpp:60 #2 0x559fbeb3fa35 in lttng_condition_session_consumed_size* zmalloc() ../../src/common/macros.hpp:89 #3 0x559fbeb3e6fd in lttng_condition_session_consumed_size_create conditions/session-consumed-size.cpp:206 #4 0x559fbe9dc0f1 in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:54 #5 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993 #6 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246 #7 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624 #8 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68 #9 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1) Indirect leak of 112 byte(s) in 2 object(s) allocated from: #0 0x7fe0f4e73fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x559fbeb242ad in zmalloc_internal ../../src/common/macros.hpp:60 #2 0x559fbeb27062 in zmalloc<(anonymous namespace)::lttng_rate_policy_every_n> ../../src/common/macros.hpp:89 #3 0x559fbeb25e9f in lttng_rate_policy_every_n_create actions/rate-policy.cpp:492 #4 0x559fbeb168b9 in lttng_action_notify_create actions/notify.cpp:141 #5 0x559fbe9dc34b in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:80 #6 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993 #7 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246 #8 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624 #9 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68 #10 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1) Indirect leak of 34 byte(s) in 2 object(s) allocated from: #0 0x7fe0f4e19319 in __interceptor_strdup /usr/src/debug/gcc/libsanitizer/asan/asan_interceptors.cpp:454 #1 0x559fbeb3f603 in lttng_condition_session_consumed_size_set_session_name conditions/session-consumed-size.cpp:442 #2 0x559fbe9dc2c4 in subscribe_session_consumed_size_rotation(ltt_session*, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/rotate.cpp:71 #3 0x559fbe995d6f in cmd_rotation_set_schedule(ltt_session*, bool, lttng_rotation_schedule_type, unsigned long, notification_thread_handle*) /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/cmd.cpp:5993 #4 0x559fbe9fe559 in process_client_msg /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2246 #5 0x559fbea01378 in thread_manage_clients /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/client.cpp:2624 #6 0x559fbe9ea642 in launch_thread /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng-sessiond/thread.cpp:68 #7 0x7fe0f44935c1 in start_thread (/usr/lib/libc.so.6+0x8d5c1) The rotation trigger of a session (used for size-based rotations) is never cleaned-up. It is now cleaned up every time its condition is hit and whenever the session is destroyed. Signed-off-by: Jérémie Galarneau Change-Id: I5a89341535f87b7851b548ded9838c18bd1ccb95 --- .../notification/notification-internal.h | 8 +++++ src/bin/lttng-sessiond/rotate.c | 35 +++++++++++++------ src/bin/lttng-sessiond/rotation-thread.c | 22 +++++++----- src/bin/lttng-sessiond/session.c | 1 + src/bin/lttng-sessiond/session.h | 3 +- src/common/notification.c | 18 ++++++++++ 6 files changed, 66 insertions(+), 21 deletions(-) diff --git a/include/lttng/notification/notification-internal.h b/include/lttng/notification/notification-internal.h index dc3a0e1d6..99a545642 100644 --- a/include/lttng/notification/notification-internal.h +++ b/include/lttng/notification/notification-internal.h @@ -43,4 +43,12 @@ ssize_t lttng_notification_create_from_payload( struct lttng_payload_view *view, struct lttng_notification **notification); +const struct lttng_condition *lttng_notification_get_const_condition( + const struct lttng_notification *notification); +const struct lttng_evaluation *lttng_notification_get_const_evaluation( + const struct lttng_notification *notification); + +const struct lttng_trigger *lttng_notification_get_const_trigger( + const struct lttng_notification *notification); + #endif /* LTTNG_NOTIFICATION_INTERNAL_H */ diff --git a/src/bin/lttng-sessiond/rotate.c b/src/bin/lttng-sessiond/rotate.c index cdf95f353..9e1126bdf 100644 --- a/src/bin/lttng-sessiond/rotate.c +++ b/src/bin/lttng-sessiond/rotate.c @@ -25,6 +25,8 @@ #include #include +#include +#include #include "session.h" #include "rotate.h" @@ -45,21 +47,22 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64 int ret; enum lttng_condition_status condition_status; enum lttng_notification_channel_status nc_status; - struct lttng_action *action; + struct lttng_condition *rotate_condition = NULL; + struct lttng_action *notify_action = NULL; const struct lttng_credentials session_creds = { .uid = LTTNG_OPTIONAL_INIT_VALUE(session->uid), .gid = LTTNG_OPTIONAL_INIT_VALUE(session->gid), }; - session->rotate_condition = lttng_condition_session_consumed_size_create(); - if (!session->rotate_condition) { + rotate_condition = lttng_condition_session_consumed_size_create(); + if (!rotate_condition) { ERR("Failed to create session consumed size condition object"); ret = -1; goto end; } condition_status = lttng_condition_session_consumed_size_set_threshold( - session->rotate_condition, size); + rotate_condition, size); if (condition_status != LTTNG_CONDITION_STATUS_OK) { ERR("Could not set session consumed size condition threshold (size = %" PRIu64 ")", size); @@ -69,7 +72,7 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64 condition_status = lttng_condition_session_consumed_size_set_session_name( - session->rotate_condition, session->name); + rotate_condition, session->name); if (condition_status != LTTNG_CONDITION_STATUS_OK) { ERR("Could not set session consumed size condition session name (name = %s)", session->name); @@ -77,15 +80,16 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64 goto end; } - action = lttng_action_notify_create(); - if (!action) { + notify_action = lttng_action_notify_create(); + if (!notify_action) { ERR("Could not create notify action"); ret = -1; goto end; } - session->rotate_trigger = lttng_trigger_create(session->rotate_condition, - action); + assert(!session->rotate_trigger); + session->rotate_trigger = lttng_trigger_create(rotate_condition, + notify_action); if (!session->rotate_trigger) { ERR("Could not create size-based rotation trigger"); ret = -1; @@ -98,7 +102,7 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64 session->rotate_trigger, &session_creds); nc_status = lttng_notification_channel_subscribe( - rotate_notification_channel, session->rotate_condition); + rotate_notification_channel, rotate_condition); if (nc_status != LTTNG_NOTIFICATION_CHANNEL_STATUS_OK) { ERR("Could not subscribe to session consumed size notification"); ret = -1; @@ -117,6 +121,11 @@ int subscribe_session_consumed_size_rotation(struct ltt_session *session, uint64 ret = 0; end: + lttng_condition_put(rotate_condition); + lttng_action_put(notify_action); + if (ret) { + lttng_trigger_put(session->rotate_trigger); + } return ret; } @@ -126,9 +135,10 @@ int unsubscribe_session_consumed_size_rotation(struct ltt_session *session, int ret = 0; enum lttng_notification_channel_status status; + assert(session->rotate_trigger); status = lttng_notification_channel_unsubscribe( rotate_notification_channel, - session->rotate_condition); + lttng_trigger_get_const_condition(session->rotate_trigger)); if (status != LTTNG_NOTIFICATION_CHANNEL_STATUS_OK) { ERR("Session unsubscribe error: %d", (int) status); ret = -1; @@ -142,6 +152,9 @@ int unsubscribe_session_consumed_size_rotation(struct ltt_session *session, goto end; } + lttng_trigger_put(session->rotate_trigger); + session->rotate_trigger = NULL; + ret = 0; end: return ret; diff --git a/src/bin/lttng-sessiond/rotation-thread.c b/src/bin/lttng-sessiond/rotation-thread.c index 590b95dd2..e4811bd44 100644 --- a/src/bin/lttng-sessiond/rotation-thread.c +++ b/src/bin/lttng-sessiond/rotation-thread.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "rotation-thread.h" #include "lttng-sessiond.h" @@ -619,8 +620,7 @@ end: } static -int handle_condition(const struct lttng_condition *condition, - const struct lttng_evaluation *evaluation, +int handle_condition(const struct lttng_notification *notification, struct notification_thread_handle *notification_thread_handle) { int ret = 0; @@ -630,6 +630,10 @@ int handle_condition(const struct lttng_condition *condition, enum lttng_evaluation_status evaluation_status; uint64_t consumed; struct ltt_session *session; + const struct lttng_condition *condition = + lttng_notification_get_const_condition(notification); + const struct lttng_evaluation *evaluation = + lttng_notification_get_const_evaluation(notification); condition_type = lttng_condition_get_type(condition); @@ -671,6 +675,13 @@ int handle_condition(const struct lttng_condition *condition, } session_lock(session); + if (!lttng_trigger_is_equal(session->rotate_trigger, + lttng_notification_get_const_trigger(notification))) { + /* Notification does not originate from our rotation trigger. */ + ret = 0; + goto end_unlock; + } + ret = unsubscribe_session_consumed_size_rotation(session, notification_thread_handle); if (ret) { @@ -713,8 +724,6 @@ int handle_notification_channel(int fd, bool notification_pending; struct lttng_notification *notification = NULL; enum lttng_notification_channel_status status; - const struct lttng_evaluation *notification_evaluation; - const struct lttng_condition *notification_condition; status = lttng_notification_channel_has_pending_notification( rotate_notification_channel, ¬ification_pending); @@ -752,10 +761,7 @@ int handle_notification_channel(int fd, goto end; } - notification_condition = lttng_notification_get_condition(notification); - notification_evaluation = lttng_notification_get_evaluation(notification); - - ret = handle_condition(notification_condition, notification_evaluation, + ret = handle_condition(notification, handle->notification_thread_handle); end: diff --git a/src/bin/lttng-sessiond/session.c b/src/bin/lttng-sessiond/session.c index 6f52bcf4f..635192f2b 100644 --- a/src/bin/lttng-sessiond/session.c +++ b/src/bin/lttng-sessiond/session.c @@ -1032,6 +1032,7 @@ void session_release(struct urcu_ref *ref) lttng_dynamic_array_reset(&session->clear_notifiers); free(session->last_archived_chunk_name); free(session->base_path); + lttng_trigger_put(session->rotate_trigger); free(session); if (session_published) { /* diff --git a/src/bin/lttng-sessiond/session.h b/src/bin/lttng-sessiond/session.h index 1a68d8e2f..7b6e620ff 100644 --- a/src/bin/lttng-sessiond/session.h +++ b/src/bin/lttng-sessiond/session.h @@ -181,9 +181,8 @@ struct ltt_session { */ bool rotated; /* - * Condition and trigger for size-based rotations. + * Trigger for size-based rotations. */ - struct lttng_condition *rotate_condition; struct lttng_trigger *rotate_trigger; LTTNG_OPTIONAL(uint64_t) most_recent_chunk_id; struct lttng_trace_chunk *current_trace_chunk; diff --git a/src/common/notification.c b/src/common/notification.c index be09c1fcc..a5d887007 100644 --- a/src/common/notification.c +++ b/src/common/notification.c @@ -179,6 +179,24 @@ const struct lttng_evaluation *lttng_notification_get_evaluation( return notification ? notification->evaluation : NULL; } +const struct lttng_condition *lttng_notification_get_const_condition( + const struct lttng_notification *notification) +{ + return notification ? lttng_trigger_get_const_condition(notification->trigger) : NULL; +} + +const struct lttng_evaluation *lttng_notification_get_const_evaluation( + const struct lttng_notification *notification) +{ + return notification ? notification->evaluation : NULL; +} + +const struct lttng_trigger *lttng_notification_get_const_trigger( + const struct lttng_notification *notification) +{ + return notification ? notification->trigger : NULL; +} + const struct lttng_trigger *lttng_notification_get_trigger( struct lttng_notification *notification) { -- 2.34.1