* [PATCH babeltrace-1.5 1/6] Fix: lttng-live: use-after-free in get_next_index()
[not found] <20191205065809.16728-1-mathieu.desnoyers@efficios.com>
@ 2019-12-05 6:58 ` Mathieu Desnoyers
2019-12-05 6:58 ` [PATCH babeltrace-1.5 2/6] Fix: trace-collection: trace clock use after free Mathieu Desnoyers
` (5 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Mathieu Desnoyers @ 2019-12-05 6:58 UTC (permalink / raw)
To: jgalar; +Cc: lttng-dev
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 <mathieu.desnoyers@efficios.com>
---
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");
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH babeltrace-1.5 2/6] Fix: trace-collection: trace clock use after free
[not found] <20191205065809.16728-1-mathieu.desnoyers@efficios.com>
2019-12-05 6:58 ` [PATCH babeltrace-1.5 1/6] Fix: lttng-live: use-after-free in get_next_index() Mathieu Desnoyers
@ 2019-12-05 6:58 ` Mathieu Desnoyers
2019-12-05 6:58 ` [PATCH babeltrace-1.5 3/6] Fix: lttng-live: lttng_live_open_trace_read memory leak Mathieu Desnoyers
` (4 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Mathieu Desnoyers @ 2019-12-05 6:58 UTC (permalink / raw)
To: jgalar; +Cc: lttng-dev
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 <mathieu.desnoyers@efficios.com>
---
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;
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH babeltrace-1.5 3/6] Fix: lttng-live: lttng_live_open_trace_read memory leak
[not found] <20191205065809.16728-1-mathieu.desnoyers@efficios.com>
2019-12-05 6:58 ` [PATCH babeltrace-1.5 1/6] Fix: lttng-live: use-after-free in get_next_index() Mathieu Desnoyers
2019-12-05 6:58 ` [PATCH babeltrace-1.5 2/6] Fix: trace-collection: trace clock use after free Mathieu Desnoyers
@ 2019-12-05 6:58 ` Mathieu Desnoyers
2019-12-05 6:58 ` [PATCH babeltrace-1.5 4/6] Fix: lib/iterator.c: unbalanced ctx put (leak) Mathieu Desnoyers
` (3 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Mathieu Desnoyers @ 2019-12-05 6:58 UTC (permalink / raw)
To: jgalar; +Cc: lttng-dev
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
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);
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH babeltrace-1.5 4/6] Fix: lib/iterator.c: unbalanced ctx put (leak)
[not found] <20191205065809.16728-1-mathieu.desnoyers@efficios.com>
` (2 preceding siblings ...)
2019-12-05 6:58 ` [PATCH babeltrace-1.5 3/6] Fix: lttng-live: lttng_live_open_trace_read memory leak Mathieu Desnoyers
@ 2019-12-05 6:58 ` Mathieu Desnoyers
2019-12-05 6:58 ` [PATCH babeltrace-1.5 5/6] Fix: lttng-live: ctf_live_packet_seek stream hang up handling Mathieu Desnoyers
` (2 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Mathieu Desnoyers @ 2019-12-05 6:58 UTC (permalink / raw)
To: jgalar; +Cc: lttng-dev
Missing context put in iterator init error path.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
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)
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH babeltrace-1.5 5/6] Fix: lttng-live: ctf_live_packet_seek stream hang up handling
[not found] <20191205065809.16728-1-mathieu.desnoyers@efficios.com>
` (3 preceding siblings ...)
2019-12-05 6:58 ` [PATCH babeltrace-1.5 4/6] Fix: lib/iterator.c: unbalanced ctx put (leak) Mathieu Desnoyers
@ 2019-12-05 6:58 ` Mathieu Desnoyers
2019-12-05 6:58 ` [PATCH babeltrace-1.5 6/6] Fix: lttng-live format: do not error out on empty streams hang up Mathieu Desnoyers
[not found] ` <20191205065809.16728-7-mathieu.desnoyers@efficios.com>
6 siblings, 0 replies; 7+ messages in thread
From: Mathieu Desnoyers @ 2019-12-05 6:58 UTC (permalink / raw)
To: jgalar; +Cc: lttng-dev
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 <mathieu.desnoyers@efficios.com>
---
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;
+ }
}
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH babeltrace-1.5 6/6] Fix: lttng-live format: do not error out on empty streams hang up
[not found] <20191205065809.16728-1-mathieu.desnoyers@efficios.com>
` (4 preceding siblings ...)
2019-12-05 6:58 ` [PATCH babeltrace-1.5 5/6] Fix: lttng-live: ctf_live_packet_seek stream hang up handling Mathieu Desnoyers
@ 2019-12-05 6:58 ` Mathieu Desnoyers
[not found] ` <20191205065809.16728-7-mathieu.desnoyers@efficios.com>
6 siblings, 0 replies; 7+ messages in thread
From: Mathieu Desnoyers @ 2019-12-05 6:58 UTC (permalink / raw)
To: jgalar; +Cc: lttng-dev
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 <mathieu.desnoyers@efficios.com>
---
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.
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <20191205065809.16728-7-mathieu.desnoyers@efficios.com>]
* Re: [PATCH babeltrace-1.5 6/6] Fix: lttng-live format: do not error out on empty streams hang up
[not found] ` <20191205065809.16728-7-mathieu.desnoyers@efficios.com>
@ 2019-12-13 1:03 ` Jérémie Galarneau
0 siblings, 0 replies; 7+ messages in thread
From: Jérémie Galarneau @ 2019-12-13 1:03 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: lttng-dev, Jeremie Galarneau
All patches of this series were merged in stable-1.5.
Thanks!
Jérémie
On Thu, 5 Dec 2019 at 01:58, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> 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 <mathieu.desnoyers@efficios.com>
> ---
> 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.
> --
> 2.17.1
>
--
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 7+ messages in thread