From 595ca10f3768d13000c41c8e51239b90d9cbd7ba Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 19 Dec 2024 14:26:14 +0100 Subject: [PATCH 01/10] nspawn: use DEVNUM_FORMAT_STR/DEVNUM_FORMAT_VAL more --- src/nspawn/nspawn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index d465cd58e2..fbddc5948b 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -42,6 +42,7 @@ #include "copy.h" #include "cpu-set-util.h" #include "dev-setup.h" +#include "devnum-util.h" #include "discover-image.h" #include "dissect-image.h" #include "env-util.h" @@ -2323,7 +2324,7 @@ static int copy_devnode_one(const char *dest, const char *node, bool ignore_mkno return log_error_errno(r, "Failed to create '%s': %m", dn); _cleanup_free_ char *sl = NULL; - if (asprintf(&sl, "%s/%u:%u", dn, major(st.st_rdev), minor(st.st_rdev)) < 0) + if (asprintf(&sl, "%s/" DEVNUM_FORMAT_STR, dn, DEVNUM_FORMAT_VAL(st.st_rdev)) < 0) return log_oom(); _cleanup_free_ char *prefixed = path_join(dest, sl); From 5229cd839a0034f2f3ecb06a8b3f6fd2ded825b0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Nov 2024 12:18:14 +0100 Subject: [PATCH 02/10] nspawn: rename 'fd' variable to something more descriptive --- src/nspawn/nspawn.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index fbddc5948b..74e9fa04bd 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3933,7 +3933,7 @@ static int outer_child( _cleanup_(bind_user_context_freep) BindUserContext *bind_user_context = NULL; _cleanup_strv_free_ char **os_release_pairs = NULL; - _cleanup_close_ int fd = -EBADF, mntns_fd = -EBADF; + _cleanup_close_ int mntns_fd = -EBADF; bool idmap = false, enable_fuse; const char *p; pid_t pid; @@ -4326,6 +4326,7 @@ static int outer_child( * visible. Hence there we do it the other way round: we first allocate a new set of namespaces * (and fork for it) for which we then mount sysfs/procfs, and only then switch root. */ + _cleanup_close_ int notify_fd = -EBADF; if (arg_privileged) { /* Mark everything as shared so our mounts get propagated down. This is required to make new * bind mounts available in systemd services inside the container that create a new mount @@ -4361,11 +4362,11 @@ static int outer_child( return r; } - fd = setup_notify_child(NULL); + notify_fd = setup_notify_child(NULL); } else - fd = setup_notify_child(directory); - if (fd < 0) - return fd; + notify_fd = setup_notify_child(directory); + if (notify_fd < 0) + return notify_fd; pid = raw_clone(SIGCHLD|CLONE_NEWNS| arg_clone_ns_flags | @@ -4430,7 +4431,7 @@ static int outer_child( return log_error_errno(SYNTHETIC_ERRNO(EIO), "Short write while sending machine ID."); - l = send_one_fd(fd_outer_socket, fd, 0); + l = send_one_fd(fd_outer_socket, notify_fd, 0); if (l < 0) return log_error_errno(l, "Failed to send notify fd: %m"); From 8f9ea89ce4656831cd75b042bfa31b368f12f076 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Nov 2024 12:08:59 +0100 Subject: [PATCH 03/10] nspawn: make unexpected mkdir() failures fatal THis is just to make things easier to debug. --- src/nspawn/nspawn-mount.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index ddbdba6fb6..622a3d38e6 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -475,7 +475,8 @@ int mount_sysfs(const char *dest, MountSettingsMask mount_settings) { if (!full) return log_oom(); - (void) mkdir(full, 0755); + if (mkdir(full, 0755) < 0 && errno != EEXIST) + return log_error_errno(errno, "Failed to create directory '%s': %m", full); if (FLAGS_SET(mount_settings, MOUNT_APPLY_APIVFS_RO)) extra_flags |= MS_RDONLY; From bf1ef54d307fcf0b50028959ba51263e91f9f11c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Nov 2024 11:54:21 +0100 Subject: [PATCH 04/10] nspawn: add some additional useful debug logging --- src/nspawn/nspawn-mount.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 622a3d38e6..eda455c871 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -1409,6 +1409,8 @@ done: int pin_fully_visible_fs(void) { int r; + log_debug("Pinning fully visible API FS"); + (void) mkdir_p(NSPAWN_PRIVATE_FULLY_VISIBLE_PROCFS, 0755); (void) mkdir_p(NSPAWN_PRIVATE_FULLY_VISIBLE_SYSFS, 0755); @@ -1443,6 +1445,8 @@ int wipe_fully_visible_fs(int mntns_fd) { _cleanup_close_ int orig_mntns_fd = -EBADF; int r, rr; + log_debug("Wiping fully visible API FS"); + r = namespace_open(0, /* ret_pidns_fd = */ NULL, &orig_mntns_fd, From b83358b87f0f6feaffca75f0ebd1d9fd80f34217 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Nov 2024 11:32:35 +0100 Subject: [PATCH 05/10] =?UTF-8?q?nspawn:=20rename=20pin=5Ffully=5Fvisible?= =?UTF-8?q?=5Ffs()=20=E2=86=92=20pin=5Ffully=5Fvisible=5Fapi=5Ffs()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function pins the *API* FS, i.e. /proc/ + /sys/, not just any fs. Hence clarify this in the name. (At least we call these two fs "API (V)FS" in our codebase, hence continue to do so here) --- src/nspawn/nspawn-mount.c | 8 ++++---- src/nspawn/nspawn-mount.h | 5 +++-- src/nspawn/nspawn.c | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index eda455c871..af88e82d04 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -1406,7 +1406,7 @@ done: #define NSPAWN_PRIVATE_FULLY_VISIBLE_PROCFS "/run/host/proc" #define NSPAWN_PRIVATE_FULLY_VISIBLE_SYSFS "/run/host/sys" -int pin_fully_visible_fs(void) { +int pin_fully_visible_api_fs(void) { int r; log_debug("Pinning fully visible API FS"); @@ -1425,7 +1425,7 @@ int pin_fully_visible_fs(void) { return 0; } -static int do_wipe_fully_visible_fs(void) { +static int do_wipe_fully_visible_api_fs(void) { if (umount2(NSPAWN_PRIVATE_FULLY_VISIBLE_PROCFS, MNT_DETACH) < 0) return log_error_errno(errno, "Failed to unmount temporary proc: %m"); @@ -1441,7 +1441,7 @@ static int do_wipe_fully_visible_fs(void) { return 0; } -int wipe_fully_visible_fs(int mntns_fd) { +int wipe_fully_visible_api_fs(int mntns_fd) { _cleanup_close_ int orig_mntns_fd = -EBADF; int r, rr; @@ -1464,7 +1464,7 @@ int wipe_fully_visible_fs(int mntns_fd) { if (r < 0) return log_error_errno(r, "Failed to enter mount namespace: %m"); - rr = do_wipe_fully_visible_fs(); + rr = do_wipe_fully_visible_api_fs(); r = namespace_enter(/* pidns_fd = */ -EBADF, orig_mntns_fd, diff --git a/src/nspawn/nspawn-mount.h b/src/nspawn/nspawn-mount.h index 5f66bc7328..53aa993d6a 100644 --- a/src/nspawn/nspawn-mount.h +++ b/src/nspawn/nspawn-mount.h @@ -73,5 +73,6 @@ int pivot_root_parse(char **pivot_root_new, char **pivot_root_old, const char *s int setup_pivot_root(const char *directory, const char *pivot_root_new, const char *pivot_root_old); int tmpfs_patch_options(const char *options,uid_t uid_shift, const char *selinux_apifs_context, char **ret); -int pin_fully_visible_fs(void); -int wipe_fully_visible_fs(int mntns_fd); + +int pin_fully_visible_api_fs(void); +int wipe_fully_visible_api_fs(int mntns_fd); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 74e9fa04bd..8a07000fce 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4357,7 +4357,7 @@ static int outer_child( * Note, the inner child wouldn't be able to unmount the instances on its own since * it doesn't own the originating mount namespace. IOW, the outer child needs to do * this. */ - r = pin_fully_visible_fs(); + r = pin_fully_visible_api_fs(); if (r < 0) return r; } @@ -5625,7 +5625,7 @@ static int run_container( return log_error_errno(SYNTHETIC_ERRNO(ESRCH), "Child died too early."); if (arg_userns_mode != USER_NAMESPACE_NO) { - r = wipe_fully_visible_fs(mntns_fd); + r = wipe_fully_visible_api_fs(mntns_fd); if (r < 0) return r; mntns_fd = safe_close(mntns_fd); From 009a02b26383a0b252d4331dd74604600339b68d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Nov 2024 12:15:08 +0100 Subject: [PATCH 06/10] nspawn: trivial improvements --- src/nspawn/nspawn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 8a07000fce..f0753d18ae 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -1251,7 +1251,7 @@ static int parse_argv(int argc, char *argv[]) { arg_uid_range = UINT32_C(0x10000); } else if (streq(optarg, "identity")) { - /* identity: User namespaces on, UID range is map the 0…0xFFFF range to + /* identity: User namespaces on, UID range is map of the 0…0xFFFF range to * itself, i.e. we don't actually map anything, but do take benefit of * isolation of capability sets. */ arg_userns_mode = USER_NAMESPACE_FIXED; @@ -3255,6 +3255,7 @@ static int chase_and_update(char **p, unsigned flags) { } static int determine_uid_shift(const char *directory) { + assert(directory); if (arg_userns_mode == USER_NAMESPACE_NO) { arg_uid_shift = 0; From 91cdc8ab0ffdd324b5ebd8be74aea35d7c22f3d7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Nov 2024 11:35:11 +0100 Subject: [PATCH 07/10] mount-util: add debug output when we switched root --- src/shared/mount-util.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index 576f7e83aa..c8486b1b4a 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -536,6 +536,8 @@ int mount_switch_root_full(const char *path, unsigned long mount_propagation_fla } } + log_debug("Successfully switched root to '%s'.", path); + /* Finally, let's establish the requested propagation flags. */ if (mount_propagation_flag == 0) return 0; From b1b128d0e298942deb4a2f6a298f476b9e285a0f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 19 Dec 2024 14:27:03 +0100 Subject: [PATCH 08/10] mount-util: add debug message to make_userns() failure --- src/shared/mount-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index c8486b1b4a..756fb74c35 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -1321,7 +1321,7 @@ int make_userns(uid_t uid_shift, uid_t uid_range, uid_t source_owner, uid_t dest * process whose only purpose is to give us a new user namespace. It's killed when we got it. */ if (!userns_shift_range_valid(uid_shift, uid_range)) - return -EINVAL; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid UID range for user namespace."); if (IN_SET(idmapping, REMOUNT_IDMAPPING_NONE, REMOUNT_IDMAPPING_HOST_ROOT)) { if (asprintf(&line, UID_FMT " " UID_FMT " " UID_FMT "\n", 0u, uid_shift, uid_range) < 0) From 312cf910054f6d8e15787a785568ba96fdd8011b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Nov 2024 14:19:09 +0100 Subject: [PATCH 09/10] nsresource: print nicer error message when trying to acquire an unpriv user ns range where this isn't possible --- src/shared/nsresource.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shared/nsresource.c b/src/shared/nsresource.c index 24ea2e2be5..2475315315 100644 --- a/src/shared/nsresource.c +++ b/src/shared/nsresource.c @@ -87,6 +87,8 @@ int nsresource_allocate_userns(const char *name, uint64_t size) { SD_JSON_BUILD_PAIR("userNamespaceFileDescriptor", SD_JSON_BUILD_UNSIGNED(userns_fd_idx))); if (r < 0) return log_debug_errno(r, "Failed to call AllocateUserRange() varlink call: %m"); + if (streq_ptr(error_id, "io.systemd.NamespaceResource.UserNamespaceInterfaceNotSupported")) + return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Unprivileged user namespace delegation is not supported on this system."); if (error_id) return log_debug_errno(sd_varlink_error_to_errno(error_id, reply), "Failed to allocate user namespace with %" PRIu64 " users: %s", size, error_id); From 5ceb38cb1e2efc58b85c1006c1d5a3d4005a99cf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Nov 2024 12:25:22 +0100 Subject: [PATCH 10/10] nspawn: switch to read_virtual_file() for reading audit loginuid --- src/nspawn/nspawn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index f0753d18ae..66a8771eea 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2848,7 +2848,7 @@ static int reset_audit_loginuid(void) { if (!arg_privileged) return 0; - r = read_one_line_file("/proc/self/loginuid", &p); + r = read_virtual_file("/proc/self/loginuid", SIZE_MAX, &p, /* ret_size= */ NULL); if (r == -ENOENT) return 0; if (r < 0)