From 94dbd8e4ed88cd56829159e1fef374a16fddd593 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Tue, 20 Apr 2021 12:48:05 -0400 Subject: [PATCH] Fix: lttng-ctl: assertion failure during unregistration of trigger MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Issue observed ============== lt-trigger_name: trigger.c:302: int lttng_trigger_serialize(const struct lttng_trigger *, struct lttng_payload *): Assertion `(creds->uid).is_set' failed. Program terminated with signal SIGABRT, Aborted. #0 0x00007fb74129eef5 in raise () from /usr/lib/libc.so.6 #1 0x00007fb741288862 in abort () from /usr/lib/libc.so.6 #2 0x00007fb741288747 in __assert_fail_base.cold () from /usr/lib/libc.so.6 #3 0x00007fb741297646 in __assert_fail () from /usr/lib/libc.so.6 #4 0x00007fb74169bab7 in lttng_trigger_serialize (trigger=0x5616f6f70060, payload=0x7ffe5819d140) at trigger.c:302 #5 0x00007fb74169cef0 in lttng_trigger_copy (trigger=0x5616f6f70060) at trigger.c:859 #6 0x00007fb74164302e in lttng_unregister_trigger (trigger=0x5616f6f70060) at lttng-ctl.c:3350 #7 0x00005616f50c675f in register_named_trigger () at trigger_name.c:295 #8 0x00005616f50c6879 in main (argc=1, argv=0x7ffe581a07d8) at trigger_name.c:343 Cause ===== When creating a trigger instance and using it to unregister an existing trigger, its credentials are unset (meaning 'default'). Expecting this, lttng_unregister_trigger() copies the source trigger to change its credentials to those of the caller. Unfortunately, the trigger copy operation expects credentials to be set. We don't run into this situation typically since the trigger instance used to perform the unregistration is sourced from a listing or is the same instance that was used to perform the registration (which sets the credentials before serializing). Solution ======== A proper implementation of "copy" is provided for the trigger object itself. For its condition and action, we still use the same "trick" of leveraging the serdes code to perform a deep-copy, keeping the change small Drawbacks ========= None really, except that we lose some of the code sharing between copy and serdes. Signed-off-by: Jérémie Galarneau Change-Id: I71b7b075c959bc4935621543c4d379f62b7dabdf --- include/lttng/trigger/trigger-internal.h | 6 ++ src/common/trigger.c | 76 ++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/include/lttng/trigger/trigger-internal.h b/include/lttng/trigger/trigger-internal.h index 5bab4c763..6fba66684 100644 --- a/include/lttng/trigger/trigger-internal.h +++ b/include/lttng/trigger/trigger-internal.h @@ -190,6 +190,12 @@ enum lttng_error_code lttng_trigger_generate_bytecode( struct lttng_trigger *trigger, const struct lttng_credentials *creds); +/* + * Note that the trigger object is not locked by "copy" as it is const and + * used with a number of 'const' triggers. If the trigger could be shared at + * the moment of the copy, it is the caller's responsability to lock it for + * the duration of the copy. + */ LTTNG_HIDDEN struct lttng_trigger *lttng_trigger_copy(const struct lttng_trigger *trigger); diff --git a/src/common/trigger.c b/src/common/trigger.c index 34d420807..a86295069 100644 --- a/src/common/trigger.c +++ b/src/common/trigger.c @@ -851,11 +851,16 @@ struct lttng_trigger *lttng_trigger_copy(const struct lttng_trigger *trigger) { int ret; struct lttng_payload copy_buffer; + struct lttng_condition *condition_copy = NULL; + struct lttng_action *action_copy = NULL; struct lttng_trigger *copy = NULL; + enum lttng_trigger_status trigger_status; + const char *trigger_name; + uid_t trigger_owner_uid; lttng_payload_init(©_buffer); - ret = lttng_trigger_serialize(trigger, ©_buffer); + ret = lttng_condition_serialize(trigger->condition, ©_buffer); if (ret < 0) { goto end; } @@ -864,15 +869,78 @@ struct lttng_trigger *lttng_trigger_copy(const struct lttng_trigger *trigger) struct lttng_payload_view view = lttng_payload_view_from_payload( ©_buffer, 0, -1); - ret = lttng_trigger_create_from_payload( - &view, ©); + + ret = lttng_condition_create_from_payload( + &view, &condition_copy); + if (ret < 0) { + goto end; + } + } + + lttng_payload_clear(©_buffer); + + ret = lttng_action_serialize(trigger->action, ©_buffer); + if (ret < 0) { + goto end; + } + + { + struct lttng_payload_view view = + lttng_payload_view_from_payload( + ©_buffer, 0, -1); + + ret = lttng_action_create_from_payload( + &view, &action_copy); if (ret < 0) { - copy = NULL; goto end; } } + copy = lttng_trigger_create(condition_copy, action_copy); + if (!copy) { + ERR("Failed to allocate trigger during trigger copy"); + goto end; + } + + trigger_status = lttng_trigger_get_name(trigger, &trigger_name); + switch (trigger_status) { + case LTTNG_TRIGGER_STATUS_OK: + trigger_status = lttng_trigger_set_name(copy, trigger_name); + if (trigger_status != LTTNG_TRIGGER_STATUS_OK) { + ERR("Failed to set name of new trigger during copy"); + goto error_cleanup_trigger; + } + break; + case LTTNG_TRIGGER_STATUS_UNSET: + break; + default: + ERR("Failed to get name of original trigger during copy"); + goto error_cleanup_trigger; + } + + trigger_status = lttng_trigger_get_owner_uid( + trigger, &trigger_owner_uid); + switch (trigger_status) { + case LTTNG_TRIGGER_STATUS_OK: + LTTNG_OPTIONAL_SET(©->creds.uid, trigger_owner_uid); + break; + case LTTNG_TRIGGER_STATUS_UNSET: + break; + default: + ERR("Failed to get owner uid of original trigger during copy"); + goto error_cleanup_trigger; + } + + copy->tracer_token = trigger->tracer_token; + copy->registered = trigger->registered; + goto end; + +error_cleanup_trigger: + lttng_trigger_destroy(copy); + copy = NULL; end: + lttng_condition_put(condition_copy); + lttng_action_put(action_copy); lttng_payload_reset(©_buffer); return copy; } -- 2.34.1