diff mbox series

[lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket

Message ID 20220225151202.4148809-1-vincent.whitchurch@axis.com
State New
Headers show
Series [lttng-tools] Fix: consumer-stream: use-after-free of metadata bucket | expand

Commit Message

Vincent Whitchurch Feb. 25, 2022, 3:12 p.m. UTC
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 at axis.com>
---
 src/common/consumer/consumer-stream.cpp | 1 +
 1 file changed, 1 insertion(+)

Comments

Jérémie Galarneau March 1, 2022, 5:19 p.m. UTC | #1
----- Message original -----
> De: "lttng-dev" <lttng-dev at lists.lttng.org>
> ?: "lttng-dev" <lttng-dev at lists.lttng.org>
> Cc: kernel at 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 at 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 at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
diff mbox series

Patch

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);
 }