Fix: possible unlink on uninitialized buffer path

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

diff --git a/src/bin/lttng-sessiond/save.c b/src/bin/lttng-sessiond/save.c
index 8599d2d18a6796bc3c6ba90017f4ea55420c0d6d..22fab9c05bd013020a83c368fc6a0cadf7ad09e3 100644 (file)
--- a/src/bin/lttng-sessiond/save.c
+++ b/src/bin/lttng-sessiond/save.c
@@ -1239,6 +1239,7 @@ end_net_output:
                        ret = !output->dst.net.control_isset ?
                                LTTNG_ERR_URL_CTRL_MISS :
                                LTTNG_ERR_URL_DATA_MISS;
+                       free(uri);
                        goto end;
                }
 
@@ -1397,6 +1398,7 @@ int save_session(struct ltt_session *session,
        struct lttng_save_session_attr *attr, lttng_sock_cred *creds)
 {
        int ret, fd;
+       unsigned int file_opened = 0;   /* Indicate if the file has been opened */
        char config_file_path[PATH_MAX];
        size_t len;
        struct config_writer *writer = NULL;
@@ -1419,12 +1421,13 @@ int save_session(struct ltt_session *session,
        provided_path = lttng_save_session_attr_get_output_url(attr);
        if (provided_path) {
                len = strlen(provided_path);
-               if (len > PATH_MAX) {
+               if (len >= sizeof(config_file_path)) {
                        ret = LTTNG_ERR_SET_URL;
                        goto end;
                }
                strncpy(config_file_path, provided_path, len);
        } else {
+               ssize_t ret_len;
                char *home_dir = utils_get_user_home_dir(
                        LTTNG_SOCK_GET_UID_CRED(creds));
                if (!home_dir) {
@@ -1432,22 +1435,24 @@ int save_session(struct ltt_session *session,
                        goto end;
                }
 
-               len = snprintf(config_file_path, PATH_MAX,
+               ret_len = snprintf(config_file_path, sizeof(config_file_path),
                                DEFAULT_SESSION_HOME_CONFIGPATH, home_dir);
                free(home_dir);
-               if (len < 0) {
+               if (ret_len < 0) {
                        PERROR("snprintf save session");
                        ret = LTTNG_ERR_SET_URL;
                        goto end;
                }
+               len = ret_len;
        }
 
        /*
-        * Check the path fits in PATH_MAX, including the / followed by trailing
-        * .lttng extension and the NULL terminated string.
+        * Check the path fits in the config file path dst including the '/'
+        * followed by trailing .lttng extension and the NULL terminated string.
         */
-       if (len + session_name_len + 2 +
-                       sizeof(DEFAULT_SESSION_CONFIG_FILE_EXTENSION) > PATH_MAX) {
+       if ((len + session_name_len + 2 +
+                       sizeof(DEFAULT_SESSION_CONFIG_FILE_EXTENSION))
+                       > sizeof(config_file_path)) {
                ret = LTTNG_ERR_SET_URL;
                goto end;
        }
@@ -1459,6 +1464,10 @@ int save_session(struct ltt_session *session,
                goto end;
        }
 
+       /*
+        * At this point, we know that everything fits in the buffer. Validation
+        * was done just above.
+        */
        config_file_path[len++] = '/';
        strncpy(config_file_path + len, session->name, session_name_len);
        len += session_name_len;
@@ -1477,6 +1486,7 @@ int save_session(struct ltt_session *session,
                ret = LTTNG_ERR_SAVE_IO_FAIL;
                goto end;
        }
+       file_opened = 1;
 
        writer = config_writer_create(fd);
        if (!writer) {
@@ -1571,7 +1581,7 @@ end:
        }
        if (ret) {
                /* Delete file in case of error */
-               if (unlink(config_file_path)) {
+               if (file_opened && unlink(config_file_path)) {
                        PERROR("Unlinking XML session configuration.");
                }
        }