tmpfiles: port from dirname/basename to path_extract_directory/filename()

let's use our better, newer internal APIs for these purposes. This gets us
two things: safer handling when the root dir is specified, and better
handling of paths with trailing slashes, as we can refuse them whenever
a directory is not acceptable.

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 6aaf31867d90c63f6e05cffd869ce484249a079a..05e3b99590b535788340d6e0edbed347f2c1a314 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -930,20 +930,20 @@ static int path_open_parent_safe(const char *path) {
         _cleanup_free_ char *dn = NULL;
         int r, fd;
 
-        if (path_equal(path, "/") || !path_is_normalized(path))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Failed to open parent of '%s': invalid path.",
-                                       path);
+        if (!path_is_normalized(path))
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to open parent of '%s': path not normalized.", path);
 
-        dn = dirname_malloc(path);
-        if (!dn)
-                return log_oom();
+        r = path_extract_directory(path, &dn);
+        if (r < 0)
+                return log_error_errno(r, "Unable to determine parent directory of '%s': %m", path);
 
         r = chase_symlinks(dn, arg_root, CHASE_SAFE|CHASE_WARN, NULL, &fd);
-        if (r < 0 && r != -ENOLINK)
-                return log_error_errno(r, "Failed to validate path %s: %m", path);
+        if (r == -ENOLINK) /* Unsafe symlink: already covered by CHASE_WARN */
+                return r;
+        if (r < 0)
+                return log_error_errno(r, "Failed to open path '%s': %m", dn);
 
-        return r < 0 ? r : fd;
+        return fd;
 }
 
 static int path_open_safe(const char *path) {
@@ -956,15 +956,15 @@ static int path_open_safe(const char *path) {
         assert(path);
 
         if (!path_is_normalized(path))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Failed to open invalid path '%s'.",
-                                       path);
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to open invalid path '%s'.", path);
 
         r = chase_symlinks(path, arg_root, CHASE_SAFE|CHASE_WARN|CHASE_NOFOLLOW, NULL, &fd);
-        if (r < 0 && r != -ENOLINK)
-                return log_error_errno(r, "Failed to validate path %s: %m", path);
+        if (r == -ENOLINK)
+                return r; /* Unsafe symlink: already covered by CHASE_WARN */
+        if (r < 0)
+                return log_error_errno(r, "Failed to open path %s: %m", path);
 
-        return r < 0 ? r : fd;
+        return fd;
 }
 
 static int path_set_perms(Item *i, const char *path) {
@@ -1331,7 +1331,7 @@ static int path_set_attribute(Item *item, const char *path) {
 
 static int write_one_file(Item *i, const char *path) {
         _cleanup_close_ int fd = -1, dir_fd = -1;
-        char *bn;
+        _cleanup_free_ char *bn = NULL;
         int r;
 
         assert(i);
@@ -1339,14 +1339,18 @@ static int write_one_file(Item *i, const char *path) {
         assert(i->argument);
         assert(i->type == WRITE_FILE);
 
-        /* Validate the path and keep the fd on the directory for opening the
-         * file so we're sure that it can't be changed behind our back. */
+        r = path_extract_filename(path, &bn);
+        if (r < 0)
+                return log_error_errno(r, "Failed to extract filename from path '%s': %m", path);
+        if (r == O_DIRECTORY)
+                return log_error_errno(SYNTHETIC_ERRNO(EISDIR), "Cannot open path '%s' for writing, is a directory.", path);
+
+        /* Validate the path and keep the fd on the directory for opening the file so we're sure that it
+         * can't be changed behind our back. */
         dir_fd = path_open_parent_safe(path);
         if (dir_fd < 0)
                 return dir_fd;
 
-        bn = basename(path);
-
         /* Follows symlinks */
         fd = openat(dir_fd, bn,
                     O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY|(i->append_or_force ? O_APPEND : 0),
@@ -1375,9 +1379,9 @@ static int write_one_file(Item *i, const char *path) {
 
 static int create_file(Item *i, const char *path) {
         _cleanup_close_ int fd = -1, dir_fd = -1;
+        _cleanup_free_ char *bn = NULL;
         struct stat stbuf, *st = NULL;
         int r = 0;
-        char *bn;
 
         assert(i);
         assert(path);
@@ -1385,14 +1389,18 @@ static int create_file(Item *i, const char *path) {
 
         /* 'f' operates on regular files exclusively. */
 
-        /* Validate the path and keep the fd on the directory for opening the
-         * file so we're sure that it can't be changed behind our back. */
+        r = path_extract_filename(path, &bn);
+        if (r < 0)
+                return log_error_errno(r, "Failed to extract filename from path '%s': %m", path);
+        if (r == O_DIRECTORY)
+                return log_error_errno(SYNTHETIC_ERRNO(EISDIR), "Cannot open path '%s' for writing, is a directory.", path);
+
+        /* Validate the path and keep the fd on the directory for opening the file so we're sure that it
+         * can't be changed behind our back. */
         dir_fd = path_open_parent_safe(path);
         if (dir_fd < 0)
                 return dir_fd;
 
-        bn = basename(path);
-
         RUN_WITH_UMASK(0000) {
                 mac_selinux_create_file_prepare(path, S_IFREG);
                 fd = openat(dir_fd, bn, O_CREAT|O_EXCL|O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode);
@@ -1442,28 +1450,31 @@ static int create_file(Item *i, const char *path) {
 
 static int truncate_file(Item *i, const char *path) {
         _cleanup_close_ int fd = -1, dir_fd = -1;
+        _cleanup_free_ char *bn = NULL;
         struct stat stbuf, *st = NULL;
         bool erofs = false;
         int r = 0;
-        char *bn;
 
         assert(i);
         assert(path);
         assert(i->type == TRUNCATE_FILE || (i->type == CREATE_FILE && i->append_or_force));
 
-        /* We want to operate on regular file exclusively especially since
-         * O_TRUNC is unspecified if the file is neither a regular file nor a
-         * fifo nor a terminal device. Therefore we first open the file and make
-         * sure it's a regular one before truncating it. */
+        /* We want to operate on regular file exclusively especially since O_TRUNC is unspecified if the file
+         * is neither a regular file nor a fifo nor a terminal device. Therefore we first open the file and
+         * make sure it's a regular one before truncating it. */
 
-        /* Validate the path and keep the fd on the directory for opening the
-         * file so we're sure that it can't be changed behind our back. */
+        r = path_extract_filename(path, &bn);
+        if (r < 0)
+                return log_error_errno(r, "Failed to extract filename from path '%s': %m", path);
+        if (r == O_DIRECTORY)
+                return log_error_errno(SYNTHETIC_ERRNO(EISDIR), "Cannot open path '%s' for truncation, is a directory.", path);
+
+        /* Validate the path and keep the fd on the directory for opening the file so we're sure that it
+         * can't be changed behind our back. */
         dir_fd = path_open_parent_safe(path);
         if (dir_fd < 0)
                 return dir_fd;
 
-        bn = basename(path);
-
         RUN_WITH_UMASK(0000) {
                 mac_selinux_create_file_prepare(path, S_IFREG);
                 fd = openat(dir_fd, bn, O_CREAT|O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode);
@@ -1526,16 +1537,17 @@ static int truncate_file(Item *i, const char *path) {
 
 static int copy_files(Item *i) {
         _cleanup_close_ int dfd = -1, fd = -1;
-        char *bn;
+        _cleanup_free_ char *bn = NULL;
         int r;
 
         log_debug("Copying tree \"%s\" to \"%s\".", i->argument, i->path);
 
-        bn = basename(i->path);
+        r = path_extract_filename(i->path, &bn);
+        if (r < 0)
+                return log_error_errno(r, "Failed to extract filename from path '%s': %m", i->path);
 
-        /* Validate the path and use the returned directory fd for copying the
-         * target so we're sure that the path can't be changed behind our
-         * back. */
+        /* Validate the path and use the returned directory fd for copying the target so we're sure that the
+         * path can't be changed behind our back. */
         dfd = path_open_parent_safe(i->path);
         if (dfd < 0)
                 return dfd;
@@ -1593,6 +1605,7 @@ static const char *const creation_mode_verb_table[_CREATION_MODE_MAX] = {
 DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(creation_mode_verb, CreationMode);
 
 static int create_directory_or_subvolume(const char *path, mode_t mode, bool subvol, CreationMode *creation) {
+        _cleanup_free_ char *bn = NULL;
         _cleanup_close_ int pfd = -1;
         CreationMode c;
         int r;
@@ -1602,6 +1615,10 @@ static int create_directory_or_subvolume(const char *path, mode_t mode, bool sub
         if (!creation)
                 creation = &c;
 
+        r = path_extract_filename(path, &bn);
+        if (r < 0)
+                return log_error_errno(r, "Failed to extract filename from path '%s': %m", path);
+
         pfd = path_open_parent_safe(path);
         if (pfd < 0)
                 return pfd;
@@ -1627,14 +1644,14 @@ static int create_directory_or_subvolume(const char *path, mode_t mode, bool sub
                         subvol = false;
                 else {
                         RUN_WITH_UMASK((~mode) & 0777)
-                                r = btrfs_subvol_make_fd(pfd, basename(path));
+                                r = btrfs_subvol_make_fd(pfd, bn);
                 }
         } else
                 r = 0;
 
         if (!subvol || r == -ENOTTY)
                 RUN_WITH_UMASK(0000)
-                        r = mkdirat_label(pfd, basename(path), mode);
+                        r = mkdirat_label(pfd, bn, mode);
 
         if (r < 0) {
                 int k;
@@ -1657,9 +1674,9 @@ static int create_directory_or_subvolume(const char *path, mode_t mode, bool sub
 
         log_debug("%s directory \"%s\".", creation_mode_verb_to_string(*creation), path);
 
-        r = openat(pfd, basename(path), O_NOCTTY|O_CLOEXEC|O_DIRECTORY);
+        r = openat(pfd, bn, O_NOCTTY|O_CLOEXEC|O_DIRECTORY);
         if (r < 0)
-                return log_error_errno(errno, "Failed to open directory '%s': %m", basename(path));
+                return log_error_errno(errno, "Failed to open directory '%s': %m", bn);
 
         return r;
 }
@@ -1742,18 +1759,21 @@ static int empty_directory(Item *i, const char *path) {
 
 static int create_device(Item *i, mode_t file_type) {
         _cleanup_close_ int dfd = -1, fd = -1;
+        _cleanup_free_ char *bn = NULL;
         CreationMode creation;
-        char *bn;
         int r;
 
         assert(i);
         assert(IN_SET(file_type, S_IFBLK, S_IFCHR));
 
-        bn = basename(i->path);
+        r = path_extract_filename(i->path, &bn);
+        if (r < 0)
+                return log_error_errno(r, "Failed to extract filename from path '%s': %m", i->path);
+        if (r == O_DIRECTORY)
+                return log_error_errno(SYNTHETIC_ERRNO(EISDIR), "Cannot open path '%s' for creating device node, is a directory.", i->path);
 
-        /* Validate the path and use the returned directory fd for copying the
-         * target so we're sure that the path can't be changed behind our
-         * back. */
+        /* Validate the path and use the returned directory fd for copying the target so we're sure that the
+         * path can't be changed behind our back. */
         dfd = path_open_parent_safe(i->path);
         if (dfd < 0)
                 return dfd;
@@ -1816,17 +1836,21 @@ static int create_device(Item *i, mode_t file_type) {
 
 static int create_fifo(Item *i, const char *path) {
         _cleanup_close_ int pfd = -1, fd = -1;
+        _cleanup_free_ char *bn = NULL;
         CreationMode creation;
         struct stat st;
-        char *bn;
         int r;
 
+        r = path_extract_filename(i->path, &bn);
+        if (r < 0)
+                return log_error_errno(r, "Failed to extract filename from path '%s': %m", path);
+        if (r == O_DIRECTORY)
+                return log_error_errno(SYNTHETIC_ERRNO(EISDIR), "Cannot open path '%s' for creating FIFO, is a directory.", path);
+
         pfd = path_open_parent_safe(path);
         if (pfd < 0)
                 return pfd;
 
-        bn = basename(path);
-
         RUN_WITH_UMASK(0000) {
                 mac_selinux_create_file_prepare(path, S_IFIFO);
                 r = mkfifoat(pfd, bn, i->mode);