Fix: RCU lock imbalance on error in cmd_snapshot_list_outputs()
[lttng-tools.git] / src / bin / lttng-sessiond / cmd.c
index ffc6e739f8f4faf41728936eeba06aaf29bd6f7c..ffbfd4a6208017fd2543713c041b4f5bf877a5cc 100644 (file)
@@ -170,7 +170,10 @@ static void list_lttng_channels(int domain, struct ltt_session *session,
                rcu_read_lock();
                cds_lfht_for_each_entry(session->ust_session->domain_global.channels->ht,
                                &iter.iter, uchan, node.node) {
-                       strncpy(channels[i].name, uchan->name, LTTNG_SYMBOL_NAME_LEN);
+                       if (lttng_strncpy(channels[i].name, uchan->name,
+                                       LTTNG_SYMBOL_NAME_LEN)) {
+                               break;
+                       }
                        channels[i].attr.overwrite = uchan->attr.overwrite;
                        channels[i].attr.subbuf_size = uchan->attr.subbuf_size;
                        channels[i].attr.num_subbuf = uchan->attr.num_subbuf;
@@ -508,12 +511,15 @@ static int add_uri_to_consumer(struct consumer_output *consumer,
                DBG2("Setting trace directory path from URI to %s", uri->dst.path);
                memset(consumer->dst.trace_path, 0,
                                sizeof(consumer->dst.trace_path));
-               strncpy(consumer->dst.trace_path, uri->dst.path,
-                               sizeof(consumer->dst.trace_path));
+               /* Explicit length checks for strcpy and strcat. */
+               if (strlen(uri->dst.path) + strlen(default_trace_dir)
+                               >= sizeof(consumer->dst.trace_path)) {
+                       ret = LTTNG_ERR_FATAL;
+                       goto error;
+               }
+               strcpy(consumer->dst.trace_path, uri->dst.path);
                /* Append default trace dir */
-               strncat(consumer->dst.trace_path, default_trace_dir,
-                               sizeof(consumer->dst.trace_path) -
-                               strlen(consumer->dst.trace_path) - 1);
+               strcat(consumer->dst.trace_path, default_trace_dir);
                /* Flag consumer as local. */
                consumer->type = CONSUMER_DST_LOCAL;
                break;
@@ -1062,16 +1068,6 @@ int cmd_enable_channel(struct ltt_session *session,
                attr->attr.switch_timer_interval = 0;
        }
 
-       /*
-        * The ringbuffer (both in user space and kernel) behave badly in overwrite
-        * mode and with less than 2 subbuf so block it right away and send back an
-        * invalid attribute error.
-        */
-       if (attr->attr.overwrite && attr->attr.num_subbuf < 2) {
-               ret = LTTNG_ERR_INVALID;
-               goto error;
-       }
-
        switch (domain->type) {
        case LTTNG_DOMAIN_KERNEL:
        {
@@ -1543,7 +1539,12 @@ static int _cmd_enable_event(struct ltt_session *session,
                                ret = LTTNG_ERR_FATAL;
                                goto error;
                        }
-                       strncpy(attr->name, channel_name, sizeof(attr->name));
+                       if (lttng_strncpy(attr->name, channel_name,
+                                       sizeof(attr->name))) {
+                               ret = LTTNG_ERR_INVALID;
+                               free(attr);
+                               goto error;
+                       }
 
                        ret = cmd_enable_channel(session, domain, attr, wpipe);
                        if (ret != LTTNG_OK) {
@@ -1680,7 +1681,12 @@ static int _cmd_enable_event(struct ltt_session *session,
                                ret = LTTNG_ERR_FATAL;
                                goto error;
                        }
-                       strncpy(attr->name, channel_name, sizeof(attr->name));
+                       if (lttng_strncpy(attr->name, channel_name,
+                                       sizeof(attr->name))) {
+                               ret = LTTNG_ERR_INVALID;
+                               free(attr);
+                               goto error;
+                       }
 
                        ret = cmd_enable_channel(session, domain, attr, wpipe);
                        if (ret != LTTNG_OK) {
@@ -2128,7 +2134,15 @@ int cmd_stop_trace(struct ltt_session *session)
        if (ksession && ksession->active) {
                DBG("Stop kernel tracing");
 
-               /* Flush metadata if exist */
+               ret = kernel_stop_session(ksession);
+               if (ret < 0) {
+                       ret = LTTNG_ERR_KERN_STOP_FAIL;
+                       goto error;
+               }
+
+               kernel_wait_quiescent(kernel_tracer_fd);
+
+               /* Flush metadata after stopping (if exists) */
                if (ksession->metadata_stream_fd >= 0) {
                        ret = kernel_metadata_flush_buffer(ksession->metadata_stream_fd);
                        if (ret < 0) {
@@ -2136,7 +2150,7 @@ int cmd_stop_trace(struct ltt_session *session)
                        }
                }
 
-               /* Flush all buffers before stopping */
+               /* Flush all buffers after stopping */
                cds_list_for_each_entry(kchan, &ksession->channel_list.head, list) {
                        ret = kernel_flush_buffer(kchan);
                        if (ret < 0) {
@@ -2144,14 +2158,6 @@ int cmd_stop_trace(struct ltt_session *session)
                        }
                }
 
-               ret = kernel_stop_session(ksession);
-               if (ret < 0) {
-                       ret = LTTNG_ERR_KERN_STOP_FAIL;
-                       goto error;
-               }
-
-               kernel_wait_quiescent(kernel_tracer_fd);
-
                ksession->active = 0;
        }
 
@@ -3027,17 +3033,25 @@ ssize_t cmd_snapshot_list_outputs(struct ltt_session *session,
                assert(output->consumer);
                list[idx].id = output->id;
                list[idx].max_size = output->max_size;
-               strncpy(list[idx].name, output->name, sizeof(list[idx].name));
+               if (lttng_strncpy(list[idx].name, output->name,
+                               sizeof(list[idx].name))) {
+                       ret = -LTTNG_ERR_INVALID;
+                       goto error_unlock;
+               }
                if (output->consumer->type == CONSUMER_DST_LOCAL) {
-                       strncpy(list[idx].ctrl_url, output->consumer->dst.trace_path,
-                                       sizeof(list[idx].ctrl_url));
+                       if (lttng_strncpy(list[idx].ctrl_url,
+                                       output->consumer->dst.trace_path,
+                                       sizeof(list[idx].ctrl_url))) {
+                               ret = -LTTNG_ERR_INVALID;
+                               goto error_unlock;
+                       }
                } else {
                        /* Control URI. */
                        ret = uri_to_str_url(&output->consumer->dst.net.control,
                                        list[idx].ctrl_url, sizeof(list[idx].ctrl_url));
                        if (ret < 0) {
                                ret = -LTTNG_ERR_NOMEM;
-                               goto error;
+                               goto error_unlock;
                        }
 
                        /* Data URI. */
@@ -3045,7 +3059,7 @@ ssize_t cmd_snapshot_list_outputs(struct ltt_session *session,
                                        list[idx].data_url, sizeof(list[idx].data_url));
                        if (ret < 0) {
                                ret = -LTTNG_ERR_NOMEM;
-                               goto error;
+                               goto error_unlock;
                        }
                }
                idx++;
@@ -3054,9 +3068,10 @@ ssize_t cmd_snapshot_list_outputs(struct ltt_session *session,
        *outputs = list;
        list = NULL;
        ret = session->snapshot.nb_output;
+error_unlock:
+       rcu_read_unlock();
 error:
        free(list);
-       rcu_read_unlock();
        return ret;
 }
 
@@ -3121,13 +3136,6 @@ static int record_kernel_snapshot(struct ltt_kernel_session *ksess,
        assert(output);
        assert(session);
 
-       /* Get the datetime for the snapshot output directory. */
-       ret = utils_get_current_time_str("%Y%m%d-%H%M%S", output->datetime,
-                       sizeof(output->datetime));
-       if (!ret) {
-               ret = LTTNG_ERR_INVALID;
-               goto error;
-       }
 
        /*
         * Copy kernel session sockets so we can communicate with the right
@@ -3175,14 +3183,6 @@ static int record_ust_snapshot(struct ltt_ust_session *usess,
        assert(output);
        assert(session);
 
-       /* Get the datetime for the snapshot output directory. */
-       ret = utils_get_current_time_str("%Y%m%d-%H%M%S", output->datetime,
-                       sizeof(output->datetime));
-       if (!ret) {
-               ret = LTTNG_ERR_INVALID;
-               goto error;
-       }
-
        /*
         * Copy UST session sockets so we can communicate with the right
         * consumer for the snapshot record command.
@@ -3204,9 +3204,6 @@ static int record_ust_snapshot(struct ltt_ust_session *usess,
                case EINVAL:
                        ret = LTTNG_ERR_INVALID;
                        break;
-               case ENODATA:
-                       ret = LTTNG_ERR_SNAPSHOT_NODATA;
-                       break;
                default:
                        ret = LTTNG_ERR_SNAPSHOT_FAIL;
                        break;
@@ -3324,12 +3321,21 @@ int cmd_snapshot_record(struct ltt_session *session,
        unsigned int use_tmp_output = 0;
        struct snapshot_output tmp_output;
        unsigned int snapshot_success = 0;
+       char datetime[16];
 
        assert(session);
        assert(output);
 
        DBG("Cmd snapshot record for session %s", session->name);
 
+       /* Get the datetime for the snapshot output directory. */
+       ret = utils_get_current_time_str("%Y%m%d-%H%M%S", datetime,
+                       sizeof(datetime));
+       if (!ret) {
+               ret = LTTNG_ERR_INVALID;
+               goto error;
+       }
+
        /*
         * Permission denied to create an output if the session is not
         * set in no output mode.
@@ -3360,6 +3366,9 @@ int cmd_snapshot_record(struct ltt_session *session,
                }
                /* Use the global session count for the temporary snapshot. */
                tmp_output.nb_snapshot = session->snapshot.nb_snapshot;
+
+               /* Use the global datetime */
+               memcpy(tmp_output.datetime, datetime, sizeof(datetime));
                use_tmp_output = 1;
        }
 
@@ -3422,11 +3431,16 @@ int cmd_snapshot_record(struct ltt_session *session,
 
                        /* Use temporary name. */
                        if (*output->name != '\0') {
-                               strncpy(tmp_output.name, output->name,
-                                               sizeof(tmp_output.name));
+                               if (lttng_strncpy(tmp_output.name, output->name,
+                                               sizeof(tmp_output.name))) {
+                                       ret = LTTNG_ERR_INVALID;
+                                       rcu_read_unlock();
+                                       goto error;
+                               }
                        }
 
                        tmp_output.nb_snapshot = session->snapshot.nb_snapshot;
+                       memcpy(tmp_output.datetime, datetime, sizeof(datetime));
 
                        if (session->kernel_session) {
                                ret = record_kernel_snapshot(session->kernel_session,
This page took 0.027279 seconds and 4 git commands to generate.