Merge pull request #9668 from poettering/open-parent

introduce open_parent() helper

5 files changed, 59 insertions, 51 deletions

diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c
index 6d2490f3d7..efac0b9420 100644
--- a/src/basic/btrfs-util.c
+++ b/src/basic/btrfs-util.c
@@ -28,6 +28,7 @@
 #include "device-nodes.h"
 #include "fd-util.h"
 #include "fileio.h"
+#include "fs-util.h"
 #include "io-util.h"
 #include "macro.h"
 #include "missing.h"
@@ -59,23 +60,6 @@ static int validate_subvolume_name(const char *name) {
         return 0;
 }
 
-static int open_parent(const char *path, int flags) {
-        _cleanup_free_ char *parent = NULL;
-        int fd;
-
-        assert(path);
-
-        parent = dirname_malloc(path);
-        if (!parent)
-                return -ENOMEM;
-
-        fd = open(parent, flags);
-        if (fd < 0)
-                return -errno;
-
-        return fd;
-}
-
 static int extract_subvolume_name(const char *path, const char **subvolume) {
         const char *fn;
         int r;
@@ -144,7 +128,7 @@ int btrfs_subvol_make(const char *path) {
         if (r < 0)
                 return r;
 
-        fd = open_parent(path, O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY);
+        fd = open_parent(path, O_CLOEXEC, 0);
         if (fd < 0)
                 return fd;
 
@@ -1283,7 +1267,7 @@ int btrfs_subvol_remove(const char *path, BtrfsRemoveFlags flags) {
         if (r < 0)
                 return r;
 
-        fd = open_parent(path, O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY);
+        fd = open_parent(path, O_CLOEXEC, 0);
         if (fd < 0)
                 return fd;
 
@@ -1723,7 +1707,7 @@ int btrfs_subvol_snapshot_fd(int old_fd, const char *new_path, BtrfsSnapshotFlag
         if (r < 0)
                 return r;
 
-        new_fd = open_parent(new_path, O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY);
+        new_fd = open_parent(new_path, O_CLOEXEC, 0);
         if (new_fd < 0)
                 return new_fd;
 
diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index 6b0bad5b71..20d3f567c9 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -1225,9 +1225,13 @@ int tempfn_xxxxxx(const char *p, const char *extra, char **ret) {
         const char *fn;
         char *t;
 
-        assert(p);
         assert(ret);
 
+        if (isempty(p))
+                return -EINVAL;
+        if (path_equal(p, "/"))
+                return -EINVAL;
+
         /*
          * Turns this:
          *         /foo/bar/waldo
@@ -1258,9 +1262,13 @@ int tempfn_random(const char *p, const char *extra, char **ret) {
         uint64_t u;
         unsigned i;
 
-        assert(p);
         assert(ret);
 
+        if (isempty(p))
+                return -EINVAL;
+        if (path_equal(p, "/"))
+                return -EINVAL;
+
         /*
          * Turns this:
          *         /foo/bar/waldo
@@ -1311,7 +1319,8 @@ int tempfn_random_child(const char *p, const char *extra, char **ret) {
                 r = tmp_dir(&p);
                 if (r < 0)
                         return r;
-        }
+        } else if (isempty(p))
+                return -EINVAL;
 
         extra = strempty(extra);
 
@@ -1404,7 +1413,8 @@ int open_tmpfile_unlinkable(const char *directory, int flags) {
                 r = tmp_dir(&directory);
                 if (r < 0)
                         return r;
-        }
+        } else if (isempty(directory))
+                return -EINVAL;
 
         /* Returns an unlinked temporary file that cannot be linked into the file system anymore */
 
@@ -1439,22 +1449,14 @@ int open_tmpfile_linkable(const char *target, int flags, char **ret_path) {
          * which case "ret_path" will be returned as NULL. If not possible a the tempoary path name used is returned in
          * "ret_path". Use link_tmpfile() below to rename the result after writing the file in full. */
 
-        {
-                _cleanup_free_ char *dn = NULL;
-
-                dn = dirname_malloc(target);
-                if (!dn)
-                        return -ENOMEM;
-
-                fd = open(dn, O_TMPFILE|flags, 0640);
-                if (fd >= 0) {
-                        *ret_path = NULL;
-                        return fd;
-                }
-
-                log_debug_errno(errno, "Failed to use O_TMPFILE on %s: %m", dn);
+        fd = open_parent(target, O_TMPFILE|flags, 0640);
+        if (fd >= 0) {
+                *ret_path = NULL;
+                return fd;
         }
 
+        log_debug_errno(fd, "Failed to use O_TMPFILE for %s: %m", target);
+
         r = tempfn_random(target, NULL, &tmp);
         if (r < 0)
                 return r;
diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index 3a8b32d881..aca9921de7 100644
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -1156,7 +1156,7 @@ int unlinkat_deallocate(int fd, const char *name, int flags) {
 }
 
 int fsync_directory_of_file(int fd) {
-        _cleanup_free_ char *path = NULL, *dn = NULL;
+        _cleanup_free_ char *path = NULL;
         _cleanup_close_ int dfd = -1;
         int r;
 
@@ -1182,16 +1182,40 @@ int fsync_directory_of_file(int fd) {
         if (!path_is_absolute(path))
                 return -EINVAL;
 
-        dn = dirname_malloc(path);
-        if (!dn)
-                return -ENOMEM;
-
-        dfd = open(dn, O_RDONLY|O_CLOEXEC|O_DIRECTORY);
+        dfd = open_parent(path, O_CLOEXEC, 0);
         if (dfd < 0)
-                return -errno;
+                return dfd;
 
         if (fsync(dfd) < 0)
                 return -errno;
 
         return 0;
 }
+
+int open_parent(const char *path, int flags, mode_t mode) {
+        _cleanup_free_ char *parent = NULL;
+        int fd;
+
+        if (isempty(path))
+                return -EINVAL;
+        if (path_equal(path, "/")) /* requesting the parent of the root dir is fishy, let's prohibit that */
+                return -EINVAL;
+
+        parent = dirname_malloc(path);
+        if (!parent)
+                return -ENOMEM;
+
+        /* Let's insist on O_DIRECTORY since the parent of a file or directory is a directory. Except if we open an
+         * O_TMPFILE file, because in that case we are actually create a regular file below the parent directory. */
+
+        if ((flags & O_PATH) == O_PATH)
+                flags |= O_DIRECTORY;
+        else if ((flags & O_TMPFILE) != O_TMPFILE)
+                flags |= O_DIRECTORY|O_RDONLY;
+
+        fd = open(parent, flags, mode);
+        if (fd < 0)
+                return -errno;
+
+        return fd;
+}
diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h
index 28566773c6..754163defd 100644
--- a/src/basic/fs-util.h
+++ b/src/basic/fs-util.h
@@ -103,3 +103,5 @@ void unlink_tempfilep(char (*p)[]);
 int unlinkat_deallocate(int fd, const char *name, int flags);
 
 int fsync_directory_of_file(int fd);
+
+int open_parent(const char *path, int flags, mode_t mode);
diff --git a/src/basic/mount-util.c b/src/basic/mount-util.c
index ebe41a4c6c..54d911b095 100644
--- a/src/basic/mount-util.c
+++ b/src/basic/mount-util.c
@@ -261,7 +261,7 @@ fallback_fstat:
 
 /* flags can be AT_SYMLINK_FOLLOW or 0 */
 int path_is_mount_point(const char *t, const char *root, int flags) {
-        _cleanup_free_ char *canonical = NULL, *parent = NULL;
+        _cleanup_free_ char *canonical = NULL;
         _cleanup_close_ int fd = -1;
         int r;
 
@@ -283,11 +283,7 @@ int path_is_mount_point(const char *t, const char *root, int flags) {
                 t = canonical;
         }
 
-        parent = dirname_malloc(t);
-        if (!parent)
-                return -ENOMEM;
-
-        fd = openat(AT_FDCWD, parent, O_DIRECTORY|O_CLOEXEC|O_PATH);
+        fd = open_parent(t, O_PATH|O_CLOEXEC, 0);
         if (fd < 0)
                 return -errno;