From: Mike Yuan Date: Mon, 20 Jan 2025 20:48:27 +0000 (+0100) Subject: mountpoint-util: some tweaks for fd_is_mount_point() X-Git-Tag: v258-rc1~1536^2~2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e2f97c790e33d21676bc8d700cb8e474be9fbea2;p=thirdparty%2Fsystemd.git mountpoint-util: some tweaks for fd_is_mount_point() - Drop fstat() fallback path now that we assume fdinfo is available - Use at_flags_normalize_nofollow() - Accept empty path the same way as NULL - Accept fd being AT_FDCWD and filename being "." --- diff --git a/src/basic/mountpoint-util.c b/src/basic/mountpoint-util.c index b596d32680c..750bcfa71be 100644 --- a/src/basic/mountpoint-util.c +++ b/src/basic/mountpoint-util.c @@ -213,27 +213,35 @@ bool file_handle_equal(const struct file_handle *a, const struct file_handle *b) } int fd_is_mount_point(int fd, const char *filename, int flags) { - _cleanup_free_ struct file_handle *h = NULL, *h_parent = NULL; - int mount_id = -1, mount_id_parent = -1; - bool nosupp = false, check_st_dev = true; - STRUCT_STATX_DEFINE(sx); - struct stat a, b; + bool fd_is_self; int r; - assert(fd >= 0); + assert(fd >= 0 || fd == AT_FDCWD); assert((flags & ~AT_SYMLINK_FOLLOW) == 0); - if (!filename) { - /* If the file name is specified as NULL we'll see if the specified 'fd' is a mount - * point. That's only supported if the kernel supports 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 = ""; - } else if (!filename_possibly_with_slash_suffix(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. */ - return -EINVAL; + if (isempty(filename)) { + if (fd == AT_FDCWD) + filename = "."; + else { + /* If the file name is empty we'll see if the specified 'fd' is a mount point. + * That's only supported if the kernel supports 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 = ""; + } + + fd_is_self = true; + } else if (STR_IN_SET(filename, ".", "./")) + fd_is_self = true; + else { + /* 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; + + fd_is_self = false; + } /* First we will try statx()' STATX_ATTR_MOUNT_ROOT attribute, which is our ideal API, available * since kernel 5.8. @@ -246,18 +254,17 @@ int fd_is_mount_point(int fd, const char *filename, int flags) { * If that didn't work we will try to read the mount id from /proc/self/fdinfo/. This is almost * as good as name_to_handle_at(), however, does not return the opaque file handle. The opaque file * handle is pretty useful to detect the root directory, which we should always consider a mount - * point. Hence we use this only as fallback. Exporting the mnt_id in fdinfo is a pretty recent - * kernel addition. + * point. Hence we use this only as fallback. * - * As last fallback we do traditional fstat() based st_dev comparisons. This is how things were - * traditionally done, but unionfs breaks this since it exposes file systems with a variety of st_dev - * reported. Also, btrfs subvolumes have different st_dev, even though they aren't real mounts of - * their own. */ - - if (statx(fd, - filename, - (FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? 0 : AT_SYMLINK_NOFOLLOW) | - (flags & AT_EMPTY_PATH) | + * Note that traditionally the check is done via fstat()-based st_dev comparisons. However, various + * file systems don't guarantee same st_dev across single fs anymore, e.g. unionfs exposes file systems + * with a variety of st_dev reported. Also, btrfs subvolumes have different st_dev, even though + * they aren't real mounts of their own. */ + + STRUCT_STATX_DEFINE(sx); + + if (statx(fd, filename, + 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, @@ -271,6 +278,10 @@ int fd_is_mount_point(int fd, const char *filename, int flags) { } else if (FLAGS_SET(sx.stx_attributes_mask, STATX_ATTR_MOUNT_ROOT)) /* yay! */ return FLAGS_SET(sx.stx_attributes, STATX_ATTR_MOUNT_ROOT); + _cleanup_free_ struct file_handle *h = NULL, *h_parent = NULL; + int mount_id = -1, mount_id_parent = -1; + bool nosupp = false; + r = name_to_handle_at_try_fid(fd, filename, &h, &mount_id, flags); if (r < 0) { if (is_name_to_handle_at_fatal_error(r)) @@ -278,13 +289,12 @@ int fd_is_mount_point(int fd, const char *filename, int flags) { if (!ERRNO_IS_NOT_SUPPORTED(r)) goto fallback_fdinfo; - /* This kernel or file system does not support name_to_handle_at(), hence let's see - * if the upper fs supports it (in which case it is a mount point), otherwise fall - * back to the traditional stat() logic */ + /* This file system does not support name_to_handle_at(), hence let's see if the upper fs + * supports it (in which case it is a mount point), otherwise fall back to the fdinfo logic. */ nosupp = true; } - if (isempty(filename)) + if (fd_is_self) r = name_to_handle_at_try_fid(fd, "..", &h_parent, &mount_id_parent, 0); /* can't work for non-directories 😢 */ else r = name_to_handle_at_try_fid(fd, "", &h_parent, &mount_id_parent, AT_EMPTY_PATH); @@ -316,12 +326,10 @@ int fd_is_mount_point(int fd, const char *filename, int flags) { fallback_fdinfo: r = fd_fdinfo_mnt_id(fd, filename, flags, &mount_id); - if (ERRNO_IS_NEG_NOT_SUPPORTED(r) || ERRNO_IS_NEG_PRIVILEGE(r)) - goto fallback_fstat; if (r < 0) return r; - if (isempty(filename)) + if (fd_is_self) r = fd_fdinfo_mnt_id(fd, "..", 0, &mount_id_parent); /* can't work for non-directories 😢 */ else r = fd_fdinfo_mnt_id(fd, "", AT_EMPTY_PATH, &mount_id_parent); @@ -333,31 +341,27 @@ fallback_fdinfo: /* Hmm, so, the mount ids are the same. This leaves one special case though for the root file * system. For that, let's see if the parent directory has the same inode as we are interested - * in. Hence, let's also do fstat() checks now, too, but avoid the st_dev comparisons, since they - * aren't that useful on unionfs mounts. */ - check_st_dev = false; + * in. */ + + struct stat a, b; -fallback_fstat: /* yay for fstatat() taking a different set of flags than the other _at() above */ - if (flags & AT_SYMLINK_FOLLOW) - flags &= ~AT_SYMLINK_FOLLOW; - else - flags |= AT_SYMLINK_NOFOLLOW; - if (fstatat(fd, filename, &a, flags) < 0) + if (fstatat(fd, filename, &a, at_flags_normalize_nofollow(flags)) < 0) return -errno; - if (isempty(filename)) + if (fd_is_self) r = fstatat(fd, "..", &b, 0); else r = fstatat(fd, "", &b, AT_EMPTY_PATH); if (r < 0) return -errno; - /* A directory with same device and inode as its parent? Must be the root directory */ - if (stat_inode_same(&a, &b)) - return 1; - - return check_st_dev && (a.st_dev != b.st_dev); + /* A directory with same device and inode as its parent must be the root directory. Otherwise + * not a mount point. + * + * NB: we avoid inode_same_at() here because it internally attempts name_to_handle_at_try_fid() first, + * which is redundant. */ + return stat_inode_same(&a, &b); } /* flags can be AT_SYMLINK_FOLLOW or 0 */ diff --git a/src/test/test-mountpoint-util.c b/src/test/test-mountpoint-util.c index 89093d02124..dc202cbda6d 100644 --- a/src/test/test-mountpoint-util.c +++ b/src/test/test-mountpoint-util.c @@ -273,6 +273,7 @@ TEST(path_is_mount_point) { TEST(fd_is_mount_point) { _cleanup_(rm_rf_physical_and_freep) char *tmpdir = NULL; + _cleanup_free_ char *pwd = NULL; _cleanup_close_ int fd = -EBADF; int r; @@ -281,11 +282,8 @@ TEST(fd_is_mount_point) { /* Not allowed, since "/" is a path, not a plain filename */ assert_se(fd_is_mount_point(fd, "/", 0) == -EINVAL); - assert_se(fd_is_mount_point(fd, ".", 0) == -EINVAL); - assert_se(fd_is_mount_point(fd, "./", 0) == -EINVAL); assert_se(fd_is_mount_point(fd, "..", 0) == -EINVAL); assert_se(fd_is_mount_point(fd, "../", 0) == -EINVAL); - assert_se(fd_is_mount_point(fd, "", 0) == -EINVAL); assert_se(fd_is_mount_point(fd, "/proc", 0) == -EINVAL); assert_se(fd_is_mount_point(fd, "/proc/", 0) == -EINVAL); assert_se(fd_is_mount_point(fd, "proc/sys", 0) == -EINVAL); @@ -307,9 +305,20 @@ TEST(fd_is_mount_point) { fd = open("/proc", O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOCTTY); assert_se(fd >= 0); - assert_se(fd_is_mount_point(fd, NULL, 0) > 0); - assert_se(fd_is_mount_point(fd, "", 0) == -EINVAL); - assert_se(fd_is_mount_point(fd, "version", 0) == 0); + ASSERT_OK_POSITIVE(fd_is_mount_point(fd, NULL, 0)); + ASSERT_OK_POSITIVE(fd_is_mount_point(fd, "", 0)); + ASSERT_OK_POSITIVE(fd_is_mount_point(fd, ".", 0)); + ASSERT_OK_POSITIVE(fd_is_mount_point(fd, "./", 0)); + ASSERT_OK_ZERO(fd_is_mount_point(fd, "version", 0)); + + ASSERT_OK(safe_getcwd(&pwd)); + ASSERT_OK_ERRNO(fchdir(fd)); + + ASSERT_OK_POSITIVE(fd_is_mount_point(AT_FDCWD, NULL, 0)); + ASSERT_OK_POSITIVE(fd_is_mount_point(AT_FDCWD, "", 0)); + ASSERT_OK_POSITIVE(fd_is_mount_point(AT_FDCWD, "./", 0)); + + ASSERT_OK_ERRNO(chdir(pwd)); safe_close(fd); fd = open("/proc/version", O_RDONLY|O_CLOEXEC|O_NOCTTY); @@ -317,7 +326,6 @@ TEST(fd_is_mount_point) { r = fd_is_mount_point(fd, NULL, 0); assert_se(IN_SET(r, 0, -ENOTDIR)); /* on old kernels we can't determine if regular files are mount points if we have no directory fd */ - assert_se(fd_is_mount_point(fd, "", 0) == -EINVAL); if (!mount_new_api_supported()) return;