mountpoint-util: make is_mount_point_at() take usual dir_fd + path style arguments

diff --git a/src/basic/mountpoint-util.c b/src/basic/mountpoint-util.c
index b02746b14cb1d45a924439fd46c1205b0ff4adb7..1453340c55a845c1f3fdc7f8f02e8117fe5ceccb 100644 (file)
--- a/src/basic/mountpoint-util.c
+++ b/src/basic/mountpoint-util.c
@@ -163,29 +163,6 @@ static int fd_fdinfo_mnt_id(int fd, const char *filename, int flags, int *ret_mn
         return safe_atoi(p, ret_mnt_id);
 }
 
-static bool filename_possibly_with_slash_suffix(const char *s) {
-        const char *slash, *copied;
-
-        /* Checks whether the specified string is either file name, or a filename with a suffix of
-         * slashes. But nothing else.
-         *
-         * this is OK: foo, bar, foo/, bar/, foo//, bar///
-         * this is not OK: "", "/", "/foo", "foo/bar", ".", ".." … */
-
-        slash = strchr(s, '/');
-        if (!slash)
-                return filename_is_valid(s);
-
-        if (slash - s > PATH_MAX) /* We want to allocate on the stack below, hence do a size check first */
-                return false;
-
-        if (slash[strspn(slash, "/")] != 0) /* Check that the suffix consist only of one or more slashes */
-                return false;
-
-        copied = strndupa_safe(s, slash - s);
-        return filename_is_valid(copied);
-}
-
 bool file_handle_equal(const struct file_handle *a, const struct file_handle *b) {
         if (a == b)
                 return true;
@@ -197,37 +174,30 @@ bool file_handle_equal(const struct file_handle *a, const struct file_handle *b)
         return memcmp_nn(a->f_handle, a->handle_bytes, b->f_handle, b->handle_bytes) == 0;
 }
 
-int is_mount_point_at(int fd, const char *filename, int flags) {
+int is_mount_point_at(int dir_fd, const char *path, int flags) {
         int r;
 
-        assert(fd >= 0 || fd == AT_FDCWD);
+        assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
         assert((flags & ~AT_SYMLINK_FOLLOW) == 0);
 
-        if (isempty(filename)) {
-                if (fd == AT_FDCWD)
-                        filename = ".";
+        if (path_equal(path, "/"))
+                return true;
+
+        if (isempty(path)) {
+                if (dir_fd == AT_FDCWD)
+                        path = ".";
                 else {
-                        /* If the file name is empty we'll see if the specified 'fd' is a mount point.
-                         * That's only supported by statx(), or if the inode specified via 'fd' refers to a
-                         * directory. Otherwise, we'll have to fail (ENOTDIR), because we have no kernel API
-                         * to query the information we need. */
                         flags |= AT_EMPTY_PATH;
-                        filename = "";
+                        path = "";
                 }
-
-        } else if (!STR_IN_SET(filename, ".", "./")) {
-                /* Insist that the specified filename is actually a filename, and not a path, i.e. some inode
-                 * further up or down the tree then immediately below the specified directory fd. */
-                if (!filename_possibly_with_slash_suffix(filename))
-                        return -EINVAL;
         }
 
         struct statx sx = {}; /* explicitly initialize the struct to make msan silent. */
-        if (statx(fd, filename,
+        if (statx(dir_fd, path,
                   at_flags_normalize_nofollow(flags) |
                   AT_NO_AUTOMOUNT |            /* don't trigger automounts – mounts are a local concept, hence no need to trigger automounts to determine STATX_ATTR_MOUNT_ROOT */
                   AT_STATX_DONT_SYNC,          /* don't go to the network for this – for similar reasons */
-                  STATX_TYPE,
+                  STATX_TYPE|STATX_INO,
                   &sx) < 0)
                 return -errno;
 
@@ -235,30 +205,37 @@ int is_mount_point_at(int fd, const char *filename, int flags) {
         if (r < 0)
                 return r;
 
-        return FLAGS_SET(sx.stx_attributes, STATX_ATTR_MOUNT_ROOT);
+        if (FLAGS_SET(sx.stx_attributes, STATX_ATTR_MOUNT_ROOT))
+                return true;
+
+        /* When running on chroot environment, the root may not be a mount point, but we unconditionally
+         * return true when the input is "/" in the above, but the shortcut may not work e.g. when the path
+         * is relative. */
+        struct statx sx2 = {}; /* explicitly initialize the struct to make msan silent. */
+        if (statx(AT_FDCWD, "/", AT_STATX_DONT_SYNC, STATX_TYPE|STATX_INO, &sx2) < 0)
+                return -errno;
+
+        return statx_inode_same(&sx, &sx2);
 }
 
 /* flags can be AT_SYMLINK_FOLLOW or 0 */
 int path_is_mount_point_full(const char *path, const char *root, int flags) {
-        _cleanup_close_ int dfd = -EBADF;
-        _cleanup_free_ char *fn = NULL;
+        _cleanup_close_ int dir_fd = -EBADF;
+        int r;
 
         assert(path);
         assert((flags & ~AT_SYMLINK_FOLLOW) == 0);
 
-        if (path_equal(path, "/"))
-                return 1;
-
-        /* we need to resolve symlinks manually, we can't just rely on is_mount_point_at() to do that for us;
-         * if we have a structure like /bin -> /usr/bin/ and /usr is a mount point, then the parent that we
-         * look at needs to be /usr, not /. */
-        dfd = chase_and_open_parent(path, root,
-                                    CHASE_TRAIL_SLASH|(FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? 0 : CHASE_NOFOLLOW),
-                                    &fn);
-        if (dfd < 0)
-                return dfd;
-
-        return is_mount_point_at(dfd, fn, flags);
+        if (empty_or_root(root))
+                return is_mount_point_at(AT_FDCWD, path, flags);
+
+        r = chase(path, root,
+                  FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? 0 : CHASE_NOFOLLOW,
+                  /* ret_path= */ NULL, &dir_fd);
+        if (r < 0)
+                return r;
+
+        return is_mount_point_at(dir_fd, /* path= */ NULL, flags);
 }
 
 int path_get_mnt_id_at_fallback(int dir_fd, const char *path, int *ret) {

diff --git a/src/basic/mountpoint-util.h b/src/basic/mountpoint-util.h
index 180d75343cba31d3626710d6795678f743e33ed8..206815277bcdc60f8832975823dbed44835d482d 100644 (file)
--- a/src/basic/mountpoint-util.h
+++ b/src/basic/mountpoint-util.h
@@ -45,7 +45,7 @@ static inline int path_get_mnt_id(const char *path, int *ret) {
         return path_get_mnt_id_at(AT_FDCWD, path, ret);
 }
 
-int is_mount_point_at(int fd, const char *filename, int flags);
+int is_mount_point_at(int dir_fd, const char *path, int flags);
 int path_is_mount_point_full(const char *path, const char *root, int flags);
 static inline int path_is_mount_point(const char *path) {
         return path_is_mount_point_full(path, NULL, 0);

diff --git a/src/core/namespace.c b/src/core/namespace.c
index 422c18d23851b8ebea671a29f226203f3f19707c..c35226d63902be87b5e9caa6936c38ea368b4ad8 100644 (file)
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -3467,7 +3467,7 @@ static int is_extension_overlay(const char *path, int fd) {
                 fd = dfd;
         }
 
-        r = is_mount_point_at(fd, /* filename= */ NULL, /* flags= */ 0);
+        r = is_mount_point_at(fd, /* path= */ NULL, /* flags= */ 0);
         if (r < 0)
                 return log_debug_errno(r, "Unable to determine whether '%s' is a mount point: %m", path);
         if (r == 0)

diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c
index eae5512efc3cb3392159e498208873ecf34a7c8e..7dd45aacea5672be9e2eb0d27373b08198956755 100644 (file)
--- a/src/mount/mount-tool.c
+++ b/src/mount/mount-tool.c
@@ -1113,7 +1113,7 @@ static int action_umount(sd_bus *bus, int argc, char **argv) {
                 if (fstat(fd, &st) < 0)
                         return log_error_errno(errno, "Can't stat '%s' (from %s): %m", p, argv[i]);
 
-                r = is_mount_point_at(fd, /* filename= */ NULL, /* flags= */ 0);
+                r = is_mount_point_at(fd, /* path= */ NULL, /* flags= */ 0);
                 fd = safe_close(fd); /* before continuing make sure the dir is not keeping anything busy */
                 if (r > 0)
                         RET_GATHER(ret, stop_mounts(bus, p));

diff --git a/src/nspawn/nspawn-cgroup.c b/src/nspawn/nspawn-cgroup.c
index 55ce8e1e062597637fef8e63b47b7c45bd042a00..4206fe94e529a1730eecea17cc714712f3e44b12 100644 (file)
--- a/src/nspawn/nspawn-cgroup.c
+++ b/src/nspawn/nspawn-cgroup.c
@@ -147,7 +147,7 @@ int mount_cgroups(const char *dest, bool accept_existing) {
         if (r < 0)
                 return log_error_errno(r, "Failed to chase %s/sys/fs/cgroup: %m", strempty(dest));
 
-        r = is_mount_point_at(fd, /* filename= */ NULL, /* flags= */ 0);
+        r = is_mount_point_at(fd, /* path= */ NULL, /* flags= */ 0);
         if (r < 0)
                 return log_error_errno(r, "Failed to determine if %s is mounted already: %m", p);
         if (r > 0) {

diff --git a/src/test/test-mountpoint-util.c b/src/test/test-mountpoint-util.c
index d4c830b4768708f18e75b261a2615f1728e5f6c7..ba2ed3b3479448dc226e92482d6c33317ef4d068 100644 (file)
--- a/src/test/test-mountpoint-util.c
+++ b/src/test/test-mountpoint-util.c
@@ -283,18 +283,16 @@ TEST(is_mount_point_at) {
         fd = open("/", O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOCTTY);
         assert_se(fd >= 0);
 
-        /* Not allowed, since "/" is a path, not a plain filename */
-        assert_se(is_mount_point_at(fd, "/", 0) == -EINVAL);
-        assert_se(is_mount_point_at(fd, "..", 0) == -EINVAL);
-        assert_se(is_mount_point_at(fd, "../", 0) == -EINVAL);
-        assert_se(is_mount_point_at(fd, "/proc", 0) == -EINVAL);
-        assert_se(is_mount_point_at(fd, "/proc/", 0) == -EINVAL);
-        assert_se(is_mount_point_at(fd, "proc/sys", 0) == -EINVAL);
-        assert_se(is_mount_point_at(fd, "proc/sys/", 0) == -EINVAL);
-
-        /* This one definitely is a mount point */
-        assert_se(is_mount_point_at(fd, "proc", 0) > 0);
-        assert_se(is_mount_point_at(fd, "proc/", 0) > 0);
+        ASSERT_OK_POSITIVE(is_mount_point_at(fd, "/", /* flags= */ 0));
+        ASSERT_OK_POSITIVE(is_mount_point_at(fd, "..", /* flags= */ 0));
+        ASSERT_OK_POSITIVE(is_mount_point_at(fd, "../", /* flags= */ 0));
+        r = ASSERT_OK(proc_mounted());
+        ASSERT_OK_EQ(is_mount_point_at(fd, "/proc", /* flags= */ 0), r);
+        ASSERT_OK_EQ(is_mount_point_at(fd, "/proc/", /* flags= */ 0), r);
+        ASSERT_OK_EQ(is_mount_point_at(fd, "proc", /* flags= */ 0), r);
+        ASSERT_OK_EQ(is_mount_point_at(fd, "proc/", /* flags= */ 0), r);
+        ASSERT_OK_ZERO(is_mount_point_at(fd, "usr/lib", /* flags= */ 0));
+        ASSERT_OK_ZERO(is_mount_point_at(fd, "usr/lib", /* flags= */ 0));
 
         safe_close(fd);
         fd = open("/tmp", O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOCTTY);

diff --git a/src/validatefs/validatefs.c b/src/validatefs/validatefs.c
index 2ac3a3072f571bdf1a25cdb7fbd6785cc06c6084..74a784d8ab1f91e7679d72dd79ed479ffaf2128c 100644 (file)
--- a/src/validatefs/validatefs.c
+++ b/src/validatefs/validatefs.c
@@ -426,7 +426,7 @@ static int run(int argc, char *argv[]) {
         if (target_fd < 0)
                 return log_error_errno(target_fd, "Failed to open directory '%s': %m", arg_target);
 
-        r = is_mount_point_at(target_fd, /* filename= */ NULL, /* flags= */ 0);
+        r = is_mount_point_at(target_fd, /* path= */ NULL, /* flags= */ 0);
         if (r < 0)
                 return log_error_errno(r, "Failed to determine whether '%s' is a mount point: %m", resolved);
         if (!r)