From: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Date: Mon, 9 Sep 2019 15:22:39 +0000 (-0400)
Subject: Fix: leak of trace_path on error in ust_app_snapshot_record
X-Git-Tag: v2.11.0-rc4~78
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=9cb4b50c55c0266c7f6de0b033bd59df0fdedd70;p=lttng-tools.git

Fix: leak of trace_path on error in ust_app_snapshot_record

trace_path is leaked in some error paths of ust_app_snapshot_record().
Lift trace_path to the function's scope and free it whenever it is not
NULL. This is not clean, but this function should be cleaned-up in a
separate patch.

Moreover, this fixes a use after free in the PER_PID-case as
trace_path is free'd when a channel is not found.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
---

diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index 0e97b944c..66b50eb43 100644
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -5885,6 +5885,7 @@ enum lttng_error_code ust_app_snapshot_record(
 	enum lttng_error_code status = LTTNG_OK;
 	struct lttng_ht_iter iter;
 	struct ust_app *app;
+	char *trace_path = NULL;
 
 	assert(usess);
 	assert(output);
@@ -5899,7 +5900,6 @@ enum lttng_error_code ust_app_snapshot_record(
 		cds_list_for_each_entry(reg, &usess->buffer_reg_uid_list, lnode) {
 			struct buffer_reg_channel *reg_chan;
 			struct consumer_socket *socket;
-			char *trace_path = NULL;
 			char pathname[PATH_MAX];
 
 			if (!reg->registry->reg.ust->metadata_key) {
@@ -5928,6 +5928,8 @@ enum lttng_error_code ust_app_snapshot_record(
 				status = LTTNG_ERR_INVALID;
 				goto error;
 			}
+			/* Free path allowed on previous iteration. */
+			free(trace_path);
 			trace_path = setup_channel_trace_path(usess->consumer, pathname);
 			if (!trace_path) {
 				status = LTTNG_ERR_INVALID;
@@ -5942,14 +5944,12 @@ enum lttng_error_code ust_app_snapshot_record(
 						usess->gid, trace_path, wait,
 						nb_packets_per_stream);
 				if (status != LTTNG_OK) {
-					free(trace_path);
 					goto error;
 				}
 			}
 			status = consumer_snapshot_channel(socket,
 					reg->registry->reg.ust->metadata_key, output, 1,
 					usess->uid, usess->gid, trace_path, wait, 0);
-			free(trace_path);
 			if (status != LTTNG_OK) {
 				goto error;
 			}
@@ -5964,7 +5964,6 @@ enum lttng_error_code ust_app_snapshot_record(
 			struct ust_app_channel *ua_chan;
 			struct ust_app_session *ua_sess;
 			struct ust_registry_session *registry;
-			char *trace_path = NULL;
 			char pathname[PATH_MAX];
 
 			ua_sess = lookup_session_by_app(usess, app);
@@ -5990,6 +5989,8 @@ enum lttng_error_code ust_app_snapshot_record(
 				PERROR("snprintf snapshot path");
 				goto error;
 			}
+			/* Free path allowed on previous iteration. */
+			free(trace_path);
 			trace_path = setup_channel_trace_path(usess->consumer, pathname);
 			if (!trace_path) {
 				status = LTTNG_ERR_INVALID;
@@ -6009,10 +6010,8 @@ enum lttng_error_code ust_app_snapshot_record(
 				case LTTNG_OK:
 					break;
 				case LTTNG_ERR_CHAN_NOT_FOUND:
-					free(trace_path);
 					continue;
 				default:
-					free(trace_path);
 					goto error;
 				}
 			}
@@ -6027,7 +6026,6 @@ enum lttng_error_code ust_app_snapshot_record(
 					ua_sess->effective_credentials.uid,
 					ua_sess->effective_credentials.gid,
 					trace_path, wait, 0);
-			free(trace_path);
 			switch (status) {
 			case LTTNG_OK:
 				break;
@@ -6045,6 +6043,7 @@ enum lttng_error_code ust_app_snapshot_record(
 	}
 
 error:
+	free(trace_path);
 	rcu_read_unlock();
 	return status;
 }