From 5f5865f0ad8069ae01119e34c048a91eb5057143 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 29 Nov 2022 15:29:25 +0100 Subject: [PATCH 1/4] fd-util: add new helper fd_reopen_conditional() This is a wrapper around fd_reopen() that will reopen an fd if the F_GETFL flags indicate this is necessary, and otherwise not. This is useful for various utility calls that shall be able to operate on O_PATH and without it, and might need to convert between the two depending on what's passed in. --- src/basic/fd-util.c | 31 +++++++++++++++++++++++++++ src/basic/fd-util.h | 1 + src/test/test-fd-util.c | 47 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index ec33a61588..01a45e4384 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -780,6 +780,37 @@ int fd_reopen(int fd, int flags) { return new_fd; } +int fd_reopen_condition( + int fd, + int flags, + int mask, + int *ret_new_fd) { + + int r, new_fd; + + assert(fd >= 0); + + /* Invokes fd_reopen(fd, flags), but only if the existing F_GETFL flags don't match the specified + * flags (masked by the specified mask). This is useful for converting O_PATH fds into real fds if + * needed, but only then. */ + + r = fcntl(fd, F_GETFL); + if (r < 0) + return -errno; + + if ((r & mask) == (flags & mask)) { + *ret_new_fd = -1; + return fd; + } + + new_fd = fd_reopen(fd, flags); + if (new_fd < 0) + return new_fd; + + *ret_new_fd = new_fd; + return new_fd; +} + int read_nr_open(void) { _cleanup_free_ char *nr_open = NULL; int r; diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index 29c7d86f27..fbaa458613 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -108,6 +108,7 @@ static inline int make_null_stdio(void) { }) int fd_reopen(int fd, int flags); +int fd_reopen_condition(int fd, int flags, int mask, int *ret_new_fd); int read_nr_open(void); int fd_get_diskseq(int fd, uint64_t *ret); diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index 2b85ceab82..df6ca13785 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -483,6 +483,53 @@ TEST(fd_reopen) { fd1 = -1; } +TEST(fd_reopen_condition) { + _cleanup_close_ int fd1 = -1, fd3 = -1; + int fd2, fl; + + /* Open without O_PATH */ + fd1 = open("/usr/", O_RDONLY|O_DIRECTORY|O_CLOEXEC); + assert_se(fd1 >= 0); + + fl = fcntl(fd1, F_GETFL); + assert_se(FLAGS_SET(fl, O_DIRECTORY)); + assert_se(!FLAGS_SET(fl, O_PATH)); + + fd2 = fd_reopen_condition(fd1, O_DIRECTORY, O_DIRECTORY|O_PATH, &fd3); + assert_se(fd2 == fd1); + assert_se(fd3 < 0); + + /* Switch on O_PATH */ + fd2 = fd_reopen_condition(fd1, O_DIRECTORY|O_PATH, O_DIRECTORY|O_PATH, &fd3); + assert_se(fd2 != fd1); + assert_se(fd3 == fd2); + + fl = fcntl(fd2, F_GETFL); + assert_se(FLAGS_SET(fl, O_DIRECTORY)); + assert_se(FLAGS_SET(fl, O_PATH)); + + close_and_replace(fd1, fd3); + + fd2 = fd_reopen_condition(fd1, O_DIRECTORY|O_PATH, O_DIRECTORY|O_PATH, &fd3); + assert_se(fd2 == fd1); + assert_se(fd3 < 0); + + /* Switch off O_PATH again */ + fd2 = fd_reopen_condition(fd1, O_DIRECTORY, O_DIRECTORY|O_PATH, &fd3); + assert_se(fd2 != fd1); + assert_se(fd3 == fd2); + + fl = fcntl(fd2, F_GETFL); + assert_se(FLAGS_SET(fl, O_DIRECTORY)); + assert_se(!FLAGS_SET(fl, O_PATH)); + + close_and_replace(fd1, fd3); + + fd2 = fd_reopen_condition(fd1, O_DIRECTORY, O_DIRECTORY|O_PATH, &fd3); + assert_se(fd2 == fd1); + assert_se(fd3 < 0); +} + TEST(take_fd) { _cleanup_close_ int fd1 = -1, fd2 = -1; int array[2] = { -1, -1 }, i = 0; From f1cab3d6dcb882f4537891e465dc2f080301015e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 29 Nov 2022 15:30:55 +0100 Subject: [PATCH 2/4] btrfs-util: convert to fd_reopen_condition() --- src/shared/btrfs-util.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c index 4574a7899e..41e01e0b10 100644 --- a/src/shared/btrfs-util.c +++ b/src/shared/btrfs-util.c @@ -107,19 +107,11 @@ int btrfs_subvol_make_fd(int fd, const char *subvolume) { if (r < 0) return r; - r = fcntl(fd, F_GETFL); - if (r < 0) - return -errno; - if (FLAGS_SET(r, O_PATH)) { - /* An O_PATH fd was specified, let's convert here to a proper one, as btrfs ioctl's can't deal with - * O_PATH. */ - - real_fd = fd_reopen(fd, O_RDONLY|O_CLOEXEC|O_DIRECTORY); - if (real_fd < 0) - return real_fd; - - fd = real_fd; - } + /* If an O_PATH fd was specified, let's convert here to a proper one, as btrfs ioctl's can't deal + * with O_PATH. */ + fd = fd_reopen_condition(fd, O_RDONLY|O_CLOEXEC|O_DIRECTORY, O_PATH|O_DIRECTORY, &real_fd); + if (fd < 0) + return fd; strncpy(args.name, subvolume, sizeof(args.name)-1); From 698bc1861e8ed830517136177c2ddd5100de1994 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 29 Nov 2022 15:31:31 +0100 Subject: [PATCH 3/4] blockdev-util: move O_PATH fd conversion into btrfs_get_block_device_fd() to shorten things And let's use a simple call to the new fd_reopen_condition() helper there. --- src/shared/blockdev-util.c | 34 ++-------------------------------- src/shared/btrfs-util.c | 5 +++++ 2 files changed, 7 insertions(+), 32 deletions(-) diff --git a/src/shared/blockdev-util.c b/src/shared/blockdev-util.c index 72fad160ed..eb9f54306f 100644 --- a/src/shared/blockdev-util.c +++ b/src/shared/blockdev-util.c @@ -43,23 +43,7 @@ static int fd_get_devnum(int fd, BlockDeviceLookupFlag flags, dev_t *ret) { /* If major(st.st_dev) is zero, this might mean we are backed by btrfs, which needs special * handing, to get the backing device node. */ - r = fcntl(fd, F_GETFL); - if (r < 0) - return -errno; - - if (FLAGS_SET(r, O_PATH)) { - _cleanup_close_ int regfd = -1; - - /* The fstat() above we can execute on an O_PATH fd. But the btrfs ioctl we cannot. - * Hence acquire a "real" fd first, without the O_PATH flag. */ - - regfd = fd_reopen(fd, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); - if (regfd < 0) - return regfd; - - r = btrfs_get_block_device_fd(regfd, &devnum); - } else - r = btrfs_get_block_device_fd(fd, &devnum); + r = btrfs_get_block_device_fd(fd, &devnum); if (r == -ENOTTY) /* not btrfs */ return -ENOTBLK; if (r < 0) @@ -288,21 +272,7 @@ int get_block_device_fd(int fd, dev_t *ret) { return 1; } - r = fcntl(fd, F_GETFL); - if (r < 0) - return -errno; - if (FLAGS_SET(r, O_PATH) && (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode))) { - _cleanup_close_ int real_fd = -1; - - /* The fstat() above we can execute on an O_PATH fd. But the btrfs ioctl we cannot. Hence - * acquire a "real" fd first, without the O_PATH flag. */ - - real_fd = fd_reopen(fd, O_RDONLY|O_CLOEXEC); - if (real_fd < 0) - return real_fd; - r = btrfs_get_block_device_fd(real_fd, ret); - } else - r = btrfs_get_block_device_fd(fd, ret); + r = btrfs_get_block_device_fd(fd, ret); if (r > 0) return 1; if (r != -ENOTTY) /* not btrfs */ diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c index 41e01e0b10..affe06ec80 100644 --- a/src/shared/btrfs-util.c +++ b/src/shared/btrfs-util.c @@ -247,12 +247,17 @@ int btrfs_clone_range(int infd, uint64_t in_offset, int outfd, uint64_t out_offs int btrfs_get_block_device_fd(int fd, dev_t *dev) { struct btrfs_ioctl_fs_info_args fsi = {}; + _cleanup_close_ int regfd = -1; uint64_t id; int r; assert(fd >= 0); assert(dev); + fd = fd_reopen_condition(fd, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY, O_PATH, ®fd); + if (fd < 0) + return fd; + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; From e6d1d4c0b31275c4394676bce4f9bc28e3fad5a5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 29 Nov 2022 15:31:50 +0100 Subject: [PATCH 4/4] btrfs-util: convert O_PATH if necessary, in btrfs quota call Fixes: #25468 --- src/shared/btrfs-util.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c index affe06ec80..ba02de17f8 100644 --- a/src/shared/btrfs-util.c +++ b/src/shared/btrfs-util.c @@ -1765,6 +1765,7 @@ int btrfs_qgroup_find_parents(int fd, uint64_t qgroupid, uint64_t **ret) { int btrfs_subvol_auto_qgroup_fd(int fd, uint64_t subvol_id, bool insert_intermediary_qgroup) { _cleanup_free_ uint64_t *qgroups = NULL; + _cleanup_close_ int real_fd = -1; uint64_t parent_subvol; bool changed = false; int n = 0, r; @@ -1808,6 +1809,11 @@ int btrfs_subvol_auto_qgroup_fd(int fd, uint64_t subvol_id, bool insert_intermed * qgroup that then includes all its own child subvolumes. */ + /* Turn this into a proper fd, if it is currently O_PATH */ + fd = fd_reopen_condition(fd, O_RDONLY|O_CLOEXEC, O_PATH, &real_fd); + if (fd < 0) + return fd; + if (subvol_id == 0) { r = btrfs_is_subvol_fd(fd); if (r < 0)