From a789f72ed23afeb4324b880c5207d067ff5ff11c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 20 Apr 2023 18:45:09 +0200 Subject: [PATCH 1/4] mount-util: add umount_and_free() helper --- src/shared/mount-util.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/shared/mount-util.h b/src/shared/mount-util.h index d63fddeb10..8a84d61622 100644 --- a/src/shared/mount-util.h +++ b/src/shared/mount-util.h @@ -83,6 +83,14 @@ static inline char* umount_and_rmdir_and_free(char *p) { } DEFINE_TRIVIAL_CLEANUP_FUNC(char*, umount_and_rmdir_and_free); +static inline char *umount_and_free(char *p) { + PROTECT_ERRNO; + if (p) + (void) umount_recursive(p, 0); + return mfree(p); +} +DEFINE_TRIVIAL_CLEANUP_FUNC(char*, umount_and_free); + int bind_mount_in_namespace(pid_t target, const char *propagate_path, const char *incoming_path, const char *src, const char *dest, bool read_only, bool make_file_or_directory); int mount_image_in_namespace(pid_t target, const char *propagate_path, const char *incoming_path, const char *src, const char *dest, bool read_only, bool make_file_or_directory, const MountOptions *options, const ImagePolicy *image_policy); From 3690bfb5b2279b75840d19a9b2763b00318b05c7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 15 May 2023 11:35:15 +0200 Subject: [PATCH 2/4] namespace: introduce a common dir in /run/ that we can use to see new root fs up on This creates a new dir /run/systemd/mount-rootfs/ early in PID 1 that thus always exists. It's supposed to be used by any code that creates its own mount namespace and then sets up a new root dir to switch into. So far in many cases we used a temporary dir (which needed explicit clean-up) or a purpose-specific fixed dir. Let's create a common dir instead, that always exists (as it is created in PID 1 early on, always). Besides making things more robust, as manual clean-up of the inode is not necessary anymore this also opens the door for unprivileged programs to use the same dir, since it now always exists. Set the access mode to 555 (instead of the otherwise previously used 0755, 0700 or similar), so that unprivileged programs can access it, but we make clear it's not supposed to be written directly to, by anyone, not even root. --- src/shared/mount-setup.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/shared/mount-setup.c b/src/shared/mount-setup.c index a920e8a92a..901c3cd777 100644 --- a/src/shared/mount-setup.c +++ b/src/shared/mount-setup.c @@ -554,6 +554,11 @@ int mount_setup(bool loaded_policy, bool leave_propagation) { (void) mkdir_label("/run/systemd", 0755); (void) mkdir_label("/run/systemd/system", 0755); + /* Make sure there's always a place where sandboxed environments can mount root file systems they are + * about to move into, even when unprivileged, without having to create a temporary one in /tmp/ + * (which they then have to keep track of and clean) */ + (void) mkdir_label("/run/systemd/mount-rootfs", 0555); + /* Make sure we have a mount point to hide in sandboxes */ (void) mkdir_label("/run/credentials", 0755); From be79c23569f7e4065a5a26dbf15612bce28e6bb3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 15 May 2023 12:32:54 +0200 Subject: [PATCH 3/4] pid1: port unit namespacing to new /run/systemd/mount-rootfs dir --- src/core/namespace.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 2fcc096217..1116ece59d 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -2166,10 +2166,11 @@ int setup_namespace( * in the root. The temporary directory prevents any mounts from being potentially obscured * my other mounts we already applied. We use the same mount point for all images, which is * safe, since they all live in their own namespaces after all, and hence won't see each - * other. */ + * other. (Note: this directory is also created by PID 1 early on, we create it here for + * similar reasons as /run/systemd/ first.) */ + root = "/run/systemd/mount-rootfs"; + (void) mkdir_label(root, 0555); - root = "/run/systemd/unit-root"; - (void) mkdir_label(root, 0700); require_prefix = true; } From a4b3e9423696f604be33b4ab93c4bb5c6c807554 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 15 May 2023 11:49:48 +0200 Subject: [PATCH 4/4] dissect-image: port mount_image_privately_interactively() to use /run/systemd/mount-rootfs/ too Let's use the same common directory as the unit logic uses. This means we have less to clean up, and opens the door to eventually allow unprivileged operation of the mount_image_privately_interactively() logic. --- src/analyze/analyze.c | 6 ++-- src/boot/bootctl.c | 6 ++-- src/coredump/coredumpctl.c | 2 +- src/firstboot/firstboot.c | 6 ++-- src/journal/journalctl.c | 6 ++-- src/machine-id-setup/machine-id-setup-main.c | 6 ++-- src/partition/repart.c | 2 +- src/shared/dissect-image.c | 32 +++++++++++--------- src/systemctl/systemctl.c | 2 +- src/sysupdate/sysupdate.c | 2 +- src/sysusers/sysusers.c | 6 ++-- src/tmpfiles/tmpfiles.c | 6 ++-- 12 files changed, 43 insertions(+), 39 deletions(-) diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 09a38e7930..b555c713fc 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -599,7 +599,7 @@ static int parse_argv(int argc, char *argv[]) { static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *unlink_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; static const Verb verbs[] = { { "help", VERB_ANY, VERB_ANY, 0, help }, @@ -660,13 +660,13 @@ static int run(int argc, char *argv[]) { DISSECT_IMAGE_GENERIC_ROOT | DISSECT_IMAGE_RELAX_VAR_CHECK | DISSECT_IMAGE_READ_ONLY, - &unlink_dir, + &mounted_dir, /* ret_dir_fd= */ NULL, &loop_device); if (r < 0) return r; - arg_root = strdup(unlink_dir); + arg_root = strdup(mounted_dir); if (!arg_root) return log_oom(); } diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 0480e320c0..65608f5e83 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -445,7 +445,7 @@ static int bootctl_main(int argc, char *argv[]) { static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *unlink_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; int r; log_setup(); @@ -493,13 +493,13 @@ static int run(int argc, char *argv[]) { arg_image_policy, DISSECT_IMAGE_GENERIC_ROOT | DISSECT_IMAGE_RELAX_VAR_CHECK, - &unlink_dir, + &mounted_dir, /* ret_dir_fd= */ NULL, &loop_device); if (r < 0) return r; - arg_root = strdup(unlink_dir); + arg_root = strdup(mounted_dir); if (!arg_root) return log_oom(); } diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index bc52cc0b06..e34d74765e 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -1350,7 +1350,7 @@ static int coredumpctl_main(int argc, char *argv[]) { static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; int r, units_active; setlocale(LC_ALL, ""); diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 71b1e25c9d..6d50054baf 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -1506,7 +1506,7 @@ static int parse_argv(int argc, char *argv[]) { static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *unlink_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; _cleanup_close_ int rfd = -EBADF; int r; @@ -1546,13 +1546,13 @@ static int run(int argc, char *argv[]) { DISSECT_IMAGE_RELAX_VAR_CHECK | DISSECT_IMAGE_FSCK | DISSECT_IMAGE_GROWFS, - &unlink_dir, + &mounted_dir, &rfd, &loop_device); if (r < 0) return r; - arg_root = strdup(unlink_dir); + arg_root = strdup(mounted_dir); if (!arg_root) return log_oom(); } else { diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 12119b302c..62f74551ad 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -2112,7 +2112,7 @@ static int wait_for_change(sd_journal *j, int poll_fd) { static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *unlink_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; bool previous_boot_id_valid = false, first_line = true, ellipsized = false, need_seek = false, since_seeked = false; bool use_cursor = false, after_cursor = false; _cleanup_(sd_journal_closep) sd_journal *j = NULL; @@ -2143,13 +2143,13 @@ static int run(int argc, char *argv[]) { DISSECT_IMAGE_VALIDATE_OS | DISSECT_IMAGE_RELAX_VAR_CHECK | (arg_action == ACTION_UPDATE_CATALOG ? DISSECT_IMAGE_FSCK|DISSECT_IMAGE_GROWFS : DISSECT_IMAGE_READ_ONLY), - &unlink_dir, + &mounted_dir, /* ret_dir_fd= */ NULL, &loop_device); if (r < 0) return r; - arg_root = strdup(unlink_dir); + arg_root = strdup(mounted_dir); if (!arg_root) return log_oom(); } diff --git a/src/machine-id-setup/machine-id-setup-main.c b/src/machine-id-setup/machine-id-setup-main.c index 38d66def06..59aad985f8 100644 --- a/src/machine-id-setup/machine-id-setup-main.c +++ b/src/machine-id-setup/machine-id-setup-main.c @@ -136,7 +136,7 @@ static int parse_argv(int argc, char *argv[]) { static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *unlink_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; int r; log_parse_environment(); @@ -157,13 +157,13 @@ static int run(int argc, char *argv[]) { DISSECT_IMAGE_RELAX_VAR_CHECK | DISSECT_IMAGE_FSCK | DISSECT_IMAGE_GROWFS, - &unlink_dir, + &mounted_dir, /* ret_dir_fd= */ NULL, &loop_device); if (r < 0) return r; - arg_root = strdup(unlink_dir); + arg_root = strdup(mounted_dir); if (!arg_root) return log_oom(); } diff --git a/src/partition/repart.c b/src/partition/repart.c index 1f3e78a84e..995a40655d 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -6669,7 +6669,7 @@ static int determine_auto_size(Context *c) { static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; _cleanup_(context_freep) Context* context = NULL; bool node_is_our_loop = false; int r; diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index b84ef46442..39f75dd0dd 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -3667,8 +3667,7 @@ int mount_image_privately_interactively( _cleanup_(verity_settings_done) VeritySettings verity = VERITY_SETTINGS_DEFAULT; _cleanup_(loop_device_unrefp) LoopDevice *d = NULL; _cleanup_(dissected_image_unrefp) DissectedImage *dissected_image = NULL; - _cleanup_(rmdir_and_freep) char *created_dir = NULL; - _cleanup_free_ char *temp = NULL; + _cleanup_free_ char *dir = NULL; int r; /* Mounts an OS image at a temporary place, inside a newly created mount namespace of our own. This @@ -3676,7 +3675,6 @@ int mount_image_privately_interactively( * easily. */ assert(image); - assert(ret_directory); assert(ret_loop_device); /* We intend to mount this right-away, hence add the partitions if needed and pin them. */ @@ -3687,10 +3685,6 @@ int mount_image_privately_interactively( if (r < 0) return log_error_errno(r, "Failed to load root hash data: %m"); - r = tempfn_random_child(NULL, program_invocation_short_name, &temp); - if (r < 0) - return log_error_errno(r, "Failed to generate temporary mount directory: %m"); - r = loop_device_make_by_path( image, FLAGS_SET(flags, DISSECT_IMAGE_DEVICE_READ_ONLY) ? O_RDONLY : O_RDWR, @@ -3723,13 +3717,16 @@ int mount_image_privately_interactively( if (r < 0) return log_error_errno(r, "Failed to detach mount namespace: %m"); - r = mkdir_p(temp, 0700); + r = mkdir_p("/run/systemd/mount-rootfs", 0555); if (r < 0) return log_error_errno(r, "Failed to create mount point: %m"); - created_dir = TAKE_PTR(temp); - - r = dissected_image_mount_and_warn(dissected_image, created_dir, UID_INVALID, UID_INVALID, flags); + r = dissected_image_mount_and_warn( + dissected_image, + "/run/systemd/mount-rootfs", + /* uid_shift= */ UID_INVALID, + /* uid_range= */ UID_INVALID, + flags); if (r < 0) return r; @@ -3741,19 +3738,26 @@ int mount_image_privately_interactively( if (r < 0) return log_error_errno(r, "Failed to relinquish DM and loopback block devices: %m"); + if (ret_directory) { + dir = strdup("/run/systemd/mount-rootfs"); + if (!dir) + return log_oom(); + } + if (ret_dir_fd) { _cleanup_close_ int dir_fd = -EBADF; - dir_fd = open(created_dir, O_CLOEXEC|O_DIRECTORY); + dir_fd = open("/run/systemd/mount-rootfs", O_CLOEXEC|O_DIRECTORY); if (dir_fd < 0) return log_error_errno(errno, "Failed to open mount point directory: %m"); *ret_dir_fd = TAKE_FD(dir_fd); } - *ret_directory = TAKE_PTR(created_dir); - *ret_loop_device = TAKE_PTR(d); + if (ret_directory) + *ret_directory = TAKE_PTR(dir); + *ret_loop_device = TAKE_PTR(d); return 0; } diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 71c068b09e..b31a59785b 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1224,7 +1224,7 @@ static int systemctl_main(int argc, char *argv[]) { static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; int r; setlocale(LC_ALL, ""); diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 29cd552ea8..76777dc08e 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -861,7 +861,7 @@ static int process_image( LoopDevice **ret_loop_device) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; int r; assert(ret_mounted_dir); diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 58246b5d85..aa1f1356dc 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -2157,7 +2157,7 @@ static int read_credential_lines(void) { static int run(int argc, char *argv[]) { #ifndef STANDALONE _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *unlink_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; #endif _cleanup_close_ int lock = -EBADF; Item *i; @@ -2191,13 +2191,13 @@ static int run(int argc, char *argv[]) { DISSECT_IMAGE_RELAX_VAR_CHECK | DISSECT_IMAGE_FSCK | DISSECT_IMAGE_GROWFS, - &unlink_dir, + &mounted_dir, /* ret_dir_fd= */ NULL, &loop_device); if (r < 0) return r; - arg_root = strdup(unlink_dir); + arg_root = strdup(mounted_dir); if (!arg_root) return log_oom(); } diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 382fa8b56a..be04b25653 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -4223,7 +4223,7 @@ DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(item_array_hash_ops, char, string_ static int run(int argc, char *argv[]) { #ifndef STANDALONE _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *unlink_dir = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; #endif _cleanup_strv_free_ char **config_dirs = NULL; bool invalid_config = false; @@ -4314,13 +4314,13 @@ static int run(int argc, char *argv[]) { DISSECT_IMAGE_RELAX_VAR_CHECK | DISSECT_IMAGE_FSCK | DISSECT_IMAGE_GROWFS, - &unlink_dir, + &mounted_dir, /* ret_dir_fd= */ NULL, &loop_device); if (r < 0) return r; - arg_root = strdup(unlink_dir); + arg_root = strdup(mounted_dir); if (!arg_root) return log_oom(); }