Fix: possible NULL deref. in sessiond error path
[lttng-tools.git] / src / bin / lttng-sessiond / main.c
index 195174a4edf259d24e2e0d523f0dd243805e4f01..0fb80fa44cb7e07477a3d61527fedc169fe5d776 100644 (file)
@@ -1014,7 +1014,6 @@ restart:
        }
 
        health_code_update();
-
        if (code == LTTCOMM_CONSUMERD_COMMAND_SOCK_READY) {
                /* Connect both socket, command and metadata. */
                consumer_data->cmd_sock =
@@ -1171,13 +1170,13 @@ error:
                }
                consumer_data->cmd_sock = -1;
        }
-       if (*consumer_data->metadata_sock.fd_ptr >= 0) {
+       if (consumer_data->metadata_sock.fd_ptr &&
+           *consumer_data->metadata_sock.fd_ptr >= 0) {
                ret = close(*consumer_data->metadata_sock.fd_ptr);
                if (ret) {
                        PERROR("close");
                }
        }
-
        if (sock >= 0) {
                ret = close(sock);
                if (ret) {
@@ -1191,9 +1190,10 @@ error:
        pthread_mutex_unlock(&consumer_data->lock);
 
        /* Cleanup metadata socket mutex. */
-       pthread_mutex_destroy(consumer_data->metadata_sock.lock);
-       free(consumer_data->metadata_sock.lock);
-
+       if (consumer_data->metadata_sock.lock) {
+               pthread_mutex_destroy(consumer_data->metadata_sock.lock);
+               free(consumer_data->metadata_sock.lock);
+       }
        lttng_poll_clean(&events);
 error_poll:
        if (err) {
@@ -1308,23 +1308,7 @@ static void *thread_manage_apps(void *data)
                                                goto error;
                                        }
 
-                                       /*
-                                        * Set socket timeout for both receiving and ending.
-                                        * app_socket_timeout is in seconds, whereas
-                                        * lttcomm_setsockopt_rcv_timeout and
-                                        * lttcomm_setsockopt_snd_timeout expect msec as
-                                        * parameter.
-                                        */
-                                       (void) lttcomm_setsockopt_rcv_timeout(sock,
-                                                       app_socket_timeout * 1000);
-                                       (void) lttcomm_setsockopt_snd_timeout(sock,
-                                                       app_socket_timeout * 1000);
-
                                        DBG("Apps with sock %d added to poll set", sock);
-
-                                       health_code_update();
-
-                                       break;
                                }
                        } else {
                                /*
@@ -1340,7 +1324,6 @@ static void *thread_manage_apps(void *data)
 
                                        /* Socket closed on remote end. */
                                        ust_app_unregister(pollfd);
-                                       break;
                                }
                        }
 
@@ -1377,6 +1360,9 @@ error_testpoint:
  * Send a socket to a thread This is called from the dispatch UST registration
  * thread once all sockets are set for the application.
  *
+ * The sock value can be invalid, we don't really care, the thread will handle
+ * it and make the necessary cleanup if so.
+ *
  * On success, return 0 else a negative value being the errno message of the
  * write().
  */
@@ -1384,9 +1370,14 @@ static int send_socket_to_thread(int fd, int sock)
 {
        int ret;
 
-       /* Sockets MUST be set or else this should not have been called. */
-       assert(fd >= 0);
-       assert(sock >= 0);
+       /*
+        * It's possible that the FD is set as invalid with -1 concurrently just
+        * before calling this function being a shutdown state of the thread.
+        */
+       if (fd < 0) {
+               ret = -EBADF;
+               goto error;
+       }
 
        do {
                ret = write(fd, &sock, sizeof(sock));
@@ -1555,7 +1546,7 @@ static void *thread_dispatch_ust_registration(void *data)
                                        if (ret < 0) {
                                                PERROR("close ust sock dispatch %d", ust_cmd->sock);
                                        }
-                                       lttng_fd_put(1, LTTNG_FD_APPS);
+                                       lttng_fd_put(LTTNG_FD_APPS, 1);
                                        free(ust_cmd);
                                        goto error;
                                }
@@ -1569,7 +1560,7 @@ static void *thread_dispatch_ust_registration(void *data)
                                        if (ret < 0) {
                                                PERROR("close ust sock dispatch %d", ust_cmd->sock);
                                        }
-                                       lttng_fd_put(1, LTTNG_FD_APPS);
+                                       lttng_fd_put(LTTNG_FD_APPS, 1);
                                        free(wait_node);
                                        free(ust_cmd);
                                        continue;
@@ -1617,7 +1608,7 @@ static void *thread_dispatch_ust_registration(void *data)
                                        if (ret < 0) {
                                                PERROR("close ust sock dispatch %d", ust_cmd->sock);
                                        }
-                                       lttng_fd_put(1, LTTNG_FD_APPS);
+                                       lttng_fd_put(LTTNG_FD_APPS, 1);
                                }
                                free(ust_cmd);
                        }
@@ -1649,7 +1640,12 @@ static void *thread_dispatch_ust_registration(void *data)
                                if (ret < 0) {
                                        rcu_read_unlock();
                                        session_unlock_list();
-                                       /* No notify thread, stop the UST tracing. */
+                                       /*
+                                        * No notify thread, stop the UST tracing. However, this is
+                                        * not an internal error of the this thread thus setting
+                                        * the health error code to a normal exit.
+                                        */
+                                       err = 0;
                                        goto error;
                                }
 
@@ -1674,7 +1670,12 @@ static void *thread_dispatch_ust_registration(void *data)
                                if (ret < 0) {
                                        rcu_read_unlock();
                                        session_unlock_list();
-                                       /* No apps. thread, stop the UST tracing. */
+                                       /*
+                                        * No apps. thread, stop the UST tracing. However, this is
+                                        * not an internal error of the this thread thus setting
+                                        * the health error code to a normal exit.
+                                        */
+                                       err = 0;
                                        goto error;
                                }
 
@@ -1804,6 +1805,18 @@ static void *thread_registration_apps(void *data)
                                                goto error;
                                        }
 
+                                       /*
+                                        * Set socket timeout for both receiving and ending.
+                                        * app_socket_timeout is in seconds, whereas
+                                        * lttcomm_setsockopt_rcv_timeout and
+                                        * lttcomm_setsockopt_snd_timeout expect msec as
+                                        * parameter.
+                                        */
+                                       (void) lttcomm_setsockopt_rcv_timeout(sock,
+                                                       app_socket_timeout * 1000);
+                                       (void) lttcomm_setsockopt_snd_timeout(sock,
+                                                       app_socket_timeout * 1000);
+
                                        /*
                                         * Set the CLOEXEC flag. Return code is useless because
                                         * either way, the show must go on.
@@ -2001,19 +2014,23 @@ static int spawn_consumer_thread(struct consumer_data *consumer_data)
        if (ret != 0) {
                errno = ret;
                if (ret == ETIMEDOUT) {
+                       int pth_ret;
+
                        /*
                         * Call has timed out so we kill the kconsumerd_thread and return
                         * an error.
                         */
                        ERR("Condition timed out. The consumer thread was never ready."
                                        " Killing it");
-                       ret = pthread_cancel(consumer_data->thread);
-                       if (ret < 0) {
+                       pth_ret = pthread_cancel(consumer_data->thread);
+                       if (pth_ret < 0) {
                                PERROR("pthread_cancel consumer thread");
                        }
                } else {
                        PERROR("pthread_cond_wait failed consumer thread");
                }
+               /* Caller is expecting a negative value on failure. */
+               ret = -1;
                goto error;
        }
 
@@ -2099,10 +2116,11 @@ static pid_t spawn_consumerd(struct consumer_data *consumer_data)
                                consumer_to_use = consumerd32_bin;
                        } else {
                                DBG("Could not find any valid consumerd executable");
+                               ret = -EINVAL;
                                break;
                        }
                        DBG("Using kernel consumer at: %s",  consumer_to_use);
-                       execl(consumer_to_use,
+                       ret = execl(consumer_to_use,
                                "lttng-consumerd", verbosity, "-k",
                                "--consumerd-cmd-sock", consumer_data->cmd_unix_sock_path,
                                "--consumerd-err-sock", consumer_data->err_unix_sock_path,
@@ -2148,9 +2166,6 @@ static pid_t spawn_consumerd(struct consumer_data *consumer_data)
                        if (consumerd64_libdir[0] != '\0') {
                                free(tmpnew);
                        }
-                       if (ret) {
-                               goto error;
-                       }
                        break;
                }
                case LTTNG_CONSUMER32_UST:
@@ -2193,9 +2208,6 @@ static pid_t spawn_consumerd(struct consumer_data *consumer_data)
                        if (consumerd32_libdir[0] != '\0') {
                                free(tmpnew);
                        }
-                       if (ret) {
-                               goto error;
-                       }
                        break;
                }
                default:
@@ -2203,8 +2215,9 @@ static pid_t spawn_consumerd(struct consumer_data *consumer_data)
                        exit(EXIT_FAILURE);
                }
                if (errno != 0) {
-                       PERROR("kernel start consumer exec");
+                       PERROR("Consumer execl()");
                }
+               /* Reaching this point, we got a failure on our execl(). */
                exit(EXIT_FAILURE);
        } else if (pid > 0) {
                ret = pid;
@@ -3859,7 +3872,7 @@ static void usage(void)
        fprintf(stderr, "  -d, --daemonize                    Start as a daemon.\n");
        fprintf(stderr, "  -g, --group NAME                   Specify the tracing group name. (default: tracing)\n");
        fprintf(stderr, "  -V, --version                      Show version number.\n");
-       fprintf(stderr, "  -S, --sig-parent                   Send SIGCHLD to parent pid to notify readiness.\n");
+       fprintf(stderr, "  -S, --sig-parent                   Send SIGUSR1 to parent pid to notify readiness.\n");
        fprintf(stderr, "  -q, --quiet                        No output at all.\n");
        fprintf(stderr, "  -v, --verbose                      Verbose mode. Activate DBG() macro.\n");
        fprintf(stderr, "  -p, --pidfile FILE                 Write a pid to FILE name overriding the default value.\n");
@@ -4652,6 +4665,14 @@ int main(int argc, char **argv)
 
        /* Initialize communication library */
        lttcomm_init();
+       /* This is to get the TCP timeout value. */
+       lttcomm_inet_init();
+
+       /*
+        * Initialize the health check subsystem. This call should set the
+        * appropriate time values.
+        */
+       health_init();
 
        /* Create thread to manage the client socket */
        ret = pthread_create(&ht_cleanup_thread, NULL,
This page took 0.027831 seconds and 4 git commands to generate.