From 75a151671b0e3d2a90eeff2ccd1ab32d4a1729de Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Wed, 15 May 2019 20:14:42 -0400 Subject: [PATCH] Fix: flt.utils.trimmer: bt_message_put_ref() on freed message MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Change-Id: Id589f25d9ec6f555abd44ccb71e72af8e1684272 Reviewed-on: https://review.lttng.org/c/babeltrace/+/1314 Tested-by: jenkins Reviewed-by: Jérémie Galarneau --- plugins/utils/trimmer/trimmer.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/plugins/utils/trimmer/trimmer.c b/plugins/utils/trimmer/trimmer.c index d9407bfd..8e114e17 100644 --- a/plugins/utils/trimmer/trimmer.c +++ b/plugins/utils/trimmer/trimmer.c @@ -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; } -- 2.34.1