From d35ff4b60e4b5ca166aca48afb5bb7fde3275cac Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 Jul 2023 01:07:36 +0900 Subject: [PATCH 1/4] stat-util: fix error handling of statx() When newer glibc is used, but kernel does not support statx(), then glibc try to fallback with fstatat(). That's quite similar to our implementation, but the supported flags are different, and if unsupported flags are specified, it returns EINVAL. Let's handle the case more gracefully. --- src/basic/stat-util.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c index b4905220fa..c54374b2c9 100644 --- a/src/basic/stat-util.c +++ b/src/basic/stat-util.c @@ -394,21 +394,35 @@ bool statx_mount_same(const struct new_statx *a, const struct new_statx *b) { a->stx_dev_minor == b->stx_dev_minor; } +static bool is_statx_fatal_error(int err, int flags) { + assert(err < 0); + + /* If statx() is not supported or if we see EPERM (which might indicate seccomp filtering or so), + * let's do a fallback. Note that on EACCES we'll not fall back, since that is likely an indication of + * fs access issues, which we should propagate. */ + if (ERRNO_IS_NOT_SUPPORTED(err) || err == -EPERM) + return false; + + /* When unsupported flags are specified, glibc's fallback function returns -EINVAL. + * See statx_generic() in glibc. */ + if (err != -EINVAL) + return true; + + if ((flags & ~(AT_EMPTY_PATH | AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW | AT_STATX_SYNC_AS_STAT)) != 0) + return false; /* Unsupported flags are specified. Let's try to use our implementation. */ + + return true; +} + int statx_fallback(int dfd, const char *path, int flags, unsigned mask, struct statx *sx) { static bool avoid_statx = false; struct stat st; + int r; if (!avoid_statx) { - if (statx(dfd, path, flags, mask, sx) < 0) { - if (!ERRNO_IS_NOT_SUPPORTED(errno) && errno != EPERM) - return -errno; - - /* If statx() is not supported or if we see EPERM (which might indicate seccomp - * filtering or so), let's do a fallback. Not that on EACCES we'll not fall back, - * since that is likely an indication of fs access issues, which we should - * propagate */ - } else - return 0; + r = RET_NERRNO(statx(dfd, path, flags, mask, sx)); + if (r >= 0 || is_statx_fatal_error(r, flags)) + return r; avoid_statx = true; } From 748711576662dbf697d5b4251d3fdbce6a666e7a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 Jul 2023 01:15:59 +0900 Subject: [PATCH 2/4] mountpoint-util: statx() may return EINVAL if the kernel does not support the syscall When statx() is called with AT_STATX_DONT_SYNC, glibc's fallback code returns EINVAL. See statx_generic() in glibc. --- src/basic/mountpoint-util.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/basic/mountpoint-util.c b/src/basic/mountpoint-util.c index 3584f31787..d7b182f6dd 100644 --- a/src/basic/mountpoint-util.c +++ b/src/basic/mountpoint-util.c @@ -223,7 +223,9 @@ int fd_is_mount_point(int fd, const char *filename, int flags) { AT_STATX_DONT_SYNC, /* don't go to the network for this – for similar reasons */ STATX_TYPE, &sx) < 0) { - if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) + if (!ERRNO_IS_NOT_SUPPORTED(errno) && /* statx() is not supported by the kernel. */ + !ERRNO_IS_PRIVILEGE(errno) && /* maybe filtered by seccomp. */ + errno != EINVAL) /* glibc's fallback method returns EINVAL when AT_STATX_DONT_SYNC is set. */ return -errno; /* If statx() is not available or forbidden, fall back to name_to_handle_at() below */ @@ -371,7 +373,9 @@ int path_get_mnt_id_at(int dir_fd, const char *path, int *ret) { AT_STATX_DONT_SYNC, /* don't go to the network, mnt_id is a local concept */ STATX_MNT_ID, &buf.sx) < 0) { - if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) + if (!ERRNO_IS_NOT_SUPPORTED(errno) && /* statx() is not supported by the kernel. */ + !ERRNO_IS_PRIVILEGE(errno) && /* maybe filtered by seccomp. */ + errno != EINVAL) /* glibc's fallback method returns EINVAL when AT_STATX_DONT_SYNC is set. */ return -errno; /* Fall back to name_to_handle_at() and then fdinfo if statx is not supported or we lack From bd96111d5fa8d3c05aad6aed21243d6b0534055d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 Jul 2023 01:23:17 +0900 Subject: [PATCH 3/4] fd-util: make path_is_root_at() not fail even when /proc is mounted path_get_mnt_id_at() -> fd_fdinfo_mnt_id() may return -EOPNOTSUPP when /proc is mounted, and -ENOSYS otherwise, when an old kernel is used. --- src/basic/fd-util.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index d9ef7ec932..1c67e785aa 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -929,17 +929,18 @@ int path_is_root_at(int dir_fd, const char *path) { * $ mount --bind /tmp/x /tmp/x/y * * Note, statx() does not provide the mount ID and path_get_mnt_id_at() does not work when an old - * kernel is used without /proc mounted. In that case, let's assume that we do not have such spurious - * mount points in an early boot stage, and silently skip the following check. */ + * kernel is used. In that case, let's assume that we do not have such spurious mount points in an + * early boot stage, and silently skip the following check. */ if (!FLAGS_SET(st.nsx.stx_mask, STATX_MNT_ID)) { int mntid; r = path_get_mnt_id_at(dir_fd, "", &mntid); - if (r == -ENOSYS) - return true; /* skip the mount ID check */ - if (r < 0) + if (r < 0) { + if (ERRNO_IS_NOT_SUPPORTED(r)) + return true; /* skip the mount ID check */ return r; + } assert(mntid >= 0); st.nsx.stx_mnt_id = mntid; @@ -950,10 +951,11 @@ int path_is_root_at(int dir_fd, const char *path) { int mntid; r = path_get_mnt_id_at(dir_fd, "..", &mntid); - if (r == -ENOSYS) - return true; /* skip the mount ID check */ - if (r < 0) + if (r < 0) { + if (ERRNO_IS_NOT_SUPPORTED(r)) + return true; /* skip the mount ID check */ return r; + } assert(mntid >= 0); pst.nsx.stx_mnt_id = mntid; From 68a4fc8b5386646302af5bf35ac9b22d13501b77 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 Jul 2023 01:31:16 +0900 Subject: [PATCH 4/4] fd-util: do not call statx() twice when it does not provide mount ID Previously, in path_is_root_at(), if statx() does not provide mount ID, path_get_mnt_id_at() was called, but it also calls statx(). Let's avoid the second trial. --- src/basic/fd-util.c | 4 ++-- src/basic/mountpoint-util.c | 20 ++++++++++++++------ src/basic/mountpoint-util.h | 1 + 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 1c67e785aa..1730c9ab2d 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -935,7 +935,7 @@ int path_is_root_at(int dir_fd, const char *path) { if (!FLAGS_SET(st.nsx.stx_mask, STATX_MNT_ID)) { int mntid; - r = path_get_mnt_id_at(dir_fd, "", &mntid); + r = path_get_mnt_id_at_fallback(dir_fd, "", &mntid); if (r < 0) { if (ERRNO_IS_NOT_SUPPORTED(r)) return true; /* skip the mount ID check */ @@ -950,7 +950,7 @@ int path_is_root_at(int dir_fd, const char *path) { if (!FLAGS_SET(pst.nsx.stx_mask, STATX_MNT_ID)) { int mntid; - r = path_get_mnt_id_at(dir_fd, "..", &mntid); + r = path_get_mnt_id_at_fallback(dir_fd, "..", &mntid); if (r < 0) { if (ERRNO_IS_NOT_SUPPORTED(r)) return true; /* skip the mount ID check */ diff --git a/src/basic/mountpoint-util.c b/src/basic/mountpoint-util.c index d7b182f6dd..5af0896ad6 100644 --- a/src/basic/mountpoint-util.c +++ b/src/basic/mountpoint-util.c @@ -359,9 +359,21 @@ int path_is_mount_point(const char *t, const char *root, int flags) { return fd_is_mount_point(fd, last_path_component(t), flags); } +int path_get_mnt_id_at_fallback(int dir_fd, const char *path, int *ret) { + int r; + + assert(dir_fd >= 0 || dir_fd == AT_FDCWD); + assert(ret); + + r = name_to_handle_at_loop(dir_fd, path, NULL, ret, isempty(path) ? AT_EMPTY_PATH : 0); + if (r == 0 || is_name_to_handle_at_fatal_error(r)) + return r; + + return fd_fdinfo_mnt_id(dir_fd, path, isempty(path) ? AT_EMPTY_PATH : 0, ret); +} + int path_get_mnt_id_at(int dir_fd, const char *path, int *ret) { STRUCT_NEW_STATX_DEFINE(buf); - int r; assert(dir_fd >= 0 || dir_fd == AT_FDCWD); assert(ret); @@ -386,11 +398,7 @@ int path_get_mnt_id_at(int dir_fd, const char *path, int *ret) { return 0; } - r = name_to_handle_at_loop(dir_fd, path, NULL, ret, isempty(path) ? AT_EMPTY_PATH : 0); - if (r == 0 || is_name_to_handle_at_fatal_error(r)) - return r; - - return fd_fdinfo_mnt_id(dir_fd, path, isempty(path) ? AT_EMPTY_PATH : 0, ret); + return path_get_mnt_id_at_fallback(dir_fd, path, ret); } bool fstype_is_network(const char *fstype) { diff --git a/src/basic/mountpoint-util.h b/src/basic/mountpoint-util.h index 8c4cee2f54..846015c43c 100644 --- a/src/basic/mountpoint-util.h +++ b/src/basic/mountpoint-util.h @@ -37,6 +37,7 @@ int name_to_handle_at_loop(int fd, const char *path, struct file_handle **ret_handle, int *ret_mnt_id, int flags); +int path_get_mnt_id_at_fallback(int dir_fd, const char *path, int *ret); int path_get_mnt_id_at(int dir_fd, const char *path, int *ret); static inline int path_get_mnt_id(const char *path, int *ret) { return path_get_mnt_id_at(AT_FDCWD, path, ret);