Fix: lttng-ctl: assertion failure during unregistration of trigger

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 <jeremie.galarneau@efficios.com>
Change-Id: I71b7b075c959bc4935621543c4d379f62b7dabdf

diff --git a/include/lttng/trigger/trigger-internal.h b/include/lttng/trigger/trigger-internal.h
index 5bab4c7634c9a096a0819085cd49ccef063fdd8d..6fba6668491d71dd95f9f706a818e7e915c76d1e 100644 (file)
--- 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 34d4208076a22eec5d9e6d003ec880e887f08d40..a86295069a85c12d1b17ee8bbaa3ade4643fa29c 100644 (file)
--- 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(&copy_buffer);
 
-       ret = lttng_trigger_serialize(trigger, &copy_buffer);
+       ret = lttng_condition_serialize(trigger->condition, &copy_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(
                                                &copy_buffer, 0, -1);
-               ret = lttng_trigger_create_from_payload(
-                               &view, &copy);
+
+               ret = lttng_condition_create_from_payload(
+                               &view, &condition_copy);
+               if (ret < 0) {
+                       goto end;
+               }
+       }
+
+       lttng_payload_clear(&copy_buffer);
+
+       ret = lttng_action_serialize(trigger->action, &copy_buffer);
+       if (ret < 0) {
+               goto end;
+       }
+
+       {
+               struct lttng_payload_view view =
+                               lttng_payload_view_from_payload(
+                                               &copy_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(&copy->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(&copy_buffer);
        return copy;
 }