* [lttng-dev] [PATCH lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket
@ 2022-02-25 15:12 Vincent Whitchurch via lttng-dev
2022-03-01 17:19 ` Jérémie Galarneau via lttng-dev
0 siblings, 1 reply; 5+ messages in thread
From: Vincent Whitchurch via lttng-dev @ 2022-02-25 15:12 UTC (permalink / raw)
To: lttng-dev; +Cc: kernel
When consumer_stream_destroy() is called from, for example, the error
path in setup_metadata(), consumer_stream_free() can end up being called
twice on the same stream. Since the stream->metadata_bucket is not set
to NULL after being destroyed, it leads to a use-after-free:
ERROR: AddressSanitizer: heap-use-after-free on address 0x604000000318
READ of size 8 at 0x604000000318 thread T7
#0 in metadata_bucket_destroy
#1 in consumer_stream_free
#2 in consumer_stream_destroy
#3 in setup_metadata
#4 in lttng_ustconsumer_recv_cmd
#5 in lttng_consumer_recv_cmd
#6 in consumer_thread_sessiond_poll
#7 in start_thread nptl/pthread_create.c:481
#8 in clone (/lib/x86_64-linux-gnu/libc.so.6+0xfcbde)
0x604000000318 is located 8 bytes inside of 48-byte region [0x604000000310,0x604000000340)
freed by thread T7 here:
#0 in __interceptor_free
#1 in metadata_bucket_destroy
#2 in consumer_stream_free
#3 in consumer_stream_destroy
#4 in clean_channel_stream_list
#5 in consumer_del_channel
#6 in consumer_stream_destroy
#7 in setup_metadata
#8 in lttng_ustconsumer_recv_cmd
#9 in lttng_consumer_recv_cmd
#10 in consumer_thread_sessiond_poll
#11 in start_thread nptl/pthread_create.c:481
previously allocated by thread T7 here:
#0 in __interceptor_calloc
#1 in zmalloc
#2 in metadata_bucket_create
#3 in consumer_stream_enable_metadata_bucketization
#4 in lttng_ustconsumer_set_stream_ops
#5 in lttng_ustconsumer_on_recv_stream
#6 in lttng_consumer_on_recv_stream
#7 in create_ust_streams
#8 in ask_channel
#9 in lttng_ustconsumer_recv_cmd
#10 in lttng_consumer_recv_cmd
#11 in consumer_thread_sessiond_poll
#12 in start_thread nptl/pthread_create.c:481
Thread T7 created by T0 here:
#0 in __interceptor_pthread_create
#1 in main
#2 in __libc_start_main ../csu/libc-start.c:332
SUMMARY: AddressSanitizer: heap-use-after-free in metadata_bucket_destroy
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
src/common/consumer/consumer-stream.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/common/consumer/consumer-stream.cpp b/src/common/consumer/consumer-stream.cpp
index 2dc3f002b..481611c3e 100644
--- a/src/common/consumer/consumer-stream.cpp
+++ b/src/common/consumer/consumer-stream.cpp
@@ -988,6 +988,7 @@ void consumer_stream_free(struct lttng_consumer_stream *stream)
LTTNG_ASSERT(stream);
metadata_bucket_destroy(stream->metadata_bucket);
+ stream->metadata_bucket = NULL;
call_rcu(&stream->node.head, free_stream_rcu);
}
--
2.34.1
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [lttng-dev] [PATCH lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket
2022-02-25 15:12 [lttng-dev] [PATCH lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket Vincent Whitchurch via lttng-dev
@ 2022-03-01 17:19 ` Jérémie Galarneau via lttng-dev
2022-03-02 9:27 ` Vincent Whitchurch via lttng-dev
0 siblings, 1 reply; 5+ messages in thread
From: Jérémie Galarneau via lttng-dev @ 2022-03-01 17:19 UTC (permalink / raw)
To: Vincent Whitchurch; +Cc: lttng-dev, kernel
----- Message original -----
> De: "lttng-dev" <lttng-dev@lists.lttng.org>
> À: "lttng-dev" <lttng-dev@lists.lttng.org>
> Cc: kernel@axis.com
> Envoyé: Vendredi 25 Février 2022 10:12:02
> Objet: [lttng-dev] [PATCH lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket
> When consumer_stream_destroy() is called from, for example, the error
> path in setup_metadata(), consumer_stream_free() can end up being called
> twice on the same stream. Since the stream->metadata_bucket is not set
> to NULL after being destroyed, it leads to a use-after-free:
>
> ERROR: AddressSanitizer: heap-use-after-free on address 0x604000000318
> READ of size 8 at 0x604000000318 thread T7
> #0 in metadata_bucket_destroy
> #1 in consumer_stream_free
> #2 in consumer_stream_destroy
> #3 in setup_metadata
> #4 in lttng_ustconsumer_recv_cmd
> #5 in lttng_consumer_recv_cmd
> #6 in consumer_thread_sessiond_poll
> #7 in start_thread nptl/pthread_create.c:481
> #8 in clone (/lib/x86_64-linux-gnu/libc.so.6+0xfcbde)
>
> 0x604000000318 is located 8 bytes inside of 48-byte region
> [0x604000000310,0x604000000340)
> freed by thread T7 here:
> #0 in __interceptor_free
> #1 in metadata_bucket_destroy
> #2 in consumer_stream_free
> #3 in consumer_stream_destroy
> #4 in clean_channel_stream_list
> #5 in consumer_del_channel
> #6 in consumer_stream_destroy
> #7 in setup_metadata
> #8 in lttng_ustconsumer_recv_cmd
> #9 in lttng_consumer_recv_cmd
> #10 in consumer_thread_sessiond_poll
> #11 in start_thread nptl/pthread_create.c:481
>
> previously allocated by thread T7 here:
> #0 in __interceptor_calloc
> #1 in zmalloc
> #2 in metadata_bucket_create
> #3 in consumer_stream_enable_metadata_bucketization
> #4 in lttng_ustconsumer_set_stream_ops
> #5 in lttng_ustconsumer_on_recv_stream
> #6 in lttng_consumer_on_recv_stream
> #7 in create_ust_streams
> #8 in ask_channel
> #9 in lttng_ustconsumer_recv_cmd
> #10 in lttng_consumer_recv_cmd
> #11 in consumer_thread_sessiond_poll
> #12 in start_thread nptl/pthread_create.c:481
>
> Thread T7 created by T0 here:
> #0 in __interceptor_pthread_create
> #1 in main
> #2 in __libc_start_main ../csu/libc-start.c:332
>
> SUMMARY: AddressSanitizer: heap-use-after-free in metadata_bucket_destroy
>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> ---
> src/common/consumer/consumer-stream.cpp | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/common/consumer/consumer-stream.cpp
> b/src/common/consumer/consumer-stream.cpp
> index 2dc3f002b..481611c3e 100644
> --- a/src/common/consumer/consumer-stream.cpp
> +++ b/src/common/consumer/consumer-stream.cpp
> @@ -988,6 +988,7 @@ void consumer_stream_free(struct lttng_consumer_stream
> *stream)
> LTTNG_ASSERT(stream);
>
> metadata_bucket_destroy(stream->metadata_bucket);
> + stream->metadata_bucket = NULL;
Hi Vincent,
Thanks a lot for reporting the problem. If I understand the ASAN
report correctly, the stream itself will also be double free'd, so
I don't think this is the complete fix.
There definitely seems to be a problem with regards to the ownership
of the metadata channel vs stream. Let me look into it.
I see that you fall into a case where the metadata setup fails,
can you share more info about how this can be reproduced?
Thanks!
Jérémie
> call_rcu(&stream->node.head, free_stream_rcu);
> }
>
> --
> 2.34.1
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [lttng-dev] [PATCH lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket
2022-03-01 17:19 ` Jérémie Galarneau via lttng-dev
@ 2022-03-02 9:27 ` Vincent Whitchurch via lttng-dev
2022-03-07 17:37 ` Jérémie Galarneau via lttng-dev
0 siblings, 1 reply; 5+ messages in thread
From: Vincent Whitchurch via lttng-dev @ 2022-03-02 9:27 UTC (permalink / raw)
To: Jérémie Galarneau; +Cc: lttng-dev, kernel
On Tue, Mar 01, 2022 at 06:19:23PM +0100, Jérémie Galarneau wrote:
> Thanks a lot for reporting the problem. If I understand the ASAN
> report correctly, the stream itself will also be double free'd, so
> I don't think this is the complete fix.
Yeah, it looked odd that consumer_stream_destroy() is called recursively
on the same stream but AFAICS the code's been like this for a while so I
assumed it was on purpose, and only the metadata bucket stuff was
relatively new. ASAN doesn't detect any double frees of the stream
itself, but I guess calling call_rcu(..., free_stream_rcu) twice on the
same stream is not expected behaviour and could lead to other problems.
> There definitely seems to be a problem with regards to the ownership
> of the metadata channel vs stream. Let me look into it.
Great, thank you!
> I see that you fall into a case where the metadata setup fails,
> can you share more info about how this can be reproduced?
In the core dump I received (on v2.12.4), consumer_stream_destroy() was
called from the error label in setup_metadata and ret was set to
LTTCOMM_CONSUMERD_ERROR_METADATA. So consumer_send_relayd_stream() had
returned an error. I only had the core dump and no other logs, so I did
not know which of the paths inside consumer_send_relayd_stream() had
failed, but since I was primarily interested in fixing the crash itself
I simply forced this code path to be taken:
diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index fa1c71299..97ed59632 100644
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -908,8 +908,7 @@ static int setup_metadata(struct lttng_consumer_local_data *ctx, uint64_t key)
/* Send metadata stream to relayd if needed. */
if (metadata->metadata_stream->net_seq_idx != (uint64_t) -1ULL) {
- ret = consumer_send_relayd_stream(metadata->metadata_stream,
- metadata->pathname);
+ ret = -1;
if (ret < 0) {
ret = LTTCOMM_CONSUMERD_ERROR_METADATA;
goto error;
With the above patch, I could easily reproduce the use-after-free using
the following steps on the latest release v2.13.4, and it was clear that
this use-after-free was the cause of the original core dump on the older
release too.
Build with ASAN:
lttng-tools$ LDFLAGS=-fsanitize=address CFLAGS=-fsanitize=address ./configure
Shell #1:
lttng-ust$ tests/compile/api0/hello/hello 10000
Shell #2:
lttng-tools$ ASAN_OPTIONS=detect_odr_violation=0 ./src/bin/lttng-sessiond/lttng-sessiond
Shell #3:
lttng-tools$ export ASAN_OPTIONS=detect_odr_violation=0
lttng-tools$ ./src/bin/lttng/lttng create --live && ./src/bin/lttng/lttng enable-event --userspace 1 && ./src/bin/lttng/lttng start && sleep 1 && ./src/bin/lttng/lttng stop
The ASAN splat should be seen in shell #2. Note that you may have to
run the command in shell #3 a couple of times since
LTTNG_CONSUMER_SETUP_METADATA doesn't seem to be sent every time.
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [lttng-dev] [PATCH lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket
2022-03-02 9:27 ` Vincent Whitchurch via lttng-dev
@ 2022-03-07 17:37 ` Jérémie Galarneau via lttng-dev
2022-03-08 8:10 ` Vincent Whitchurch via lttng-dev
0 siblings, 1 reply; 5+ messages in thread
From: Jérémie Galarneau via lttng-dev @ 2022-03-07 17:37 UTC (permalink / raw)
To: Vincent Whitchurch; +Cc: lttng-dev, kernel
Hi Vincent,
I had a chance to look into this and came up with the following fix:
https://review.lttng.org/c/lttng-tools/+/7478/4
Would you have a chance to try it on your end before I merge it?
Thanks for the great bug report!
Jérémie
----- Original Message -----
> From: "Vincent Whitchurch" <vincent.whitchurch@axis.com>
> To: "Jeremie Galarneau" <jeremie.galarneau@efficios.com>
> Cc: "lttng-dev" <lttng-dev@lists.lttng.org>, "kernel" <kernel@axis.com>
> Sent: Wednesday, March 2, 2022 4:27:30 AM
> Subject: Re: [lttng-dev] [PATCH lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket
> On Tue, Mar 01, 2022 at 06:19:23PM +0100, Jérémie Galarneau wrote:
>> Thanks a lot for reporting the problem. If I understand the ASAN
>> report correctly, the stream itself will also be double free'd, so
>> I don't think this is the complete fix.
>
> Yeah, it looked odd that consumer_stream_destroy() is called recursively
> on the same stream but AFAICS the code's been like this for a while so I
> assumed it was on purpose, and only the metadata bucket stuff was
> relatively new. ASAN doesn't detect any double frees of the stream
> itself, but I guess calling call_rcu(..., free_stream_rcu) twice on the
> same stream is not expected behaviour and could lead to other problems.
>
>> There definitely seems to be a problem with regards to the ownership
>> of the metadata channel vs stream. Let me look into it.
>
> Great, thank you!
>
>> I see that you fall into a case where the metadata setup fails,
>> can you share more info about how this can be reproduced?
>
> In the core dump I received (on v2.12.4), consumer_stream_destroy() was
> called from the error label in setup_metadata and ret was set to
> LTTCOMM_CONSUMERD_ERROR_METADATA. So consumer_send_relayd_stream() had
> returned an error. I only had the core dump and no other logs, so I did
> not know which of the paths inside consumer_send_relayd_stream() had
> failed, but since I was primarily interested in fixing the crash itself
> I simply forced this code path to be taken:
>
> diff --git a/src/common/ust-consumer/ust-consumer.c
> b/src/common/ust-consumer/ust-consumer.c
> index fa1c71299..97ed59632 100644
> --- a/src/common/ust-consumer/ust-consumer.c
> +++ b/src/common/ust-consumer/ust-consumer.c
> @@ -908,8 +908,7 @@ static int setup_metadata(struct lttng_consumer_local_data
> *ctx, uint64_t key)
>
> /* Send metadata stream to relayd if needed. */
> if (metadata->metadata_stream->net_seq_idx != (uint64_t) -1ULL) {
> - ret = consumer_send_relayd_stream(metadata->metadata_stream,
> - metadata->pathname);
> + ret = -1;
> if (ret < 0) {
> ret = LTTCOMM_CONSUMERD_ERROR_METADATA;
> goto error;
>
> With the above patch, I could easily reproduce the use-after-free using
> the following steps on the latest release v2.13.4, and it was clear that
> this use-after-free was the cause of the original core dump on the older
> release too.
>
> Build with ASAN:
>
> lttng-tools$ LDFLAGS=-fsanitize=address CFLAGS=-fsanitize=address ./configure
>
> Shell #1:
>
> lttng-ust$ tests/compile/api0/hello/hello 10000
>
> Shell #2:
>
> lttng-tools$ ASAN_OPTIONS=detect_odr_violation=0
> ./src/bin/lttng-sessiond/lttng-sessiond
>
> Shell #3:
>
> lttng-tools$ export ASAN_OPTIONS=detect_odr_violation=0
> lttng-tools$ ./src/bin/lttng/lttng create --live && ./src/bin/lttng/lttng
> enable-event --userspace 1 && ./src/bin/lttng/lttng start && sleep 1 &&
> ./src/bin/lttng/lttng stop
>
> The ASAN splat should be seen in shell #2. Note that you may have to
> run the command in shell #3 a couple of times since
> LTTNG_CONSUMER_SETUP_METADATA doesn't seem to be sent every time.
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [lttng-dev] [PATCH lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket
2022-03-07 17:37 ` Jérémie Galarneau via lttng-dev
@ 2022-03-08 8:10 ` Vincent Whitchurch via lttng-dev
0 siblings, 0 replies; 5+ messages in thread
From: Vincent Whitchurch via lttng-dev @ 2022-03-08 8:10 UTC (permalink / raw)
To: Jérémie Galarneau; +Cc: lttng-dev, kernel
On Mon, Mar 07, 2022 at 06:37:49PM +0100, Jérémie Galarneau wrote:
> I had a chance to look into this and came up with the following fix:
> https://review.lttng.org/c/lttng-tools/+/7478/4
>
> Would you have a chance to try it on your end before I merge it?
I've tested the patch stack in patch set #5 and it does fix the problem
for me too. Please feel free to add this if you like:
Tested-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
(By the way, I noticed that patch(1) gets confused by the reproduction
patch which is part of the commit message. This probably shouldn't
matter much though, I only noticed it when I tried to revert the patch
with git show | patch -p1 -R.)
> Thanks for the great bug report!
Thank you for the fix!
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-08 8:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 15:12 [lttng-dev] [PATCH lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket Vincent Whitchurch via lttng-dev
2022-03-01 17:19 ` Jérémie Galarneau via lttng-dev
2022-03-02 9:27 ` Vincent Whitchurch via lttng-dev
2022-03-07 17:37 ` Jérémie Galarneau via lttng-dev
2022-03-08 8:10 ` Vincent Whitchurch via lttng-dev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).