Fix: leak of trace_path on error in ust_app_snapshot_record
authorJérémie Galarneau <jeremie.galarneau@efficios.com>
Mon, 9 Sep 2019 15:22:39 +0000 (11:22 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Mon, 9 Sep 2019 16:23:02 +0000 (12:23 -0400)
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>
src/bin/lttng-sessiond/ust-app.c

index 0e97b944ca094722f4bb36f468455633e1333608..66b50eb43df98ea42c0700c5cedd5e9f28e17e8b 100644 (file)
@@ -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;
        enum lttng_error_code status = LTTNG_OK;
        struct lttng_ht_iter iter;
        struct ust_app *app;
+       char *trace_path = NULL;
 
        assert(usess);
        assert(output);
 
        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;
                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) {
                        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;
                        }
                                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;
                        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) {
                                                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);
                                        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;
                        }
                        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;
                        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);
                        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;
                        }
                                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;
                        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:
                                case LTTNG_OK:
                                        break;
                                case LTTNG_ERR_CHAN_NOT_FOUND:
-                                       free(trace_path);
                                        continue;
                                default:
                                        continue;
                                default:
-                                       free(trace_path);
                                        goto error;
                                }
                        }
                                        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);
                                        ua_sess->effective_credentials.uid,
                                        ua_sess->effective_credentials.gid,
                                        trace_path, wait, 0);
-                       free(trace_path);
                        switch (status) {
                        case LTTNG_OK:
                                break;
                        switch (status) {
                        case LTTNG_OK:
                                break;
@@ -6045,6 +6043,7 @@ enum lttng_error_code ust_app_snapshot_record(
        }
 
 error:
        }
 
 error:
+       free(trace_path);
        rcu_read_unlock();
        return status;
 }
        rcu_read_unlock();
        return status;
 }
This page took 0.041744 seconds and 4 git commands to generate.