Fix: prevent dangling pointer in utils_partial_realpath

[lttng-tools.git] / src / common / utils.c

diff --git a/src/common/utils.c b/src/common/utils.c
index db2ed8e7d11dee928a6f96be4167a94669f015c6..9fcceab1f3149c2c5e17a3e68d4ee1b60fc4932c 100644 (file)
--- a/src/common/utils.c
+++ b/src/common/utils.c
@@ -55,7 +55,7 @@
 LTTNG_HIDDEN
 char *utils_partial_realpath(const char *path, char *resolved_path, size_t size)
 {
-       char *cut_path, *try_path = NULL, *try_path_prev = NULL;
+       char *cut_path = NULL, *try_path = NULL, *try_path_prev = NULL;
        const char *next, *prev, *end;
 
        /* Safety net */
@@ -124,6 +124,7 @@ char *utils_partial_realpath(const char *path, char *resolved_path, size_t size)
 
                /* Free the allocated memory */
                free(cut_path);
+               cut_path = NULL;
        };
 
        /* Allocate memory for the resolved path if necessary */
@@ -174,6 +175,7 @@ char *utils_partial_realpath(const char *path, char *resolved_path, size_t size)
 
 error:
        free(resolved_path);
+       free(cut_path);
        return NULL;
 }
 
@@ -649,22 +651,20 @@ int utils_mkdir_recursive(const char *path, mode_t mode, int uid, int gid)
 }
 
 /*
- * Create the stream tracefile on disk.
  * path is the output parameter. It needs to be PATH_MAX len.
  *
  * Return 0 on success or else a negative value.
  */
-LTTNG_HIDDEN
-int utils_create_stream_file(const char *path_name, char *file_name, uint64_t size,
-               uint64_t count, int uid, int gid, char *suffix)
+static int utils_stream_file_name(char *path,
+               const char *path_name, const char *file_name,
+               uint64_t size, uint64_t count,
+               const char *suffix)
 {
-       int ret, out_fd, flags, mode;
-       char full_path[PATH_MAX], *path_name_suffix = NULL, *path;
+       int ret;
+       char full_path[PATH_MAX];
+       char *path_name_suffix = NULL;
        char *extra = NULL;
 
-       assert(path_name);
-       assert(file_name);
-
        ret = snprintf(full_path, sizeof(full_path), "%s/%s",
                        path_name, file_name);
        if (ret < 0) {
@@ -686,8 +686,8 @@ int utils_create_stream_file(const char *path_name, char *file_name, uint64_t si
        }
 
        /*
-        * If we split the trace in multiple files, we have to add the count at the
-        * end of the tracefile name
+        * If we split the trace in multiple files, we have to add the count at
+        * the end of the tracefile name.
         */
        if (extra) {
                ret = asprintf(&path_name_suffix, "%s%s", full_path, extra);
@@ -695,9 +695,37 @@ int utils_create_stream_file(const char *path_name, char *file_name, uint64_t si
                        PERROR("Allocating path name with extra string");
                        goto error_free_suffix;
                }
-               path = path_name_suffix;
+               strncpy(path, path_name_suffix, PATH_MAX - 1);
+               path[PATH_MAX - 1] = '\0';
        } else {
-               path = full_path;
+               strncpy(path, full_path, PATH_MAX - 1);
+       }
+       path[PATH_MAX - 1] = '\0';
+       ret = 0;
+
+       free(path_name_suffix);
+error_free_suffix:
+       free(extra);
+error:
+       return ret;
+}
+
+/*
+ * Create the stream file on disk.
+ *
+ * Return 0 on success or else a negative value.
+ */
+LTTNG_HIDDEN
+int utils_create_stream_file(const char *path_name, char *file_name, uint64_t size,
+               uint64_t count, int uid, int gid, char *suffix)
+{
+       int ret, flags, mode;
+       char path[PATH_MAX];
+
+       ret = utils_stream_file_name(path, path_name, file_name,
+                       size, count, suffix);
+       if (ret < 0) {
+               goto error;
        }
 
        flags = O_WRONLY | O_CREAT | O_TRUNC;
@@ -705,21 +733,44 @@ int utils_create_stream_file(const char *path_name, char *file_name, uint64_t si
        mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP;
 
        if (uid < 0 || gid < 0) {
-               out_fd = open(path, flags, mode);
+               ret = open(path, flags, mode);
        } else {
-               out_fd = run_as_open(path, flags, mode, uid, gid);
+               ret = run_as_open(path, flags, mode, uid, gid);
        }
-       if (out_fd < 0) {
+       if (ret < 0) {
                PERROR("open stream path %s", path);
-               goto error_open;
        }
-       ret = out_fd;
+error:
+       return ret;
+}
 
-error_open:
-       free(path_name_suffix);
-error_free_suffix:
-       free(extra);
+/*
+ * Unlink the stream tracefile from disk.
+ *
+ * Return 0 on success or else a negative value.
+ */
+LTTNG_HIDDEN
+int utils_unlink_stream_file(const char *path_name, char *file_name, uint64_t size,
+               uint64_t count, int uid, int gid, char *suffix)
+{
+       int ret;
+       char path[PATH_MAX];
+
+       ret = utils_stream_file_name(path, path_name, file_name,
+                       size, count, suffix);
+       if (ret < 0) {
+               goto error;
+       }
+       if (uid < 0 || gid < 0) {
+               ret = unlink(path);
+       } else {
+               ret = run_as_unlink(path, uid, gid);
+       }
+       if (ret < 0) {
+               goto error;
+       }
 error:
+       DBG("utils_unlink_stream_file %s returns %d", path, ret);
        return ret;
 }
 
@@ -749,7 +800,25 @@ int utils_rotate_stream_file(char *path_name, char *file_name, uint64_t size,
        }
 
        if (count > 0) {
+               /*
+                * In tracefile rotation, for the relay daemon we need
+                * to unlink the old file if present, because it may
+                * still be open in reading by the live thread, and we
+                * need to ensure that we do not overwrite the content
+                * between get_index and get_packet. Since we have no
+                * way to verify integrity of the data content compared
+                * to the associated index, we need to ensure the reader
+                * has exclusive access to the file content, and that
+                * the open of the data file is performed in get_index.
+                * Unlinking the old file rather than overwriting it
+                * achieves this.
+                */
                *new_count = (*new_count + 1) % count;
+               ret = utils_unlink_stream_file(path_name, file_name,
+                               size, *new_count, uid, gid, 0);
+               if (ret < 0 && errno != ENOENT) {
+                       goto error;
+               }
        } else {
                (*new_count)++;
        }