Fix: Consumerd error socket connect race

[lttng-tools.git] / src / bin / lttng-sessiond / main.c

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index ce2213c1bd2897e46df7b38f822e9e3cb061a20e..022bbc6d1dc5b293974166f0a92bd04818ba29e2 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -857,13 +857,6 @@ static void *thread_manage_consumer(void *data)
         */
        health_poll_update(&consumer_data->health);
 
-       health_code_update(&consumer_data->health);
-
-       ret = lttcomm_listen_unix_sock(consumer_data->err_sock);
-       if (ret < 0) {
-               goto error_listen;
-       }
-
        /*
         * Pass 2 as size here for the thread quit pipe and kconsumerd_err_sock.
         * Nothing more will be added to this poll set.
@@ -873,6 +866,11 @@ static void *thread_manage_consumer(void *data)
                goto error_poll;
        }
 
+       /*
+        * The error socket here is already in a listening state which was done
+        * just before spawning this thread to avoid a race between the consumer
+        * daemon exec trying to connect and the listen() call.
+        */
        ret = lttng_poll_add(&events, consumer_data->err_sock, LPOLLIN | LPOLLRDHUP);
        if (ret < 0) {
                goto error;
@@ -928,6 +926,12 @@ restart:
                goto error;
        }
 
+       /*
+        * Set the CLOEXEC flag. Return code is useless because either way, the
+        * show must go on.
+        */
+       (void) utils_set_fd_cloexec(sock);
+
        health_code_update(&consumer_data->health);
 
        DBG2("Receiving code from consumer err_sock");
@@ -1062,7 +1066,6 @@ error:
 
        lttng_poll_clean(&events);
 error_poll:
-error_listen:
        if (err) {
                health_error(&consumer_data->health);
                ERR("Health error occurred in %s", __func__);
@@ -1402,6 +1405,12 @@ static void *thread_registration_apps(void *data)
                                                goto error;
                                        }
 
+                                       /*
+                                        * Set the CLOEXEC flag. Return code is useless because
+                                        * either way, the show must go on.
+                                        */
+                                       (void) utils_set_fd_cloexec(sock);
+
                                        /* Create UST registration command for enqueuing */
                                        ust_cmd = zmalloc(sizeof(struct ust_command));
                                        if (ust_cmd == NULL) {
@@ -1814,7 +1823,17 @@ error:
  */
 static int start_consumerd(struct consumer_data *consumer_data)
 {
-       int ret;
+       int ret, err;
+
+       /*
+        * Set the listen() state on the socket since there is a possible race
+        * between the exec() of the consumer daemon and this call if place in the
+        * consumer thread. See bug #366 for more details.
+        */
+       ret = lttcomm_listen_unix_sock(consumer_data->err_sock);
+       if (ret < 0) {
+               goto error;
+       }
 
        pthread_mutex_lock(&consumer_data->pid_mutex);
        if (consumer_data->pid != 0) {
@@ -1845,6 +1864,13 @@ end:
        return 0;
 
 error:
+       /* Cleanup already created socket on error. */
+       if (consumer_data->err_sock >= 0) {
+               err = close(consumer_data->err_sock);
+               if (err < 0) {
+                       PERROR("close consumer data error socket");
+               }
+       }
        return ret;
 }
 
@@ -2882,6 +2908,12 @@ static void *thread_manage_health(void *data)
                goto error;
        }
 
+       /*
+        * Set the CLOEXEC flag. Return code is useless because either way, the
+        * show must go on.
+        */
+       (void) utils_set_fd_cloexec(sock);
+
        ret = lttcomm_listen_unix_sock(sock);
        if (ret < 0) {
                goto error;
@@ -2946,6 +2978,12 @@ restart:
                        goto error;
                }
 
+               /*
+                * Set the CLOEXEC flag. Return code is useless because either way, the
+                * show must go on.
+                */
+               (void) utils_set_fd_cloexec(new_sock);
+
                DBG("Receiving data from client for health...");
                ret = lttcomm_recv_unix_sock(new_sock, (void *)&msg, sizeof(msg));
                if (ret <= 0) {
@@ -3143,6 +3181,12 @@ static void *thread_manage_clients(void *data)
                        goto error;
                }
 
+               /*
+                * Set the CLOEXEC flag. Return code is useless because either way, the
+                * show must go on.
+                */
+               (void) utils_set_fd_cloexec(sock);
+
                /* Set socket option for credentials retrieval */
                ret = lttcomm_setsockopt_creds_unix_sock(sock);
                if (ret < 0) {
@@ -3443,6 +3487,14 @@ static int init_daemon_socket(void)
                goto end;
        }
 
+       /* Set the cloexec flag */
+       ret = utils_set_fd_cloexec(client_sock);
+       if (ret < 0) {
+               ERR("Unable to set CLOEXEC flag to the client Unix socket (fd: %d). "
+                               "Continuing but note that the consumer daemon will have a "
+                               "reference to this socket on exec()", client_sock);
+       }
+
        /* File permission MUST be 660 */
        ret = chmod(client_unix_sock_path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
        if (ret < 0) {
@@ -3459,6 +3511,14 @@ static int init_daemon_socket(void)
                goto end;
        }
 
+       /* Set the cloexec flag */
+       ret = utils_set_fd_cloexec(apps_sock);
+       if (ret < 0) {
+               ERR("Unable to set CLOEXEC flag to the app Unix socket (fd: %d). "
+                               "Continuing but note that the consumer daemon will have a "
+                               "reference to this socket on exec()", apps_sock);
+       }
+
        /* File permission MUST be 666 */
        ret = chmod(apps_unix_sock_path,
                        S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
@@ -3468,6 +3528,9 @@ static int init_daemon_socket(void)
                goto end;
        }
 
+       DBG3("Session daemon client socket %d and application socket %d created",
+                       client_sock, apps_sock);
+
 end:
        umask(old_umask);
        return ret;