From patchwork Thu Dec 5 06:58:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Desnoyers X-Patchwork-Id: 3552093 From: mathieu.desnoyers at efficios.com (Mathieu Desnoyers) Date: Thu, 5 Dec 2019 01:58:04 -0500 Subject: [lttng-dev] [PATCH babeltrace-1.5 1/6] Fix: lttng-live: use-after-free in get_next_index() In-Reply-To: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> References: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> Message-ID: <20191205065809.16728-2-mathieu.desnoyers@efficios.com> Running babeltrace under valgrind with a test-cases doing per-pid lttng tracing in live mode triggers this use-after-free in get_next_index() when stream is hung up. Signed-off-by: Mathieu Desnoyers --- formats/lttng-live/lttng-live-comm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/formats/lttng-live/lttng-live-comm.c b/formats/lttng-live/lttng-live-comm.c index 33a78029..96817f5e 100644 --- a/formats/lttng-live/lttng-live-comm.c +++ b/formats/lttng-live/lttng-live-comm.c @@ -1108,8 +1108,8 @@ retry: viewer_stream->in_trace = 0; bt_list_del(&viewer_stream->trace_stream_node); bt_list_del(&viewer_stream->session_stream_node); - g_free(viewer_stream); *stream_id = be64toh(rp->stream_id); + g_free(viewer_stream); break; case LTTNG_VIEWER_INDEX_ERR: fprintf(stderr, "[error] get_next_index: error\n"); From patchwork Thu Dec 5 06:58:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Desnoyers X-Patchwork-Id: 3552094 From: mathieu.desnoyers at efficios.com (Mathieu Desnoyers) Date: Thu, 5 Dec 2019 01:58:05 -0500 Subject: [lttng-dev] [PATCH babeltrace-1.5 2/6] Fix: trace-collection: trace clock use after free In-Reply-To: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> References: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> Message-ID: <20191205065809.16728-3-mathieu.desnoyers@efficios.com> The trace collection should copy the trace clock object rather than take a reference to the first trace's trace clock, because it may be freed when the trace is removed (e.g. application going away in per-pid live tracing). Signed-off-by: Mathieu Desnoyers --- lib/trace-collection.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/trace-collection.c b/lib/trace-collection.c index 035d2dc2..8e4a1432 100644 --- a/lib/trace-collection.c +++ b/lib/trace-collection.c @@ -76,7 +76,7 @@ static void clock_add(gpointer key, gpointer value, gpointer user_data) { struct clock_match *clock_match = user_data; GHashTable *tc_clocks = clock_match->clocks; - struct ctf_clock *t_clock = value; + struct ctf_clock *t_clock = value, *clock_copy; GQuark v; if (t_clock->absolute) @@ -104,9 +104,14 @@ static void clock_add(gpointer key, gpointer value, gpointer user_data) clock_match->tc->single_clock_offset_avg = clock_match->tc->offset_first; } + clock_copy = g_new0(struct ctf_clock, 1); + *clock_copy = *t_clock; + if (t_clock->description) { + clock_copy->description = g_strdup(t_clock->description); + } g_hash_table_insert(tc_clocks, (gpointer) (unsigned long) v, - value); + clock_copy); } else if (!t_clock->absolute) { int64_t diff_ns; @@ -209,11 +214,21 @@ int bt_trace_collection_remove(struct trace_collection *tc, } +static +void clock_free(gpointer data) +{ + struct ctf_clock *clock = data; + + g_free(clock->description); + g_free(clock); +} + void bt_init_trace_collection(struct trace_collection *tc) { assert(tc); tc->array = g_ptr_array_new(); - tc->clocks = g_hash_table_new(g_direct_hash, g_direct_equal); + tc->clocks = g_hash_table_new_full(g_direct_hash, g_direct_equal, + NULL, clock_free); tc->single_clock_offset_avg = 0; tc->offset_first = 0; tc->delta_offset_first_sum = 0; From patchwork Thu Dec 5 06:58:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Desnoyers X-Patchwork-Id: 3552091 From: mathieu.desnoyers at efficios.com (Mathieu Desnoyers) Date: Thu, 5 Dec 2019 01:58:06 -0500 Subject: [lttng-dev] [PATCH babeltrace-1.5 3/6] Fix: lttng-live: lttng_live_open_trace_read memory leak In-Reply-To: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> References: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> Message-ID: <20191205065809.16728-4-mathieu.desnoyers@efficios.com> Signed-off-by: Mathieu Desnoyers --- formats/lttng-live/lttng-live-plugin.c | 1 + 1 file changed, 1 insertion(+) diff --git a/formats/lttng-live/lttng-live-plugin.c b/formats/lttng-live/lttng-live-plugin.c index ed52a995..0bebdd89 100644 --- a/formats/lttng-live/lttng-live-plugin.c +++ b/formats/lttng-live/lttng-live-plugin.c @@ -288,6 +288,7 @@ static int lttng_live_open_trace_read(const char *path) } end_free: + g_array_free(ctx->session_ids, TRUE); g_hash_table_destroy(ctx->session->ctf_traces); free_session_streams(ctx->session); g_free(ctx->session); From patchwork Thu Dec 5 06:58:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Desnoyers X-Patchwork-Id: 3552095 From: mathieu.desnoyers at efficios.com (Mathieu Desnoyers) Date: Thu, 5 Dec 2019 01:58:07 -0500 Subject: [lttng-dev] [PATCH babeltrace-1.5 4/6] Fix: lib/iterator.c: unbalanced ctx put (leak) In-Reply-To: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> References: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> Message-ID: <20191205065809.16728-5-mathieu.desnoyers@efficios.com> Missing context put in iterator init error path. Signed-off-by: Mathieu Desnoyers --- lib/iterator.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/iterator.c b/lib/iterator.c index 639a2d29..77093217 100644 --- a/lib/iterator.c +++ b/lib/iterator.c @@ -778,6 +778,8 @@ int bt_iter_init(struct bt_iter *iter, error: bt_heap_free(iter->stream_heap); error_heap_init: + bt_context_put(ctx); + iter->ctx = NULL; g_free(iter->stream_heap); iter->stream_heap = NULL; error_ctx: @@ -812,6 +814,7 @@ void bt_iter_fini(struct bt_iter *iter) } iter->ctx->current_iterator = NULL; bt_context_put(iter->ctx); + iter->ctx = NULL; } void bt_iter_destroy(struct bt_iter *iter) From patchwork Thu Dec 5 06:58:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Desnoyers X-Patchwork-Id: 3552096 From: mathieu.desnoyers at efficios.com (Mathieu Desnoyers) Date: Thu, 5 Dec 2019 01:58:08 -0500 Subject: [lttng-dev] [PATCH babeltrace-1.5 5/6] Fix: lttng-live: ctf_live_packet_seek stream hang up handling In-Reply-To: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> References: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> Message-ID: <20191205065809.16728-6-mathieu.desnoyers@efficios.com> When get_next_index sets the index position to EOF, ctf_live_packet_seek() should in turn set the stream position to EOF to propagate the hung up state. Signed-off-by: Mathieu Desnoyers --- formats/lttng-live/lttng-live-comm.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/formats/lttng-live/lttng-live-comm.c b/formats/lttng-live/lttng-live-comm.c index 96817f5e..484c025d 100644 --- a/formats/lttng-live/lttng-live-comm.c +++ b/formats/lttng-live/lttng-live-comm.c @@ -1276,7 +1276,11 @@ retry: cur_index->packet_size, cur_index->offset, cur_index->content_size, cur_index->ts_cycles.timestamp_end); - + if (cur_index->offset == EOF) { + pos->offset = EOF; + ret = -BT_PACKET_SEEK_ERROR; + goto end; + } } /* From patchwork Thu Dec 5 06:58:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Desnoyers X-Patchwork-Id: 3552097 From: mathieu.desnoyers at efficios.com (Mathieu Desnoyers) Date: Thu, 5 Dec 2019 01:58:09 -0500 Subject: [lttng-dev] [PATCH babeltrace-1.5 6/6] Fix: lttng-live format: do not error out on empty streams hang up In-Reply-To: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> References: <20191205065809.16728-1-mathieu.desnoyers@efficios.com> Message-ID: <20191205065809.16728-7-mathieu.desnoyers@efficios.com> Attaching to a stream hung up before providing any trace packet causes ctf_open_mmap_stream_read() to return an error. This kind of scenario can happen with the upcoming "lttng clear" feature. Signed-off-by: Mathieu Desnoyers --- formats/ctf/ctf.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c index 980ebc9a..1ba9017f 100644 --- a/formats/ctf/ctf.c +++ b/formats/ctf/ctf.c @@ -2571,8 +2571,13 @@ int ctf_open_mmap_stream_read(struct ctf_trace *td, } ret = prepare_mmap_stream_definition(td, file_stream, packet_seek); - if (ret) + if (ret) { + /* We need to check for EOF here for empty files. */ + if (unlikely(file_stream->pos.offset == EOF)) { + ret = 0; + } goto error_index; + } /* * For now, only a single clock per trace is supported.