From 21cd84df6931a358714af9d1f8cc54b26fef1a39 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 5 Sep 2024 13:56:33 +0900 Subject: [PATCH 1/4] nspawn: use strv_extend() and friends to build directories passed to remount_idmap() No functional change, just refactoring and preparation for later change. --- src/nspawn/nspawn.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 92077866a2..9b8c8973d8 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3993,22 +3993,22 @@ static int outer_child( if (arg_userns_mode != USER_NAMESPACE_NO && IN_SET(arg_userns_ownership, USER_NAMESPACE_OWNERSHIP_MAP, USER_NAMESPACE_OWNERSHIP_AUTO) && arg_uid_shift != 0) { - _cleanup_free_ char *usr_subtree = NULL; - char *dirs[3]; - size_t i = 0; + _cleanup_strv_free_ char **dirs = NULL; - dirs[i++] = (char*) directory; + r = strv_extend(&dirs, directory); + if (r < 0) + return log_oom(); if (dissected_image && dissected_image->partitions[PARTITION_USR].found) { - usr_subtree = path_join(directory, "/usr"); - if (!usr_subtree) + char *s = path_join(directory, "/usr"); + if (!s) return log_oom(); - dirs[i++] = usr_subtree; + r = strv_consume(&dirs, s); + if (r < 0) + return log_oom(); } - dirs[i] = NULL; - r = remount_idmap(dirs, arg_uid_shift, arg_uid_range, UID_INVALID, UID_INVALID, REMOUNT_IDMAPPING_HOST_ROOT); if (r == -EINVAL || ERRNO_IS_NEG_NOT_SUPPORTED(r)) { /* This might fail because the kernel or file system doesn't support idmapping. We From 2c2511aa734c507e04e06d273b474acacac9d486 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 6 Sep 2024 13:14:14 +0900 Subject: [PATCH 2/4] nspawn: mount /var/ after remount_idmap() when --volatile=state Previously, remount_idmap() failed as /var/ was already mounted, thus remounting (strictly speaking, unmounting old root directory) failed with -EBUSY. As tmpfs /var/ is mounted with picked UID shift, it should not be remounted with idmap, but needs to be mounted after the root directory being remounted. This makes '-U --volatile=state' work as expected. --- src/nspawn/nspawn-mount.c | 35 +++++++++++++++++++++++++++++++---- src/nspawn/nspawn-mount.h | 5 +++++ src/nspawn/nspawn.c | 8 ++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index c2bd4f6c30..874d54e734 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -1061,19 +1061,30 @@ bool has_custom_root_mount(const CustomMount *mounts, size_t n) { return false; } -static int setup_volatile_state(const char *directory, uid_t uid_shift, const char *selinux_apifs_context) { - _cleanup_free_ char *buf = NULL; - const char *p, *options; +static int setup_volatile_state(const char *directory) { int r; assert(directory); /* --volatile=state means we simply overmount /var with a tmpfs, and the rest read-only. */ + /* First, remount the root directory. */ r = bind_remount_recursive(directory, MS_RDONLY, MS_RDONLY, NULL); if (r < 0) return log_error_errno(r, "Failed to remount %s read-only: %m", directory); + return 0; +} + +static int setup_volatile_state_after_remount_idmap(const char *directory, uid_t uid_shift, const char *selinux_apifs_context) { + _cleanup_free_ char *buf = NULL; + const char *p, *options; + int r; + + assert(directory); + + /* Then, after remount_idmap(), overmount /var/ with a tmpfs. */ + p = prefix_roota(directory, "/var"); r = mkdir(p, 0755); if (r < 0 && errno != EEXIST) @@ -1249,7 +1260,7 @@ int setup_volatile_mode( return setup_volatile_yes(directory, uid_shift, selinux_apifs_context); case VOLATILE_STATE: - return setup_volatile_state(directory, uid_shift, selinux_apifs_context); + return setup_volatile_state(directory); case VOLATILE_OVERLAY: return setup_volatile_overlay(directory, uid_shift, selinux_apifs_context); @@ -1259,6 +1270,22 @@ int setup_volatile_mode( } } +int setup_volatile_mode_after_remount_idmap( + const char *directory, + VolatileMode mode, + uid_t uid_shift, + const char *selinux_apifs_context) { + + switch (mode) { + + case VOLATILE_STATE: + return setup_volatile_state_after_remount_idmap(directory, uid_shift, selinux_apifs_context); + + default: + return 0; + } +} + /* Expects *pivot_root_new and *pivot_root_old to be initialised to allocated memory or NULL. */ int pivot_root_parse(char **pivot_root_new, char **pivot_root_old, const char *s) { _cleanup_free_ char *root_new = NULL, *root_old = NULL; diff --git a/src/nspawn/nspawn-mount.h b/src/nspawn/nspawn-mount.h index 54dafa78d9..5f66bc7328 100644 --- a/src/nspawn/nspawn-mount.h +++ b/src/nspawn/nspawn-mount.h @@ -63,6 +63,11 @@ int mount_custom(const char *dest, CustomMount *mounts, size_t n, uid_t uid_shif bool has_custom_root_mount(const CustomMount *mounts, size_t n); int setup_volatile_mode(const char *directory, VolatileMode mode, uid_t uid_shift, const char *selinux_apifs_context); +int setup_volatile_mode_after_remount_idmap( + const char *directory, + VolatileMode mode, + uid_t uid_shift, + const char *selinux_apifs_context); 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); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 9b8c8973d8..655dc7c697 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4028,6 +4028,14 @@ static int outer_child( } } + r = setup_volatile_mode_after_remount_idmap( + directory, + arg_volatile_mode, + arg_uid_shift, + arg_selinux_apifs_context); + if (r < 0) + return r; + if (dissected_image) { /* Now we know the uid shift, let's now mount everything else that might be in the image. */ r = dissected_image_mount_and_warn( From 025be2361b3ee7cc3732fb3963d36b8583842064 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 6 Sep 2024 13:18:59 +0900 Subject: [PATCH 3/4] nspawn: only remount /usr/ with idmap when --volatile=yes The root directory is already mounted with a picked UID shift, hence it is not necessary to remount with idmap. However, /usr/ is a bind-mount, hence it must be remounted with idmap. With this change, now '-U --volatile=yes' works fine. Fixes #34254. --- src/nspawn/nspawn.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 655dc7c697..ec73e42709 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3995,11 +3995,14 @@ static int outer_child( arg_uid_shift != 0) { _cleanup_strv_free_ char **dirs = NULL; - r = strv_extend(&dirs, directory); - if (r < 0) - return log_oom(); + if (arg_volatile_mode != VOLATILE_YES) { + r = strv_extend(&dirs, directory); + if (r < 0) + return log_oom(); + } - if (dissected_image && dissected_image->partitions[PARTITION_USR].found) { + if ((dissected_image && dissected_image->partitions[PARTITION_USR].found) || + arg_volatile_mode == VOLATILE_YES) { char *s = path_join(directory, "/usr"); if (!s) return log_oom(); From 48878074d69a558484348f14e8887ce132938377 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 5 Sep 2024 14:12:20 +0900 Subject: [PATCH 4/4] test: add test cases for --volatile= with -U For issue #34254. --- test/units/TEST-13-NSPAWN.nspawn.sh | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/units/TEST-13-NSPAWN.nspawn.sh b/test/units/TEST-13-NSPAWN.nspawn.sh index 7c21825d63..8f16ac1a59 100755 --- a/test/units/TEST-13-NSPAWN.nspawn.sh +++ b/test/units/TEST-13-NSPAWN.nspawn.sh @@ -141,6 +141,37 @@ testcase_sanity() { test ! -e "$root/var/also-nope" test ! -e "$root/usr/nope-too" + # --volatile= with -U + touch "$root/usr/has-usr" + # volatile(=yes): rootfs is tmpfs, /usr/ from the OS tree is mounted read only + systemd-nspawn --directory="$root"\ + --volatile \ + -U \ + bash -xec 'test -e /usr/has-usr; touch /usr/read-only && exit 1; touch /nope' + test ! -e "$root/nope" + test ! -e "$root/usr/read-only" + systemd-nspawn --directory="$root"\ + --volatile=yes \ + -U \ + bash -xec 'test -e /usr/has-usr; touch /usr/read-only && exit 1; touch /nope' + test ! -e "$root/nope" + test ! -e "$root/usr/read-only" + # volatile=state: rootfs is read-only, /var/ is tmpfs + systemd-nspawn --directory="$root" \ + --volatile=state \ + -U \ + bash -xec 'test -e /usr/has-usr; mountpoint /var; touch /read-only && exit 1; touch /var/nope' + test ! -e "$root/read-only" + test ! -e "$root/var/nope" + # volatile=overlay: tmpfs overlay is mounted over rootfs + systemd-nspawn --directory="$root" \ + --volatile=overlay \ + -U \ + bash -xec 'test -e /usr/has-usr; touch /nope; touch /var/also-nope; touch /usr/nope-too' + test ! -e "$root/nope" + test ! -e "$root/var/also-nope" + test ! -e "$root/usr/nope-too" + # --machine=, --hostname= systemd-nspawn --directory="$root" \ --machine="foo-bar.baz" \