[lttng-dev] [PATCH lttng-tools] Fix: acquire stream lock during kernel metadata snapshot

[Jonathan Rajotte]

From: Jérémie Galarneau <jeremie.galarneau at efficios.com>

The stream lock is not taken when interacting with the kernel
metadata stream that is created at the time a snapshot is taken.

This was noticed while reviewing the code for an unrelated reason,
so there is no known problem caused by this. Nevertheless, this
is incorrect as the stream is globally visible in the consumer.

Moreover, the stream was not cleaned-up which can cause a leak
whenever a metadata snapshot fails.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau at efficios.com>
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 src/common/kernel-consumer/kernel-consumer.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c
index 87ade12fa..3455f827b 100644
--- a/src/common/kernel-consumer/kernel-consumer.c
+++ b/src/common/kernel-consumer/kernel-consumer.c
@@ -336,7 +336,7 @@ end:
  *
  * Returns 0 on success, < 0 on error
  */
-int lttng_kconsumer_snapshot_metadata(uint64_t key, char *path,
+static int lttng_kconsumer_snapshot_metadata(uint64_t key, char *path,
 		uint64_t relayd_id, struct lttng_consumer_local_data *ctx)
 {
 	int ret, use_relayd = 0;
@@ -355,11 +355,12 @@ int lttng_kconsumer_snapshot_metadata(uint64_t key, char *path,
 	if (!metadata_channel) {
 		ERR("Kernel snapshot metadata not found for key %" PRIu64, key);
 		ret = -1;
-		goto error;
+		goto error_no_channel;
 	}
 
 	metadata_stream = metadata_channel->metadata_stream;
 	assert(metadata_stream);
+	pthread_mutex_lock(&metadata_stream->lock);
 
 	/* Flag once that we have a valid relayd for the stream. */
 	if (relayd_id != (uint64_t) -1ULL) {
@@ -369,7 +370,7 @@ int lttng_kconsumer_snapshot_metadata(uint64_t key, char *path,
 	if (use_relayd) {
 		ret = consumer_send_relayd_stream(metadata_stream, path);
 		if (ret < 0) {
-			goto error;
+			goto error_snapshot;
 		}
 	} else {
 		ret = utils_create_stream_file(path, metadata_stream->name,
@@ -377,7 +378,7 @@ int lttng_kconsumer_snapshot_metadata(uint64_t key, char *path,
 				metadata_stream->tracefile_count_current,
 				metadata_stream->uid, metadata_stream->gid, NULL);
 		if (ret < 0) {
-			goto error;
+			goto error_snapshot;
 		}
 		metadata_stream->out_fd = ret;
 	}
@@ -390,7 +391,8 @@ int lttng_kconsumer_snapshot_metadata(uint64_t key, char *path,
 			if (ret_read != -EAGAIN) {
 				ERR("Kernel snapshot reading metadata subbuffer (ret: %zd)",
 						ret_read);
-				goto error;
+				ret = ret_read;
+				goto error_snapshot;
 			}
 			/* ret_read is negative at this point so we will exit the loop. */
 			continue;
@@ -415,11 +417,12 @@ int lttng_kconsumer_snapshot_metadata(uint64_t key, char *path,
 	}
 
 	ret = 0;
-
+error_snapshot:
+	pthread_mutex_unlock(&metadata_stream->lock);
 	cds_list_del(&metadata_stream->send_node);
 	consumer_stream_destroy(metadata_stream, NULL);
 	metadata_channel->metadata_stream = NULL;
-error:
+error_no_channel:
 	rcu_read_unlock();
 	return ret;
 }
-- 
2.17.1