From 0867a46534335cd160c13f9176a5ef283588b565 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Fri, 24 Mar 2023 16:33:20 +0100 Subject: [PATCH 1/3] lock-util: Add LOCK_NONE --- src/basic/fs-util.c | 2 +- src/basic/lock-util.c | 2 ++ src/basic/lock-util.h | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 1cf0f5b945..94c0dfd3de 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1177,7 +1177,7 @@ int xopenat_lock( /* POSIX/UNPOSIX locks don't work on directories (errno is set to -EBADF so let's return early with * the same error here). */ - if (FLAGS_SET(open_flags, O_DIRECTORY) && locktype != LOCK_BSD) + if (FLAGS_SET(open_flags, O_DIRECTORY) && !IN_SET(locktype, LOCK_BSD, LOCK_NONE)) return -EBADF; for (;;) { diff --git a/src/basic/lock-util.c b/src/basic/lock-util.c index 3614fbe37c..7e67c2d583 100644 --- a/src/basic/lock-util.c +++ b/src/basic/lock-util.c @@ -174,6 +174,8 @@ int lock_generic(int fd, LockType type, int operation) { assert(fd >= 0); switch (type) { + case LOCK_NONE: + return 0; case LOCK_BSD: return RET_NERRNO(flock(fd, operation)); case LOCK_POSIX: diff --git a/src/basic/lock-util.h b/src/basic/lock-util.h index e7744476bb..b96ad85e21 100644 --- a/src/basic/lock-util.h +++ b/src/basic/lock-util.h @@ -34,6 +34,7 @@ void unposix_unlockpp(int **fd); _cleanup_(unposix_unlockpp) _unused_ int *CONCATENATE(_cleanup_unposix_unlock_, UNIQ) = &(fd) typedef enum LockType { + LOCK_NONE, /* Don't lock the file descriptor. Useful if you need to conditionally lock a file. */ LOCK_BSD, LOCK_POSIX, LOCK_UNPOSIX, From 01428141ba42a06d22f2c0e0df26049e700f4098 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Fri, 24 Mar 2023 17:12:24 +0100 Subject: [PATCH 2/3] copy: Add COPY_LOCK_BSD When making ephemeral copies of files/directories whose cleanup depends on whether they're locked or not, it's necessary to have the lock from the very beginning, so let's support that with a new COPY_LOCK_BSD flag. --- src/shared/copy.c | 81 ++++++++++++++++++++------------------------ src/shared/copy.h | 1 + src/test/test-copy.c | 21 ++++++++++++ 3 files changed, 58 insertions(+), 45 deletions(-) diff --git a/src/shared/copy.c b/src/shared/copy.c index 9c22178561..db26945be8 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -167,6 +167,7 @@ int copy_bytes_full( assert(fdf >= 0); assert(fdt >= 0); + assert(!FLAGS_SET(copy_flags, COPY_LOCK_BSD)); /* Tries to copy bytes from the file descriptor 'fdf' to 'fdt' in the smartest possible way. Copies a maximum * of 'max_bytes', which may be specified as UINT64_MAX, in which no maximum is applied. Returns negative on @@ -931,7 +932,7 @@ static int fd_copy_directory( _cleanup_close_ int fdf = -EBADF, fdt = -EBADF; _cleanup_closedir_ DIR *d = NULL; - bool exists, created; + bool exists; int r; assert(st); @@ -961,33 +962,22 @@ static int fd_copy_directory( if (!d) return -errno; - exists = false; - if (copy_flags & COPY_MERGE_EMPTY) { - r = dir_is_empty_at(dt, to, /* ignore_hidden_or_backup= */ false); - if (r < 0 && r != -ENOENT) - return r; - else if (r == 1) - exists = true; - } + r = dir_is_empty_at(dt, to, /* ignore_hidden_or_backup= */ false); + if (r < 0 && r != -ENOENT) + return r; + if ((r > 0 && !(copy_flags & (COPY_MERGE|COPY_MERGE_EMPTY))) || (r == 0 && !FLAGS_SET(copy_flags, COPY_MERGE))) + return -EEXIST; - if (exists) - created = false; - else { - if (copy_flags & COPY_MAC_CREATE) - r = mkdirat_label(dt, to, st->st_mode & 07777); - else - r = mkdirat(dt, to, st->st_mode & 07777); - if (r >= 0) - created = true; - else if (errno == EEXIST && (copy_flags & COPY_MERGE)) - created = false; - else - return -errno; - } + exists = r >= 0; - fdt = openat(dt, to, O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW); + fdt = xopenat_lock(dt, to, + O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW|(exists ? 0 : O_CREAT|O_EXCL), + (copy_flags & COPY_MAC_CREATE ? XO_LABEL : 0), + st->st_mode & 07777, + copy_flags & COPY_LOCK_BSD ? LOCK_BSD : LOCK_NONE, + LOCK_EX); if (fdt < 0) - return -errno; + return fdt; r = 0; @@ -1063,9 +1053,9 @@ static int fd_copy_directory( } q = fd_copy_tree_generic(dirfd(d), de->d_name, &buf, fdt, de->d_name, original_device, - depth_left-1, override_uid, override_gid, copy_flags, denylist, - hardlink_context, child_display_path, progress_path, progress_bytes, - userdata); + depth_left-1, override_uid, override_gid, copy_flags & ~COPY_LOCK_BSD, + denylist, hardlink_context, child_display_path, progress_path, + progress_bytes, userdata); if (q == -EINTR) /* Propagate SIGINT/SIGTERM up instantly */ return q; @@ -1076,7 +1066,7 @@ static int fd_copy_directory( } finish: - if (created) { + if (!exists) { if (fchown(fdt, uid_is_valid(override_uid) ? override_uid : st->st_uid, gid_is_valid(override_gid) ? override_gid : st->st_gid) < 0) @@ -1094,7 +1084,7 @@ finish: return -errno; } - return r; + return copy_flags & COPY_LOCK_BSD ? TAKE_FD(fdt) : 0; } static int fd_copy_leaf( @@ -1188,6 +1178,7 @@ int copy_tree_at_full( assert(from); assert(to); + assert(!FLAGS_SET(copy_flags, COPY_LOCK_BSD)); if (fstatat(fdf, from, &st, AT_SYMLINK_NOFOLLOW) < 0) return -errno; @@ -1290,6 +1281,7 @@ int copy_file_fd_at_full( assert(dir_fdf >= 0 || dir_fdf == AT_FDCWD); assert(from); assert(fdt >= 0); + assert(!FLAGS_SET(copy_flags, COPY_LOCK_BSD)); fdf = openat(dir_fdf, from, O_RDONLY|O_CLOEXEC|O_NOCTTY); if (fdf < 0) @@ -1360,17 +1352,13 @@ int copy_file_at_full( return r; WITH_UMASK(0000) { - if (copy_flags & COPY_MAC_CREATE) { - r = mac_selinux_create_file_prepare_at(dir_fdt, to, S_IFREG); - if (r < 0) - return r; - } - fdt = openat(dir_fdt, to, flags|O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY, - mode != MODE_INVALID ? mode : st.st_mode); - if (copy_flags & COPY_MAC_CREATE) - mac_selinux_create_file_clear(); + fdt = xopenat_lock(dir_fdt, to, + flags|O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY, + (copy_flags & COPY_MAC_CREATE ? XO_LABEL : 0), + mode != MODE_INVALID ? mode : st.st_mode, + copy_flags & COPY_LOCK_BSD ? LOCK_BSD : LOCK_NONE, LOCK_EX); if (fdt < 0) - return -errno; + return fdt; } if (!FLAGS_SET(flags, O_EXCL)) { /* if O_EXCL was used we created the thing as regular file, no need to check again */ @@ -1382,7 +1370,7 @@ int copy_file_at_full( if (chattr_mask != 0) (void) chattr_fd(fdt, chattr_flags, chattr_mask & CHATTR_EARLY_FL, NULL); - r = copy_bytes_full(fdf, fdt, UINT64_MAX, copy_flags, NULL, NULL, progress_bytes, userdata); + r = copy_bytes_full(fdf, fdt, UINT64_MAX, copy_flags & ~COPY_LOCK_BSD, NULL, NULL, progress_bytes, userdata); if (r < 0) goto fail; @@ -1399,9 +1387,11 @@ int copy_file_at_full( } } - r = close_nointr(TAKE_FD(fdt)); /* even if this fails, the fd is now invalidated */ - if (r < 0) - goto fail; + if (!FLAGS_SET(copy_flags, COPY_LOCK_BSD)) { + r = close_nointr(TAKE_FD(fdt)); /* even if this fails, the fd is now invalidated */ + if (r < 0) + goto fail; + } if (copy_flags & COPY_FSYNC_FULL) { r = fsync_parent_at(dir_fdt, to); @@ -1409,7 +1399,7 @@ int copy_file_at_full( goto fail; } - return 0; + return copy_flags & COPY_LOCK_BSD ? TAKE_FD(fdt) : 0; fail: /* Only unlink if we definitely are the ones who created the file */ @@ -1437,6 +1427,7 @@ int copy_file_atomic_at_full( assert(from); assert(to); + assert(!FLAGS_SET(copy_flags, COPY_LOCK_BSD)); if (copy_flags & COPY_MAC_CREATE) { r = mac_selinux_create_file_prepare_at(dir_fdt, to, S_IFREG); diff --git a/src/shared/copy.h b/src/shared/copy.h index da084b66f2..3bf496e42b 100644 --- a/src/shared/copy.h +++ b/src/shared/copy.h @@ -29,6 +29,7 @@ typedef enum CopyFlags { COPY_HOLES = 1 << 14, /* Copy holes */ COPY_GRACEFUL_WARN = 1 << 15, /* Skip copying file types that aren't supported by the target filesystem */ COPY_TRUNCATE = 1 << 16, /* Truncate to current file offset after copying */ + COPY_LOCK_BSD = 1 << 17, /* Return a BSD exclusively locked file descriptor referring to the copied image/directory. */ } CopyFlags; typedef enum DenyType { diff --git a/src/test/test-copy.c b/src/test/test-copy.c index f5bcb8756f..72aea4efb6 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include #include #include @@ -435,4 +436,24 @@ TEST_RET(copy_holes) { return 0; } +TEST(copy_lock) { + _cleanup_(rm_rf_physical_and_freep) char *t = NULL; + _cleanup_close_ int tfd = -EBADF, fd = -EBADF; + + assert_se((tfd = mkdtemp_open(NULL, 0, &t)) >= 0); + assert_se(mkdirat(tfd, "abc", 0755) >= 0); + assert_se(write_string_file_at(tfd, "abc/def", "abc", WRITE_STRING_FILE_CREATE) >= 0); + + assert_se((fd = copy_directory_at(tfd, "abc", tfd, "qed", COPY_LOCK_BSD)) >= 0); + assert_se(faccessat(tfd, "qed", F_OK, 0) >= 0); + assert_se(faccessat(tfd, "qed/def", F_OK, 0) >= 0); + assert_se(xopenat_lock(tfd, "qed", 0, 0, 0, LOCK_BSD, LOCK_EX|LOCK_NB) == -EAGAIN); + fd = safe_close(fd); + + assert_se((fd = copy_file_at(tfd, "abc/def", tfd, "poi", 0, 0644, COPY_LOCK_BSD))); + assert_se(read_file_at_and_streq(tfd, "poi", "abc\n")); + assert_se(xopenat_lock(tfd, "poi", 0, 0, 0, LOCK_BSD, LOCK_EX|LOCK_NB) == -EAGAIN); + fd = safe_close(fd); +} + DEFINE_TEST_MAIN(LOG_DEBUG); From d54f60c2af4bb5b0df9861b40f2a2609bacb768b Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 28 Mar 2023 12:32:51 +0200 Subject: [PATCH 3/3] btrfs-util: Add BTRFS_SNAPSHOT_LOCK_BSD When making ephemeral snapshots of subvolumes whose cleanup depends on whether they're locked or not, it's necessary to have the lock from the very beginning, so let's support that with a new BTRFS_SNAPSHOT_LOCK_BSD flag. --- src/shared/btrfs-util.c | 49 ++++++++++++++++++++++++++++++++++++----- src/shared/btrfs-util.h | 1 + src/test/test-btrfs.c | 19 ++++++++++++++-- 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c index f86645e4ee..5128b308ab 100644 --- a/src/shared/btrfs-util.c +++ b/src/shared/btrfs-util.c @@ -1363,9 +1363,26 @@ static int subvol_snapshot_children( if (ioctl(new_fd, BTRFS_IOC_SNAP_CREATE_V2, &vol_args) < 0) return -errno; + if (FLAGS_SET(flags, BTRFS_SNAPSHOT_LOCK_BSD)) { + subvolume_fd = xopenat_lock(new_fd, subvolume, + O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW, + /* xopen_flags = */ 0, + /* mode = */ 0, + LOCK_BSD, + LOCK_EX); + if (subvolume_fd < 0) + return subvolume_fd; + + r = btrfs_is_subvol_fd(subvolume_fd); + if (r < 0) + return r; + if (r == 0) + return -EEXIST; + } + if (!(flags & BTRFS_SNAPSHOT_RECURSIVE) && !(flags & BTRFS_SNAPSHOT_QUOTA)) - return 0; + return flags & BTRFS_SNAPSHOT_LOCK_BSD ? TAKE_FD(subvolume_fd) : 0; if (old_subvol_id == 0) { r = btrfs_subvol_get_id_fd(old_fd, &old_subvol_id); @@ -1385,7 +1402,7 @@ static int subvol_snapshot_children( if (flags & BTRFS_SNAPSHOT_QUOTA) (void) copy_subtree_quota_limits(new_fd, old_subvol_id, new_subvol_id); - return 0; + return flags & BTRFS_SNAPSHOT_LOCK_BSD ? TAKE_FD(subvolume_fd) : 0; } args.key.min_offset = args.key.max_offset = old_subvol_id; @@ -1480,7 +1497,8 @@ static int subvol_snapshot_children( return k; } - r = subvol_snapshot_children(old_child_fd, new_child_fd, p, sh->objectid, flags & ~BTRFS_SNAPSHOT_FALLBACK_COPY); + r = subvol_snapshot_children(old_child_fd, new_child_fd, p, sh->objectid, + flags & ~(BTRFS_SNAPSHOT_FALLBACK_COPY|BTRFS_SNAPSHOT_LOCK_BSD)); /* Restore the readonly flag */ if (flags & BTRFS_SNAPSHOT_READ_ONLY) { @@ -1503,7 +1521,7 @@ static int subvol_snapshot_children( if (flags & BTRFS_SNAPSHOT_QUOTA) (void) copy_subtree_quota_limits(new_fd, old_subvol_id, new_subvol_id); - return 0; + return flags & BTRFS_SNAPSHOT_LOCK_BSD ? TAKE_FD(subvolume_fd) : 0; } int btrfs_subvol_snapshot_at_full( @@ -1517,7 +1535,7 @@ int btrfs_subvol_snapshot_at_full( void *userdata) { _cleanup_free_ char *subvolume = NULL; - _cleanup_close_ int old_fd = -EBADF, new_fd = -EBADF; + _cleanup_close_ int old_fd = -EBADF, new_fd = -EBADF, subvolume_fd = -EBADF; int r; assert(dir_fdf >= 0 || dir_fdf == AT_FDCWD); @@ -1556,6 +1574,25 @@ int btrfs_subvol_snapshot_at_full( } else if (r < 0) return r; + if (FLAGS_SET(flags, BTRFS_SNAPSHOT_LOCK_BSD)) { + subvolume_fd = xopenat_lock(new_fd, subvolume, + O_RDONLY|O_NOCTTY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW, + /* xopen_flags = */ 0, + /* mode = */ 0, + LOCK_BSD, + LOCK_EX); + if (subvolume_fd < 0) + return subvolume_fd; + + if (!plain_directory) { + r = btrfs_is_subvol_fd(subvolume_fd); + if (r < 0) + return r; + if (r == 0) + return -EEXIST; + } + } + r = copy_directory_at_full( dir_fdf, from, new_fd, subvolume, @@ -1587,7 +1624,7 @@ int btrfs_subvol_snapshot_at_full( } } - return 0; + return flags & BTRFS_SNAPSHOT_LOCK_BSD ? TAKE_FD(subvolume_fd) : 0; fallback_fail: (void) rm_rf_at(new_fd, subvolume, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_SUBVOLUME); diff --git a/src/shared/btrfs-util.h b/src/shared/btrfs-util.h index de38f1e45c..c972cc0744 100644 --- a/src/shared/btrfs-util.h +++ b/src/shared/btrfs-util.h @@ -36,6 +36,7 @@ typedef enum BtrfsSnapshotFlags { BTRFS_SNAPSHOT_FALLBACK_IMMUTABLE = 1 << 5, /* When we can't create a subvolume, use the FS_IMMUTABLE attribute for indicating read-only */ BTRFS_SNAPSHOT_SIGINT = 1 << 6, /* Check for SIGINT regularly, and return EINTR if seen */ BTRFS_SNAPSHOT_SIGTERM = 1 << 7, /* Ditto, but for SIGTERM */ + BTRFS_SNAPSHOT_LOCK_BSD = 1 << 8, /* Return a BSD exclusively locked file descriptor referring to snapshot subvolume/directory. */ } BtrfsSnapshotFlags; typedef enum BtrfsRemoveFlags { diff --git a/src/test/test-btrfs.c b/src/test/test-btrfs.c index ccb1661a91..ee4772b983 100644 --- a/src/test/test-btrfs.c +++ b/src/test/test-btrfs.c @@ -4,6 +4,7 @@ #include "btrfs-util.h" #include "fd-util.h" +#include "fs-util.h" #include "fileio.h" #include "format-util.h" #include "log.h" @@ -62,6 +63,14 @@ int main(int argc, char *argv[]) { if (r < 0) log_error_errno(r, "Failed to make snapshot: %m"); + r = btrfs_subvol_snapshot_at(AT_FDCWD, "/xxxtest", AT_FDCWD, "/xxxtest4", BTRFS_SNAPSHOT_LOCK_BSD); + if (r < 0) + log_error_errno(r, "Failed to make snapshot: %m"); + if (r >= 0) + assert_se(xopenat_lock(AT_FDCWD, "/xxxtest4", 0, 0, 0, LOCK_BSD, LOCK_EX|LOCK_NB) == -EAGAIN); + + safe_close(r); + r = btrfs_subvol_remove("/xxxtest", BTRFS_REMOVE_QUOTA); if (r < 0) log_error_errno(r, "Failed to remove subvolume: %m"); @@ -74,6 +83,10 @@ int main(int argc, char *argv[]) { if (r < 0) log_error_errno(r, "Failed to remove subvolume: %m"); + r = btrfs_subvol_remove("/xxxtest4", BTRFS_REMOVE_QUOTA); + if (r < 0) + log_error_errno(r, "Failed to remove subvolume: %m"); + r = btrfs_subvol_snapshot_at(AT_FDCWD, "/etc", AT_FDCWD, "/etc2", BTRFS_SNAPSHOT_READ_ONLY|BTRFS_SNAPSHOT_FALLBACK_COPY); if (r < 0) @@ -161,13 +174,15 @@ int main(int argc, char *argv[]) { if (r < 0) log_error_errno(r, "Failed to query quota: %m"); - assert_se(quota.referenced_max == 4ULL * 1024 * 1024 * 1024); + if (r >= 0) + assert_se(quota.referenced_max == 4ULL * 1024 * 1024 * 1024); r = btrfs_subvol_get_subtree_quota("/xxxquotatest2", 0, "a); if (r < 0) log_error_errno(r, "Failed to query quota: %m"); - assert_se(quota.referenced_max == 5ULL * 1024 * 1024 * 1024); + if (r >= 0) + assert_se(quota.referenced_max == 5ULL * 1024 * 1024 * 1024); r = btrfs_subvol_remove("/xxxquotatest", BTRFS_REMOVE_QUOTA|BTRFS_REMOVE_RECURSIVE); if (r < 0)