Fix: RCU lock imbalance on error in cmd_snapshot_list_outputs()
[lttng-tools.git] / src / bin / lttng-sessiond / cmd.c
index 4cc590a9585b480142ca4ea54cc0ecf1e770b585..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;
@@ -236,7 +239,7 @@ static int list_lttng_agent_events(struct agent *agt,
                strncpy(tmp_events[i].name, event->name, sizeof(tmp_events[i].name));
                tmp_events[i].name[sizeof(tmp_events[i].name) - 1] = '\0';
                tmp_events[i].enabled = event->enabled;
-               tmp_events[i].loglevel = event->loglevel;
+               tmp_events[i].loglevel = event->loglevel_value;
                tmp_events[i].loglevel_type = event->loglevel_type;
                i++;
        }
@@ -365,7 +368,7 @@ static int list_lttng_kernel_events(char *channel_name,
 
        if (nb_event == 0) {
                *events = NULL;
-               goto syscall;
+               goto end;
        }
 
        *events = zmalloc(nb_event * sizeof(struct lttng_event));
@@ -414,19 +417,7 @@ static int list_lttng_kernel_events(char *channel_name,
                i++;
        }
 
-syscall:
-       if (syscall_table) {
-               ssize_t new_size;
-
-               new_size = syscall_list_channel(kchan, events, nb_event);
-               if (new_size < 0) {
-                       free(events);
-                       ret = -new_size;
-                       goto error;
-               }
-               nb_event = new_size;
-       }
-
+end:
        return nb_event;
 
 error:
@@ -520,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;
@@ -1074,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:
        {
@@ -1219,32 +1203,22 @@ int cmd_disable_event(struct ltt_session *session, int domain,
 
                switch (event->type) {
                case LTTNG_EVENT_ALL:
-                       ret = event_kernel_disable_event_all(kchan);
-                       if (ret != LTTNG_OK) {
-                               goto error_unlock;
-                       }
-                       break;
-               case LTTNG_EVENT_TRACEPOINT:    /* fall-through */
+               case LTTNG_EVENT_TRACEPOINT:
                case LTTNG_EVENT_SYSCALL:
-                       if (!strcmp(event_name, "*")) {
-                               ret = event_kernel_disable_event_type(kchan,
-                                       event->type);
+               case LTTNG_EVENT_PROBE:
+               case LTTNG_EVENT_FUNCTION:
+               case LTTNG_EVENT_FUNCTION_ENTRY:/* fall-through */
+                       if (event_name[0] == '\0') {
+                               ret = event_kernel_disable_event(kchan,
+                                       NULL, event->type);
                        } else {
                                ret = event_kernel_disable_event(kchan,
-                                       event_name);
+                                       event_name, event->type);
                        }
                        if (ret != LTTNG_OK) {
                                goto error_unlock;
                        }
                        break;
-               case LTTNG_EVENT_PROBE:
-               case LTTNG_EVENT_FUNCTION:
-               case LTTNG_EVENT_FUNCTION_ENTRY:
-                       ret = event_kernel_disable_event(kchan, event_name);
-                       if (ret != LTTNG_OK) {
-                               goto error_unlock;
-                       }
-                       break;
                default:
                        ret = LTTNG_ERR_UNK;
                        goto error_unlock;
@@ -1267,7 +1241,7 @@ int cmd_disable_event(struct ltt_session *session, int domain,
 
                /*
                 * If a non-default channel has been created in the
-                * session, explicitely require that -c chan_name needs
+                * session, explicitly require that -c chan_name needs
                 * to be provided.
                 */
                if (usess->has_non_default_channel && channel_name[0] == '\0') {
@@ -1284,7 +1258,16 @@ int cmd_disable_event(struct ltt_session *session, int domain,
 
                switch (event->type) {
                case LTTNG_EVENT_ALL:
-                       ret = event_ust_disable_tracepoint(usess, uchan, event_name);
+                       /*
+                        * An empty event name means that everything
+                        * should be disabled.
+                        */
+                       if (event->name[0] == '\0') {
+                               ret = event_ust_disable_all_tracepoints(usess, uchan);
+                       } else {
+                               ret = event_ust_disable_tracepoint(usess, uchan,
+                                               event_name);
+                       }
                        if (ret != LTTNG_OK) {
                                goto error_unlock;
                        }
@@ -1320,8 +1303,11 @@ int cmd_disable_event(struct ltt_session *session, int domain,
                        ret = -LTTNG_ERR_UST_EVENT_NOT_FOUND;
                        goto error_unlock;
                }
-               /* The wild card * means that everything should be disabled. */
-               if (strncmp(event->name, "*", 1) == 0 && strlen(event->name) == 1) {
+               /*
+                * An empty event name means that everything
+                * should be disabled.
+                */
+               if (event->name[0] == '\0') {
                        ret = event_agent_disable_all(usess, agt);
                } else {
                        ret = event_agent_disable(usess, agt, event_name);
@@ -1521,13 +1507,13 @@ static int _cmd_enable_event(struct ltt_session *session,
 
        DBG("Enable event command for event \'%s\'", event->name);
 
+       rcu_read_lock();
+
        ret = validate_event_name(event->name);
        if (ret) {
                goto error;
        }
 
-       rcu_read_lock();
-
        switch (domain->type) {
        case LTTNG_DOMAIN_KERNEL:
        {
@@ -1553,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) {
@@ -1619,9 +1610,10 @@ static int _cmd_enable_event(struct ltt_session *session,
                        event->type = LTTNG_EVENT_SYSCALL;      /* Hack */
                        ret = event_kernel_enable_event(kchan, event,
                                filter_expression_a, filter_a);
+                       /* We have passed ownership */
+                       filter_expression_a = NULL;
+                       filter_a = NULL;
                        if (ret != LTTNG_OK) {
-                               free(filter_expression_a);
-                               free(filter_a);
                                goto error;
                        }
                        break;
@@ -1689,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) {
@@ -1737,7 +1734,9 @@ static int _cmd_enable_event(struct ltt_session *session,
                filter_expression = NULL;
                filter = NULL;
                exclusion = NULL;
-               if (ret != LTTNG_OK) {
+               if (ret == LTTNG_ERR_UST_EVENT_ENABLED) {
+                       goto already_enabled;
+               } else if (ret != LTTNG_OK) {
                        goto error;
                }
                break;
@@ -1835,7 +1834,9 @@ static int _cmd_enable_event(struct ltt_session *session,
                                        filter_copy, NULL, wpipe);
                }
 
-               if (ret != LTTNG_OK && ret != LTTNG_ERR_UST_EVENT_ENABLED) {
+               if (ret == LTTNG_ERR_UST_EVENT_ENABLED) {
+                       goto already_enabled;
+               } else if (ret != LTTNG_OK) {
                        goto error;
                }
 
@@ -1862,6 +1863,7 @@ static int _cmd_enable_event(struct ltt_session *session,
 
        ret = LTTNG_OK;
 
+already_enabled:
 error:
        free(filter_expression);
        free(filter);
@@ -2132,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) {
@@ -2140,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) {
@@ -2148,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;
        }
 
@@ -2345,7 +2347,7 @@ int cmd_create_session_snapshot(char *name, struct lttng_uri *uris,
         * Create session in no output mode with URIs set to NULL. The uris we've
         * received are for a default snapshot output if one.
         */
-       ret = cmd_create_session_uri(name, NULL, 0, creds, -1);
+       ret = cmd_create_session_uri(name, NULL, 0, creds, 0);
        if (ret != LTTNG_OK) {
                goto error;
        }
@@ -2740,7 +2742,11 @@ ssize_t cmd_list_events(int domain, struct ltt_session *session,
                        rcu_read_lock();
                        cds_lfht_for_each_entry(session->ust_session->agents->ht,
                                        &iter.iter, agt, node.node) {
-                               nb_event = list_lttng_agent_events(agt, events);
+                               if (agt->domain == domain) {
+                                       nb_event = list_lttng_agent_events(
+                                                       agt, events);
+                                       break;
+                               }
                        }
                        rcu_read_unlock();
                }
@@ -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,138 +3366,104 @@ 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;
        }
 
-       if (session->kernel_session) {
-               struct ltt_kernel_session *ksess = session->kernel_session;
+       if (use_tmp_output) {
+               int64_t nb_packets_per_stream;
 
-               if (use_tmp_output) {
-                       int64_t nb_packets_per_stream;
+               nb_packets_per_stream = get_session_nb_packets_per_stream(session,
+                               tmp_output.max_size);
+               if (nb_packets_per_stream < 0) {
+                       ret = LTTNG_ERR_MAX_SIZE_INVALID;
+                       goto error;
+               }
 
-                       nb_packets_per_stream = get_session_nb_packets_per_stream(session,
-                                       tmp_output.max_size);
-                       if (nb_packets_per_stream < 0) {
-                               ret = LTTNG_ERR_MAX_SIZE_INVALID;
+               if (session->kernel_session) {
+                       ret = record_kernel_snapshot(session->kernel_session,
+                                       &tmp_output, session,
+                                       wait, nb_packets_per_stream);
+                       if (ret != LTTNG_OK) {
                                goto error;
                        }
-                       ret = record_kernel_snapshot(ksess, &tmp_output, session,
+               }
+
+               if (session->ust_session) {
+                       ret = record_ust_snapshot(session->ust_session,
+                                       &tmp_output, session,
                                        wait, nb_packets_per_stream);
                        if (ret != LTTNG_OK) {
                                goto error;
                        }
-                       snapshot_success = 1;
-               } else {
-                       struct snapshot_output *sout;
-                       struct lttng_ht_iter iter;
-
-                       rcu_read_lock();
-                       cds_lfht_for_each_entry(session->snapshot.output_ht->ht,
-                                       &iter.iter, sout, node.node) {
-                               int64_t nb_packets_per_stream;
-
-                               /*
-                                * Make a local copy of the output and assign the possible
-                                * temporary value given by the caller.
-                                */
-                               memset(&tmp_output, 0, sizeof(tmp_output));
-                               memcpy(&tmp_output, sout, sizeof(tmp_output));
-
-                               if (output->max_size != (uint64_t) -1ULL) {
-                                       tmp_output.max_size = output->max_size;
-                               }
+               }
 
-                               nb_packets_per_stream = get_session_nb_packets_per_stream(session,
-                                               tmp_output.max_size);
-                               if (nb_packets_per_stream < 0) {
-                                       ret = LTTNG_ERR_MAX_SIZE_INVALID;
-                                       goto error;
-                               }
+               snapshot_success = 1;
+       } else {
+               struct snapshot_output *sout;
+               struct lttng_ht_iter iter;
 
-                               /* Use temporary name. */
-                               if (*output->name != '\0') {
-                                       strncpy(tmp_output.name, output->name,
-                                                       sizeof(tmp_output.name));
-                               }
+               rcu_read_lock();
+               cds_lfht_for_each_entry(session->snapshot.output_ht->ht,
+                               &iter.iter, sout, node.node) {
+                       int64_t nb_packets_per_stream;
 
-                               tmp_output.nb_snapshot = session->snapshot.nb_snapshot;
+                       /*
+                        * Make a local copy of the output and assign the possible
+                        * temporary value given by the caller.
+                        */
+                       memset(&tmp_output, 0, sizeof(tmp_output));
+                       memcpy(&tmp_output, sout, sizeof(tmp_output));
 
-                               ret = record_kernel_snapshot(ksess, &tmp_output,
-                                               session, wait, nb_packets_per_stream);
-                               if (ret != LTTNG_OK) {
-                                       rcu_read_unlock();
-                                       goto error;
-                               }
-                               snapshot_success = 1;
+                       if (output->max_size != (uint64_t) -1ULL) {
+                               tmp_output.max_size = output->max_size;
                        }
-                       rcu_read_unlock();
-               }
-       }
-
-       if (session->ust_session) {
-               struct ltt_ust_session *usess = session->ust_session;
-
-               if (use_tmp_output) {
-                       int64_t nb_packets_per_stream;
 
                        nb_packets_per_stream = get_session_nb_packets_per_stream(session,
                                        tmp_output.max_size);
                        if (nb_packets_per_stream < 0) {
                                ret = LTTNG_ERR_MAX_SIZE_INVALID;
+                               rcu_read_unlock();
                                goto error;
                        }
-                       ret = record_ust_snapshot(usess, &tmp_output, session,
-                                       wait, nb_packets_per_stream);
-                       if (ret != LTTNG_OK) {
-                               goto error;
-                       }
-                       snapshot_success = 1;
-               } else {
-                       struct snapshot_output *sout;
-                       struct lttng_ht_iter iter;
-
-                       rcu_read_lock();
-                       cds_lfht_for_each_entry(session->snapshot.output_ht->ht,
-                                       &iter.iter, sout, node.node) {
-                               int64_t nb_packets_per_stream;
-
-                               /*
-                                * Make a local copy of the output and assign the possible
-                                * temporary value given by the caller.
-                                */
-                               memset(&tmp_output, 0, sizeof(tmp_output));
-                               memcpy(&tmp_output, sout, sizeof(tmp_output));
-
-                               if (output->max_size != (uint64_t) -1ULL) {
-                                       tmp_output.max_size = output->max_size;
-                               }
 
-                               nb_packets_per_stream = get_session_nb_packets_per_stream(session,
-                                               tmp_output.max_size);
-                               if (nb_packets_per_stream < 0) {
-                                       ret = LTTNG_ERR_MAX_SIZE_INVALID;
+                       /* Use temporary name. */
+                       if (*output->name != '\0') {
+                               if (lttng_strncpy(tmp_output.name, output->name,
+                                               sizeof(tmp_output.name))) {
+                                       ret = LTTNG_ERR_INVALID;
                                        rcu_read_unlock();
                                        goto error;
                                }
+                       }
 
-                               /* Use temporary name. */
-                               if (*output->name != '\0') {
-                                       strncpy(tmp_output.name, output->name,
-                                                       sizeof(tmp_output.name));
-                               }
+                       tmp_output.nb_snapshot = session->snapshot.nb_snapshot;
+                       memcpy(tmp_output.datetime, datetime, sizeof(datetime));
 
-                               tmp_output.nb_snapshot = session->snapshot.nb_snapshot;
+                       if (session->kernel_session) {
+                               ret = record_kernel_snapshot(session->kernel_session,
+                                               &tmp_output, session,
+                                               wait, nb_packets_per_stream);
+                               if (ret != LTTNG_OK) {
+                                       rcu_read_unlock();
+                                       goto error;
+                               }
+                       }
 
-                               ret = record_ust_snapshot(usess, &tmp_output, session,
+                       if (session->ust_session) {
+                               ret = record_ust_snapshot(session->ust_session,
+                                               &tmp_output, session,
                                                wait, nb_packets_per_stream);
                                if (ret != LTTNG_OK) {
                                        rcu_read_unlock();
                                        goto error;
                                }
-                               snapshot_success = 1;
                        }
-                       rcu_read_unlock();
+                       snapshot_success = 1;
                }
+               rcu_read_unlock();
        }
 
        if (snapshot_success) {
This page took 0.031982 seconds and 4 git commands to generate.