From 9b7cbebd7d78c0950c84cd834ce15d3298d997e6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Thu, 4 Feb 2021 18:18:32 -0500 Subject: [PATCH] Fix: sessiond: acquire session list lock when updating event notifiers MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Registering triggers with an on-event hit condition affects event notifiers, imposing a synchronization of enablers with the user space tracers. As noted in the comments of session.h, the session list lock protects those updates and is, ultimately, ill-named. The comment is adjusted to mention "tracer configurations" rather than "session configurations" since event notifiers are not part of a session, making the comment imprecise. Signed-off-by: Jérémie Galarneau Change-Id: Id1bcbcccbdeeafa91176ed3413ddddbcbab10ad2 --- src/bin/lttng-sessiond/cmd.c | 33 +++++++++++++++++++++----------- src/bin/lttng-sessiond/session.h | 2 +- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c index d8b063753..505645bf1 100644 --- a/src/bin/lttng-sessiond/cmd.c +++ b/src/bin/lttng-sessiond/cmd.c @@ -4317,7 +4317,7 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c struct lttng_trigger **return_trigger) { enum lttng_error_code ret_code; - bool must_update_event_notifier; + bool must_update_event_notifiers; const char *trigger_name; uid_t trigger_owner; enum lttng_trigger_status trigger_status; @@ -4384,7 +4384,7 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c trigger_name = trigger_status == LTTNG_TRIGGER_STATUS_OK ? trigger_name : "(unnamed)"; - ret_code = trigger_modifies_event_notifier(trigger, &must_update_event_notifier); + ret_code = trigger_modifies_event_notifier(trigger, &must_update_event_notifiers); if (ret_code != LTTNG_OK) { ERR("Failed to determine if event modifies event notifiers: trigger name = '%s', trigger owner uid = %d, error code = %d", trigger_name, (int) trigger_owner, ret_code); @@ -4394,10 +4394,11 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c /* * Synchronize tracers if the trigger adds an event notifier. */ - if (must_update_event_notifier) { + if (must_update_event_notifiers) { const enum lttng_domain_type trigger_domain = lttng_trigger_get_underlying_domain_type_restriction(trigger); + session_lock_list(); switch (trigger_domain) { case LTTNG_DOMAIN_KERNEL: { @@ -4434,19 +4435,21 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c agt = agent_create(trigger_domain); if (!agt) { ret_code = LTTNG_ERR_NOMEM; - goto end; + goto end_unlock_session_list; } agent_add(agt, trigger_agents_ht_by_domain); } ret_code = trigger_agent_enable(trigger, agt); if (ret_code != LTTNG_OK) { - goto end; + goto end_unlock_session_list; } break; } } + + session_unlock_list(); } /* @@ -4462,6 +4465,9 @@ enum lttng_error_code cmd_register_trigger(const struct lttng_credentials *cmd_c trigger = NULL; end: return ret_code; +end_unlock_session_list: + session_unlock_list(); + return ret_code; } enum lttng_error_code cmd_unregister_trigger(const struct lttng_credentials *cmd_creds, @@ -4469,7 +4475,7 @@ enum lttng_error_code cmd_unregister_trigger(const struct lttng_credentials *cmd struct notification_thread_handle *notification_thread) { enum lttng_error_code ret_code; - bool must_update_event_notifier; + bool must_update_event_notifiers; const char *trigger_name; uid_t trigger_owner; enum lttng_trigger_status trigger_status; @@ -4501,7 +4507,7 @@ enum lttng_error_code cmd_unregister_trigger(const struct lttng_credentials *cmd } } - ret_code = trigger_modifies_event_notifier(trigger, &must_update_event_notifier); + ret_code = trigger_modifies_event_notifier(trigger, &must_update_event_notifiers); if (ret_code != LTTNG_OK) { ERR("Failed to determine if event modifies event notifiers: trigger name = '%s', trigger owner uid = %d, error code = %d", trigger_name, (int) trigger_owner, ret_code); @@ -4521,11 +4527,12 @@ enum lttng_error_code cmd_unregister_trigger(const struct lttng_credentials *cmd * the tracers from producing notifications associated with this * event notifier. */ - if (must_update_event_notifier) { + if (must_update_event_notifiers) { const enum lttng_domain_type trigger_domain = lttng_trigger_get_underlying_domain_type_restriction( trigger); + session_lock_list(); switch (trigger_domain) { case LTTNG_DOMAIN_KERNEL: { @@ -4548,24 +4555,28 @@ enum lttng_error_code cmd_unregister_trigger(const struct lttng_credentials *cmd agt = agent_create(trigger_domain); if (!agt) { ret_code = LTTNG_ERR_NOMEM; - goto end; + goto end_unlock_session_list; } agent_add(agt, trigger_agents_ht_by_domain); } ret_code = trigger_agent_disable(trigger, agt); if (ret_code != LTTNG_OK) { - goto end; + goto end_unlock_session_list; } break; } } + + session_unlock_list(); } end: return ret_code; -} +end_unlock_session_list: + session_unlock_list(); + return ret_code;} int cmd_list_triggers(struct command_ctx *cmd_ctx, struct notification_thread_handle *notification_thread, diff --git a/src/bin/lttng-sessiond/session.h b/src/bin/lttng-sessiond/session.h index 9b790c1fe..5ff20ad41 100644 --- a/src/bin/lttng-sessiond/session.h +++ b/src/bin/lttng-sessiond/session.h @@ -206,7 +206,7 @@ void session_unlock(struct ltt_session *session); * also used as a multi-session lock when synchronizing newly-registered * 'user space tracer' and 'agent' applications. * - * In other words, it prevents session configurations from changing while they + * In other words, it prevents tracer configurations from changing while they * are being transmitted to the various applications. */ void session_lock_list(void); -- 2.34.1