Fix: lttng-ctl: appending to dynamic buffer invalidates its data member

Issue
=====
The following commands fail:
  lttng add-trigger --id T0 --condition on-event -u some-event --action snapshot-session ze-session3 --path /some/path
  lttng remove-trigger T0

  Error: Attempt to create buffer view from another view with invalid length (length > space left after offset in source): source size = 0, offset in source = 0, length = 25
  Error: Invalid trigger received as part of command payload

Valgrind complains in the following way:

==706109==
==706109== Invalid write of size 4
==706109==    at 0x489FED7: lttng_unregister_trigger (lttng-ctl.c:3281)
==706109==    by 0x43C175: cmd_remove_trigger (remove_trigger.c:171)
==706109==    by 0x43F56B: handle_command (lttng.c:237)
==706109==    by 0x43E9B1: parse_args (lttng.c:421)
==706109==    by 0x43E158: main (lttng.c:470)
==706109==  Address 0x73d8d20 is 4,688 bytes inside a block of size 16,384 free'd
==706109==    at 0x483DFAF: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==706109==    by 0x48C1478: lttng_dynamic_buffer_set_capacity (dynamic-buffer.c:166)
==706109==    by 0x48C138C: lttng_dynamic_buffer_append (dynamic-buffer.c:55)
==706109==    by 0x48E3325: lttng_snapshot_output_serialize (snapshot.c:120)
==706109==    by 0x48B46C3: lttng_action_snapshot_session_serialize (snapshot-session.c:173)
==706109==    by 0x48B1FB2: lttng_action_serialize (action.c:130)
==706109==    by 0x48B2DFE: lttng_action_group_serialize (group.c:165)
==706109==    by 0x48B1FB2: lttng_action_serialize (action.c:130)
==706109==    by 0x48ECE66: lttng_trigger_serialize (trigger.c:372)
==706109==    by 0x489FEA0: lttng_unregister_trigger (lttng-ctl.c:3275)
==706109==    by 0x43C175: cmd_remove_trigger (remove_trigger.c:171)
==706109==    by 0x43F56B: handle_command (lttng.c:237)
==706109==  Block was alloc'd at
==706109==    at 0x483B723: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==706109==    by 0x483E017: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==706109==    by 0x48C1478: lttng_dynamic_buffer_set_capacity (dynamic-buffer.c:166)
==706109==    by 0x48C138C: lttng_dynamic_buffer_append (dynamic-buffer.c:55)
==706109==    by 0x489FE66: lttng_unregister_trigger (lttng-ctl.c:3263)
==706109==    by 0x43C175: cmd_remove_trigger (remove_trigger.c:171)
==706109==    by 0x43F56B: handle_command (lttng.c:237)
==706109==    by 0x43E9B1: parse_args (lttng.c:421)
==706109==    by 0x43E158: main (lttng.c:470)

`lttng_unregister_trigger` samples the address of the lsm header in the
message payload. However, it does so before calling
`lttng_trigger_serialize()` which may increase the underlying buffer's
size (and cause a realloc()).

Most of the time the message buffer is large enough _or_ its realloc
yields the same address which hid the problem. However, I stumbled on a
case (a trigger which snapshots to a location) where the realloc ends-up
returning a completely different address, causing invalid data to be
sent to the session daemon.

Solution
========
Sample the lsm header address after the serialization of the trigger.

Note
====
An identical fix was done for the `lttng_register_trigger` function in:
  commit b22f4f54e95ae13edda1d4d5efd1e4845a6319c4
  Author: Jérémie Galarneau <jeremie.galarneau@efficios.com>
  Date:   Thu Feb 18 18:13:19 2021 -0500

      Fix: lttng-ctl: appending to dynamic buffer invalidates its data member

I reuse the bug explanation for this commit message.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ic50c96dcada9e0595b0fab1d2f357c183b53e1de

diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
index 7debaed6b813d412404db52c277622cbd1be7154..7efc6bd2ebea7ef007c14a164b43cee48ecc3519 100644 (file)
--- a/src/lib/lttng-ctl/lttng-ctl.c
+++ b/src/lib/lttng-ctl/lttng-ctl.c
@@ -3344,18 +3344,18 @@ int lttng_unregister_trigger(const struct lttng_trigger *trigger)
                goto end;
        }
 
-       /*
-        * This is needed to populate the trigger object size for the command
-        * header and number of fds sent.
-       */
-       message_lsm = (struct lttcomm_session_msg *) message.buffer.data;
-
        ret = lttng_trigger_serialize(copy, &message);
        if (ret < 0) {
                ret = -LTTNG_ERR_UNK;
                goto end;
        }
 
+       /*
+        * This is needed to populate the trigger object size for the command
+        * header and number of fds sent.
+       */
+       message_lsm = (struct lttcomm_session_msg *) message.buffer.data;
+
        message_lsm->u.trigger.length = (uint32_t) message.buffer.size - sizeof(lsm);
 
        {