Fix: kernel session closes fd 0 after create

This fix the make check test for kernel session handling. We ensure that
all FDs that are unset are set to -1.

Acked-by: David Goulet <dgoulet@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 9e4bab468f1462308f6c5377a049847015824e1b..ff29eb03566775b8cc351296f4dab543a732cc0a 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -100,16 +100,22 @@ static struct consumer_data kconsumer_data = {
        .type = LTTNG_CONSUMER_KERNEL,
        .err_unix_sock_path = DEFAULT_KCONSUMERD_ERR_SOCK_PATH,
        .cmd_unix_sock_path = DEFAULT_KCONSUMERD_CMD_SOCK_PATH,
+       .err_sock = -1,
+       .cmd_sock = -1,
 };
 static struct consumer_data ustconsumer64_data = {
        .type = LTTNG_CONSUMER64_UST,
        .err_unix_sock_path = DEFAULT_USTCONSUMERD64_ERR_SOCK_PATH,
        .cmd_unix_sock_path = DEFAULT_USTCONSUMERD64_CMD_SOCK_PATH,
+       .err_sock = -1,
+       .cmd_sock = -1,
 };
 static struct consumer_data ustconsumer32_data = {
        .type = LTTNG_CONSUMER32_UST,
        .err_unix_sock_path = DEFAULT_USTCONSUMERD32_ERR_SOCK_PATH,
        .cmd_unix_sock_path = DEFAULT_USTCONSUMERD32_CMD_SOCK_PATH,
+       .err_sock = -1,
+       .cmd_sock = -1,
 };
 
 static int dispatch_thread_exit;
@@ -324,7 +330,8 @@ static void teardown_kernel_session(struct ltt_session *session)
         * If a custom kernel consumer was registered, close the socket before
         * tearing down the complete kernel session structure
         */
-       if (session->kernel_session->consumer_fd != kconsumer_data.cmd_sock) {
+       if (kconsumer_data.cmd_sock >= 0 &&
+                       session->kernel_session->consumer_fd != kconsumer_data.cmd_sock) {
                lttcomm_close_unix_sock(session->kernel_session->consumer_fd);
        }
 
@@ -573,12 +580,12 @@ static int send_kconsumer_session_streams(struct consumer_data *consumer_data,
 
        DBG("Sending metadata stream fd");
 
-       /* Extra protection. It's NOT supposed to be set to 0 at this point */
-       if (session->consumer_fd == 0) {
+       /* Extra protection. It's NOT supposed to be set to -1 at this point */
+       if (session->consumer_fd < 0) {
                session->consumer_fd = consumer_data->cmd_sock;
        }
 
-       if (session->metadata_stream_fd != 0) {
+       if (session->metadata_stream_fd >= 0) {
                /* Send metadata channel fd */
                lkm.cmd_type = LTTNG_CONSUMER_ADD_CHANNEL;
                lkm.u.channel.channel_key = session->metadata->fd;
@@ -752,8 +759,8 @@ static int update_kernel_stream(struct consumer_data *consumer_data, int fd)
                        continue;
                }
 
-               /* This is not suppose to be 0 but this is an extra security check */
-               if (session->kernel_session->consumer_fd == 0) {
+               /* This is not suppose to be -1 but this is an extra security check */
+               if (session->kernel_session->consumer_fd < 0) {
                        session->kernel_session->consumer_fd = consumer_data->cmd_sock;
                }
 
@@ -1851,10 +1858,10 @@ static int init_kernel_tracing(struct ltt_kernel_session *session)
        if (session->consumer_fds_sent == 0) {
                /*
                 * Assign default kernel consumer socket if no consumer assigned to the
-                * kernel session. At this point, it's NOT suppose to be 0 but this is
+                * kernel session. At this point, it's NOT supposed to be -1 but this is
                 * an extra security check.
                 */
-               if (session->consumer_fd == 0) {
+               if (session->consumer_fd < 0) {
                        session->consumer_fd = kconsumer_data.cmd_sock;
                }
 
@@ -1942,7 +1949,7 @@ static int create_kernel_session(struct ltt_session *session)
        }
 
        /* Set kernel consumer socket fd */
-       if (kconsumer_data.cmd_sock) {
+       if (kconsumer_data.cmd_sock >= 0) {
                session->kernel_session->consumer_fd = kconsumer_data.cmd_sock;
        }
 
@@ -2851,7 +2858,7 @@ static int cmd_start_trace(struct ltt_session *session)
                }
 
                /* Open kernel metadata stream */
-               if (ksession->metadata_stream_fd == 0) {
+               if (ksession->metadata_stream_fd < 0) {
                        ret = kernel_open_metadata_stream(ksession);
                        if (ret < 0) {
                                ERR("Kernel create metadata stream failed");

diff --git a/src/bin/lttng-sessiond/trace-kernel.c b/src/bin/lttng-sessiond/trace-kernel.c
index 8fb92f12ab13c08386108c0feddcc3769a1aee95..7789d6c208a09fa8aa1fec59f7b5f2f6816bcecc 100644 (file)
--- a/src/bin/lttng-sessiond/trace-kernel.c
+++ b/src/bin/lttng-sessiond/trace-kernel.c
@@ -96,12 +96,12 @@ struct ltt_kernel_session *trace_kernel_create_session(char *path)
        }
 
        /* Init data structure */
-       lks->fd = 0;
-       lks->metadata_stream_fd = 0;
+       lks->fd = -1;
+       lks->metadata_stream_fd = -1;
        lks->channel_count = 0;
        lks->stream_count_global = 0;
        lks->metadata = NULL;
-       lks->consumer_fd = 0;
+       lks->consumer_fd = -1;
        CDS_INIT_LIST_HEAD(&lks->channel_list.head);
 
        /* Set session path */
@@ -140,7 +140,7 @@ struct ltt_kernel_channel *trace_kernel_create_channel(struct lttng_channel *cha
        }
        memcpy(lkc->channel, chan, sizeof(struct lttng_channel));
 
-       lkc->fd = 0;
+       lkc->fd = -1;
        lkc->stream_count = 0;
        lkc->event_count = 0;
        lkc->enabled = 1;
@@ -221,7 +221,7 @@ struct ltt_kernel_event *trace_kernel_create_event(struct lttng_event *ev)
        attr->name[LTTNG_KERNEL_SYM_NAME_LEN - 1] = '\0';
 
        /* Setting up a kernel event */
-       lke->fd = 0;
+       lke->fd = -1;
        lke->event = attr;
        lke->enabled = 1;
        lke->ctx = NULL;
@@ -259,7 +259,7 @@ struct ltt_kernel_metadata *trace_kernel_create_metadata(char *path)
        chan->attr.output = DEFAULT_KERNEL_CHANNEL_OUTPUT;
 
        /* Init metadata */
-       lkm->fd = 0;
+       lkm->fd = -1;
        lkm->conf = chan;
        /* Set default metadata path */
        ret = asprintf(&lkm->pathname, "%s/metadata", path);
@@ -291,7 +291,7 @@ struct ltt_kernel_stream *trace_kernel_create_stream(void)
        }
 
        /* Init stream */
-       lks->fd = 0;
+       lks->fd = -1;
        lks->pathname = NULL;
        lks->state = 0;
 
@@ -310,9 +310,11 @@ void trace_kernel_destroy_stream(struct ltt_kernel_stream *stream)
 
        DBG("[trace] Closing stream fd %d", stream->fd);
        /* Close kernel fd */
-       ret = close(stream->fd);
-       if (ret) {
-               PERROR("close");
+       if (stream->fd >= 0) {
+               ret = close(stream->fd);
+               if (ret) {
+                       PERROR("close");
+               }
        }
        /* Remove from stream list */
        cds_list_del(&stream->list);
@@ -358,9 +360,11 @@ void trace_kernel_destroy_channel(struct ltt_kernel_channel *channel)
 
        DBG("[trace] Closing channel fd %d", channel->fd);
        /* Close kernel fd */
-       ret = close(channel->fd);
-       if (ret) {
-               PERROR("close");
+       if (channel->fd >= 0) {
+               ret = close(channel->fd);
+               if (ret) {
+                       PERROR("close");
+               }
        }
 
        /* For each stream in the channel list */
@@ -391,9 +395,11 @@ void trace_kernel_destroy_metadata(struct ltt_kernel_metadata *metadata)
 
        DBG("[trace] Closing metadata fd %d", metadata->fd);
        /* Close kernel fd */
-       ret = close(metadata->fd);
-       if (ret) {
-               PERROR("close");
+       if (metadata->fd >= 0) {
+               ret = close(metadata->fd);
+               if (ret) {
+                       PERROR("close");
+               }
        }
 
        free(metadata->conf);
@@ -411,12 +417,14 @@ void trace_kernel_destroy_session(struct ltt_kernel_session *session)
 
        DBG("[trace] Closing session fd %d", session->fd);
        /* Close kernel fds */
-       ret = close(session->fd);
-       if (ret) {
-               PERROR("close");
+       if (session->fd >= 0) {
+               ret = close(session->fd);
+               if (ret) {
+                       PERROR("close");
+               }
        }
 
-       if (session->metadata_stream_fd != 0) {
+       if (session->metadata_stream_fd >= 0) {
                DBG("[trace] Closing metadata stream fd %d", session->metadata_stream_fd);
                ret = close(session->metadata_stream_fd);
                if (ret) {

diff --git a/tests/test_kernel_data_trace.c b/tests/test_kernel_data_trace.c
index 0a534699c5444552669f7c0d9043a528d6579949..41728544f5c5f4aefffcb2a5b6999e98adc60345 100644 (file)
--- a/tests/test_kernel_data_trace.c
+++ b/tests/test_kernel_data_trace.c
@@ -72,13 +72,13 @@ static void create_one_kernel_session(void)
        PRINT_OK();
 
        printf("Validating kernel session: ");
-       assert(kern->fd == 0);
-       assert(kern->metadata_stream_fd == 0);
+       assert(kern->fd == -1);
+       assert(kern->metadata_stream_fd == -1);
        assert(kern->consumer_fds_sent == 0);
        assert(kern->channel_count == 0);
        assert(kern->stream_count_global == 0);
        assert(kern->metadata == NULL);
-       assert(kern->consumer_fd == 0);
+       assert(kern->consumer_fd == -1);
        PRINT_OK();
 
        /* Init list in order to avoid sefaults from cds_list_del */
@@ -95,7 +95,7 @@ static void create_kernel_metadata(void)
        PRINT_OK();
 
        printf("Validating kernel session metadata: ");
-       assert(kern->metadata->fd == 0);
+       assert(kern->metadata->fd == -1);
        assert(strlen(kern->metadata->pathname));
        assert(kern->metadata->conf != NULL);
        assert(kern->metadata->conf->attr.overwrite
@@ -128,7 +128,7 @@ static void create_kernel_channel(void)
        PRINT_OK();
 
        printf("Validating kernel channel: ");
-       assert(chan->fd == 0);
+       assert(chan->fd == -1);
        assert(chan->enabled == 1);
        assert(strcmp(PATH1, chan->pathname) == 0);
        assert(chan->stream_count == 0);
@@ -157,7 +157,7 @@ static void create_kernel_event(void)
        PRINT_OK();
 
        printf("Validating kernel event: ");
-       assert(event->fd == 0);
+       assert(event->fd == -1);
        assert(event->enabled == 1);
        assert(event->ctx == NULL);
        assert(event->event->instrumentation == LTTNG_KERNEL_TRACEPOINT);
@@ -179,7 +179,7 @@ static void create_kernel_stream(void)
        PRINT_OK();
 
        printf("Validating kernel stream: ");
-       assert(stream->fd == 0);
+       assert(stream->fd == -1);
        assert(stream->pathname == NULL);
        assert(stream->state == 0);
        PRINT_OK();