SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fd-util: fix path_is_root_at() when dealing with detached mounts (#38636)

Browse Source 
path_is_root_at() is supposed to detect if the inode referenced by the
specified fd is the "root inode". For that it checks if the inode and
its parent are the same inode and the same mount. Traditionally this
check was correct. But these days we actually have detached mounts (i.e.
those returned by fsmount() and related calls), whose root inode also
behaves like that.

Our uses for path_is_root_at() use the function to detect if an absolute
path would be identical to a relative path based on the specified fd
(sepifically: chaseat()), which goes really wrong if used on a detached
mount.

hence, let's adjust the function a bit, and let's go by path to "/" to
check if the referenced inode is the actual root inode in our chroot.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2025-09-17 14:14:58 +02:00
committed by GitHub
parent 113ef23264 d486ae5adf
commit 08d8224c0a
2 changed files with 138 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/fd-util.c
+10 -11
View File
@@ -1035,10 +1035,9 @@ int fd_get_diskseq(int fd, uint64_t *ret) {
}
int path_is_root_at(int dir_fd, const char *path) {
_cleanup_close_ int fd = -EBADF, pfd = -EBADF;
assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
_cleanup_close_ int fd = -EBADF;
if (!isempty(path)) {
fd = openat(dir_fd, path, O_PATH|O_DIRECTORY|O_CLOEXEC);
if (fd < 0)
@@ -1047,19 +1046,19 @@ int path_is_root_at(int dir_fd, const char *path) {
dir_fd = fd;
}
pfd = openat(dir_fd, "..", O_PATH|O_DIRECTORY|O_CLOEXEC);
if (pfd < 0)
return errno == ENOTDIR ? false : -errno;
_cleanup_close_ int root_fd = openat(AT_FDCWD, "/", O_PATH|O_DIRECTORY|O_CLOEXEC);
if (root_fd < 0)
return -errno;
/* Even if the parent directory has the same inode, the fd may not point to the root directory "/",
* and we also need to check that the mount ids are the same. Otherwise, a construct like the
* following could be used to trick us:
/* Even if the root directory has the same inode as our fd, the fd may not point to the root
* directory "/", and we also need to check that the mount ids are the same. Otherwise, a construct
* like the following could be used to trick us:
*
* $ mkdir /tmp/x /tmp/x/y
* $ mount --bind /tmp/x /tmp/x/y
* $ mkdir /tmp/x
* $ mount --bind / /tmp/x
*/
return fds_are_same_mount(dir_fd, pfd);
return fds_are_same_mount(dir_fd, root_fd);
}
int fds_are_same_mount(int fd1, int fd2) {
src/test/test-fd-util.c
+128
View File
@@ -680,6 +680,134 @@ TEST(dir_fd_is_root) {
assert_se(dir_fd_is_root_or_cwd(fd) == 0);
}
static void test_path_is_root_at_one(bool expected) {
ASSERT_OK_POSITIVE(path_is_root("/"));
ASSERT_OK_POSITIVE(path_is_root("/."));
ASSERT_OK_EQ(path_is_root("/./.."), expected);
ASSERT_OK_EQ(path_is_root("/.."), expected);
ASSERT_OK_EQ(path_is_root("/../"), expected);
ASSERT_OK_EQ(path_is_root("/../."), expected);
ASSERT_OK_EQ(path_is_root("/../.."), expected);
ASSERT_OK_ZERO(path_is_root("/usr"));
ASSERT_OK_ZERO(path_is_root("/./usr"));
ASSERT_OK_ZERO(path_is_root("/../usr"));
ASSERT_OK_ZERO(path_is_root("/.././usr"));
ASSERT_OK_ZERO(path_is_root("/../../usr"));
_cleanup_close_ int fd = -EBADF;
ASSERT_OK_ERRNO(fd = open("/", O_CLOEXEC|O_PATH|O_DIRECTORY|O_NOFOLLOW));
ASSERT_OK_POSITIVE(path_is_root_at(fd, NULL));
ASSERT_OK_POSITIVE(path_is_root_at(fd, ""));
ASSERT_OK_POSITIVE(path_is_root_at(fd, "."));
ASSERT_OK_EQ(path_is_root_at(fd, "./../"), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "../"), expected);
ASSERT_OK_POSITIVE(path_is_root_at(fd, "/"));
ASSERT_OK_POSITIVE(path_is_root_at(fd, "/."));
ASSERT_OK_EQ(path_is_root_at(fd, "/./.."), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "/.."), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "/../"), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "/../."), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "/../.."), expected);
ASSERT_OK_ZERO(path_is_root_at(fd, "usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "./usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "../usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/./usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/../usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/.././usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/../../usr"));
safe_close(fd);
ASSERT_OK_ERRNO(fd = open("/../", O_CLOEXEC|O_PATH|O_DIRECTORY|O_NOFOLLOW));
ASSERT_OK_EQ(path_is_root_at(fd, NULL), expected);
ASSERT_OK_EQ(path_is_root_at(fd, ""), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "."), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "./.."), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "../"), expected);
ASSERT_OK_POSITIVE(path_is_root_at(fd, "/"));
ASSERT_OK_POSITIVE(path_is_root_at(fd, "/."));
ASSERT_OK_EQ(path_is_root_at(fd, "/./.."), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "/.."), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "/../"), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "/../."), expected);
ASSERT_OK_EQ(path_is_root_at(fd, "/../.."), expected);
ASSERT_OK_ZERO(path_is_root_at(fd, "usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "./usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "../usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/./usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/../usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/.././usr"));
ASSERT_OK_ZERO(path_is_root_at(fd, "/../../usr"));
}
TEST(path_is_root_at) {
int r;
test_path_is_root_at_one(true);
r = detach_mount_namespace();
if (r < 0)
return (void) log_tests_skipped_errno(r, "Failed to detach mount namespace");
/* Interestingly, even after bind mount a path on "/", still "/" points to the previous root
* directory, but "/../" points to the new root directory. Hence, path_is_root("/") is true but
* path_is_root("/../") is false. Such spurious situation is resolved after chroot()ing to the new
* root directory. */
ASSERT_OK(mount_nofollow_verbose(LOG_DEBUG, "/", "/", NULL, MS_BIND|MS_REC, NULL));
log_debug("/* %s: bind mount(\"/\", \"/\") */", __func__);
test_path_is_root_at_one(false);
/* chroot("/") does not change anything. */
ASSERT_OK_ERRNO(chroot("/"));
log_debug("/* %s: chroot(\"/\") */", __func__);
test_path_is_root_at_one(false);
/* chdir("/") neither change anything. */
ASSERT_OK_ERRNO(chdir("/"));
log_debug("/* %s: chdir(\"/\") */", __func__);
test_path_is_root_at_one(false);
/* chdir("/../") neither change anything. */
ASSERT_OK_ERRNO(chdir("/../"));
log_debug("/* %s: chdir(\"/../\") */", __func__);
test_path_is_root_at_one(false);
/* After chroot("/../"), both "/" and "/../" point to the new root directory. */
ASSERT_OK_ERRNO(chroot("/../"));
log_debug("/* %s: chroot(\"/../\") */", __func__);
test_path_is_root_at_one(true);
/* chdir("/../") does not change anything. */
ASSERT_OK_ERRNO(chdir("/../"));
log_debug("/* %s: chdir(\"/../\") again */", __func__);
test_path_is_root_at_one(true);
/* bind mounting to non-root directory has no problem, of course. */
_cleanup_(rm_rf_physical_and_freep) char *tmp = NULL;
ASSERT_OK(mkdtemp_malloc("/tmp/test-path_is_root-XXXXXX", &tmp));
ASSERT_OK(mount_nofollow_verbose(LOG_DEBUG, "/", tmp, NULL, MS_BIND|MS_REC, NULL));
log_debug("/* %s: bind mount(\"/\", \"%s\") */", __func__, tmp);
test_path_is_root_at_one(true);
ASSERT_OK_ERRNO(chdir(tmp));
log_debug("/* %s: chdir(\"%s\") */", __func__, tmp);
test_path_is_root_at_one(true);
ASSERT_OK_ERRNO(chroot(tmp));
log_debug("/* %s: chroot(\"%s\") */", __func__, tmp);
test_path_is_root_at_one(true);
ASSERT_OK_ERRNO(chdir(tmp));
log_debug("/* %s: chdir(\"%s\") again */", __func__, tmp);
test_path_is_root_at_one(true);
}
TEST(fds_are_same_mount) {
_cleanup_close_ int fd1 = -EBADF, fd2 = -EBADF, fd3 = -EBADF, fd4 = -EBADF;