Observed issue
==============
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
This can be easily reproduced by forcing a failure during the setup
of the metadata reproducible using the following change:
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;
Cause
=====
Channels have a list of streams that are being "setup" and are not
yet monitored for consumption. During this setup phase, the streams are
owned by the channel. On destruction of the channel, any stream in that
list will thus be cleaned-up.
When destroying a consumer stream, a reference to its channel is 'put'.
This can result in the destruction of the channel.
In the situation described above, the release of the channel's reference
is done before the stream is removed from the channel's stream list.
This causes the channel's clean-up to invoke (again) the current
stream's clean-up, resulting in the double-free of the metadata bucket.
This problem is present in a number of error paths.
Solution
========
Some error paths already manually removed the consumer stream from it's
channel's stream list before invoking consumer_stream_destroy(). The
various error paths that have to deal with this possible situation are
changed to simply invoke consumer_stream_destroy().
consumer_stream_destroy() is modified to always remove the stream from
its channel's list before performing the rest of the clean-up. This
ensures that those double clean-ups can't occur.
Drawbacks
=========
None.
Reported-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
Tested-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ibeca9b675b86fc46be3f57826f7158de4da43df8
goto error;
}
+ stream->send_node = CDS_LIST_HEAD_INIT(stream->send_node);
stream->chan = channel;
stream->key = stream_key;
stream->trace_chunk = trace_chunk;
{
assert(stream);
+ cds_list_del_init(&stream->send_node);
+
/* Stream is in monitor mode. */
if (stream->monitor) {
struct lttng_consumer_channel *free_chan = NULL;
/* Delete streams that might have been left in the stream list. */
cds_list_for_each_entry_safe(stream, stmp, &channel->streams.head,
send_node) {
- cds_list_del(&stream->send_node);
/*
* Once a stream is added to this list, the buffers were created so we
* have a guarantee that this call will succeed. Setting the monitor
struct lttng_ht_node_u64 node_channel_id;
/* HT node used in consumer_data.stream_list_ht */
struct lttng_ht_node_u64 node_session_id;
+ /*
+ * List used by channels to reference streams that are not yet globally
+ * visible.
+ */
+ struct cds_list_head send_node;
/* Pointer to associated channel. */
struct lttng_consumer_channel *chan;
/*
char name[LTTNG_SYMBOL_NAME_LEN];
/* Internal state of libustctl. */
struct lttng_ust_ctl_consumer_stream *ustream;
- struct cds_list_head send_node;
/* On-disk circular buffer */
uint64_t tracefile_size_current;
uint64_t tracefile_count_current;
ret = 0;
error_snapshot:
metadata_stream->read_subbuffer_ops.unlock(metadata_stream);
- cds_list_del(&metadata_stream->send_node);
consumer_stream_destroy(metadata_stream, NULL);
metadata_channel->metadata_stream = NULL;
rcu_read_unlock();
health_code_update();
- cds_list_del(&stream->send_node);
+ cds_list_del_init(&stream->send_node);
lttng_ust_ctl_destroy_stream(stream->ustream);
lttng_trace_chunk_put(stream->trace_chunk);
free(stream);
* global.
*/
stream->globally_visible = 1;
- cds_list_del(&stream->send_node);
+ cds_list_del_init(&stream->send_node);
ret = lttng_pipe_write(stream_pipe, &stream, sizeof(stream));
if (ret < 0) {
* will make sure to clean that list.
*/
consumer_stream_destroy(metadata->metadata_stream, NULL);
- cds_list_del(&metadata->metadata_stream->send_node);
metadata->metadata_stream = NULL;
send_streams_error:
error_no_stream:
* new metadata stream.
*/
consumer_stream_destroy(metadata_stream, NULL);
- cds_list_del(&metadata_stream->send_node);
metadata_channel->metadata_stream = NULL;
error:
projects / lttng-tools.git / commitdiff
