Fix: flt.utils.trimmer: bt_message_put_ref() on freed message
authorFrancis Deslauriers <francis.deslauriers@efficios.com>
Thu, 16 May 2019 00:14:42 +0000 (20:14 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 16 May 2019 15:53:01 +0000 (11:53 -0400)
Issue
=====
In this function, we convert a timestamp in second to a clock value in
order to create a stream activity beginning message. We create a new
message and proceed to call `clock_raw_value_from_ns_from_origin()` to
convert the timestamp to a clock value.

If this call fails, we put the reference on this message using
`bt_message_put_ref()`. This frees the message as the refcount was at 1
because it was just created.

The problem here is that we don't reset the `msg` pointer and return it
to the call via the output parameter. This pointer points to freed data.
The caller then figures that the function failed by looking at the
return value and proceed to call `bt_message_put_ref()` again resulting
and assertion failure.

This can be easily triggered by passing a `begin` parameter of value 0
to a trimmer component.

Solution
========
Use a local variable to keep the pointer to the newly created message
and move that reference to the output parameter only on success.

Drawbacks
=========
None.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Id589f25d9ec6f555abd44ccb71e72af8e1684272
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1314
Tested-by: jenkins
Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
plugins/utils/trimmer/trimmer.c

index d9407bfda205e1341f8d3ed4186eab2bf0c30d7f..8e114e1723d162aa3906f639708322803fd85e61 100644 (file)
@@ -1116,15 +1116,16 @@ bt_self_message_iterator_status create_stream_beginning_activity_message(
                const bt_stream *stream,
                const bt_clock_class *clock_class, bt_message **msg)
 {
+       bt_message *local_msg;
        bt_self_message_iterator_status status =
                BT_SELF_MESSAGE_ITERATOR_STATUS_OK;
 
        BT_ASSERT(msg);
        BT_ASSERT(!trimmer_it->begin.is_infinite);
 
-       *msg = bt_message_stream_activity_beginning_create(
+       local_msg = bt_message_stream_activity_beginning_create(
                trimmer_it->self_msg_iter, stream);
-       if (!*msg) {
+       if (!local_msg) {
                status = BT_SELF_MESSAGE_ITERATOR_STATUS_NOMEM;
                goto end;
        }
@@ -1137,14 +1138,16 @@ bt_self_message_iterator_status create_stream_beginning_activity_message(
                        trimmer_it->begin.ns_from_origin, &raw_value);
                if (ret) {
                        status = BT_SELF_MESSAGE_ITERATOR_STATUS_ERROR;
-                       bt_message_put_ref(*msg);
+                       bt_message_put_ref(local_msg);
                        goto end;
                }
 
                bt_message_stream_activity_beginning_set_default_clock_snapshot(
-                       *msg, raw_value);
+                       local_msg, raw_value);
        }
 
+       BT_MESSAGE_MOVE_REF(*msg, local_msg);
+
 end:
        return status;
 }
This page took 0.072913 seconds and 4 git commands to generate.