From 5f0a6347acf0da462cd5ac6d913ffa28e7463ef5 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Fri, 6 Dec 2019 22:45:14 +0100 Subject: [PATCH 1/4] nspawn: Enable specifying root as the mount target directory. Fixes #3847. --- src/nspawn/nspawn-mount.c | 17 ++++++++--------- src/nspawn/nspawn-mount.h | 4 +++- src/nspawn/nspawn.c | 23 ++++++++++++++++++++--- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 6407503c4c9..8225cf473a5 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -222,8 +222,6 @@ int bind_mount_parse(CustomMount **l, size_t *n, const char *s, bool read_only) if (!path_is_absolute(destination)) return -EINVAL; - if (empty_or_root(destination)) - return -EINVAL; m = custom_mount_add(l, n, CUSTOM_MOUNT_BIND); if (!m) @@ -262,8 +260,6 @@ int tmpfs_mount_parse(CustomMount **l, size_t *n, const char *s) { if (!path_is_absolute(path)) return -EINVAL; - if (empty_or_root(path)) - return -EINVAL; m = custom_mount_add(l, n, CUSTOM_MOUNT_TMPFS); if (!m) @@ -323,9 +319,6 @@ int overlay_mount_parse(CustomMount **l, size_t *n, const char *s, bool read_onl return -EINVAL; } - if (empty_or_root(destination)) - return -EINVAL; - m = custom_mount_add(l, n, CUSTOM_MOUNT_OVERLAY); if (!m) return -ENOMEM; @@ -923,7 +916,7 @@ int mount_custom( CustomMount *mounts, size_t n, bool userns, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context, - bool in_userns) { + MountSettingsMask mount_settings) { size_t i; int r; @@ -933,7 +926,13 @@ int mount_custom( for (i = 0; i < n; i++) { CustomMount *m = mounts + i; - if (m->in_userns != in_userns) + if ((mount_settings & MOUNT_IN_USERNS) != m->in_userns) + continue; + + if (mount_settings & MOUNT_ROOT_ONLY && !path_equal(m->destination, "/")) + continue; + + if (mount_settings & MOUNT_NON_ROOT_ONLY && path_equal(m->destination, "/")) continue; switch (m->type) { diff --git a/src/nspawn/nspawn-mount.h b/src/nspawn/nspawn-mount.h index ff6990c7344..08d3e68f291 100644 --- a/src/nspawn/nspawn-mount.h +++ b/src/nspawn/nspawn-mount.h @@ -14,6 +14,8 @@ typedef enum MountSettingsMask { MOUNT_APPLY_APIVFS_NETNS = 1 << 4, /* if set, /proc/sys/net will be mounted read-write. Works only if MOUNT_APPLY_APIVFS_RO is also set. */ MOUNT_APPLY_TMPFS_TMP = 1 << 5, /* if set, /tmp will be mounted as tmpfs */ + MOUNT_ROOT_ONLY = 1 << 6, /* if set, only root mounts are mounted */ + MOUNT_NON_ROOT_ONLY = 1 << 7, /* if set, only non-root mounts are mounted */ } MountSettingsMask; typedef enum CustomMountType { @@ -52,7 +54,7 @@ int inaccessible_mount_parse(CustomMount **l, size_t *n, const char *s); int mount_all(const char *dest, MountSettingsMask mount_settings, uid_t uid_shift, const char *selinux_apifs_context); int mount_sysfs(const char *dest, MountSettingsMask mount_settings); -int mount_custom(const char *dest, CustomMount *mounts, size_t n, bool userns, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context, bool in_userns); +int mount_custom(const char *dest, CustomMount *mounts, size_t n, bool userns, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context, MountSettingsMask mount_settings); int setup_volatile_mode(const char *directory, VolatileMode mode, bool userns, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 9fac3262196..31a9a6d11f5 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2979,7 +2979,7 @@ static int inner_child( 0, 0, arg_selinux_apifs_context, - true); + MOUNT_NON_ROOT_ONLY | MOUNT_IN_USERNS); if (r < 0) return r; @@ -3371,6 +3371,18 @@ static int outer_child( if (r < 0) return r; + r = mount_custom( + directory, + arg_custom_mounts, + arg_n_custom_mounts, + arg_userns_mode != USER_NAMESPACE_NO, + arg_uid_shift, + arg_uid_range, + arg_selinux_apifs_context, + MOUNT_ROOT_ONLY); + 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(dissected_image, directory, arg_uid_shift, @@ -3401,7 +3413,12 @@ static int outer_child( * inside the container that create a new mount namespace. * See https://github.com/systemd/systemd/issues/3860 * Further submounts (such as /dev) done after this will inherit the - * shared propagation mode. */ + * shared propagation mode. + * + * IMPORTANT: Do not overmount the root directory anymore from now on to + * enable moving the root directory mount to root later on. + * https://github.com/systemd/systemd/issues/3847#issuecomment-562735251 + */ r = mount_verbose(LOG_ERR, NULL, directory, NULL, MS_SHARED|MS_REC, NULL); if (r < 0) return r; @@ -3474,7 +3491,7 @@ static int outer_child( arg_uid_shift, arg_uid_range, arg_selinux_apifs_context, - false); + MOUNT_NON_ROOT_ONLY); if (r < 0) return r; From e091a5dfd162822b9ace2a84f127661301b61328 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 7 Dec 2019 11:59:59 +0100 Subject: [PATCH 2/4] nspawn-mount: Remove unused parameters --- src/nspawn/nspawn-mount.c | 33 ++++++++++----------------------- src/nspawn/nspawn-mount.h | 4 ++-- src/nspawn/nspawn.c | 8 -------- 3 files changed, 12 insertions(+), 33 deletions(-) diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 8225cf473a5..521991f412a 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -775,11 +775,7 @@ static int mount_bind(const char *dest, CustomMount *m) { return 0; } -static int mount_tmpfs( - const char *dest, - CustomMount *m, - bool userns, uid_t uid_shift, uid_t uid_range, - const char *selinux_apifs_context) { +static int mount_tmpfs(const char *dest, CustomMount *m, uid_t uid_shift, const char *selinux_apifs_context) { const char *options; _cleanup_free_ char *buf = NULL, *where = NULL; @@ -914,7 +910,7 @@ static int mount_arbitrary(const char *dest, CustomMount *m) { int mount_custom( const char *dest, CustomMount *mounts, size_t n, - bool userns, uid_t uid_shift, uid_t uid_range, + uid_t uid_shift, const char *selinux_apifs_context, MountSettingsMask mount_settings) { @@ -942,7 +938,7 @@ int mount_custom( break; case CUSTOM_MOUNT_TMPFS: - r = mount_tmpfs(dest, m, userns, uid_shift, uid_range, selinux_apifs_context); + r = mount_tmpfs(dest, m, uid_shift, selinux_apifs_context); break; case CUSTOM_MOUNT_OVERLAY: @@ -968,10 +964,7 @@ int mount_custom( return 0; } -static int setup_volatile_state( - const char *directory, - bool userns, uid_t uid_shift, uid_t uid_range, - const char *selinux_apifs_context) { +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; @@ -1000,10 +993,7 @@ static int setup_volatile_state( return mount_verbose(LOG_ERR, "tmpfs", p, "tmpfs", MS_STRICTATIME, options); } -static int setup_volatile_yes( - const char *directory, - bool userns, uid_t uid_shift, uid_t uid_range, - const char *selinux_apifs_context) { +static int setup_volatile_yes(const char *directory, uid_t uid_shift, const char *selinux_apifs_context) { bool tmpfs_mounted = false, bind_mounted = false; char template[] = "/tmp/nspawn-volatile-XXXXXX"; @@ -1090,10 +1080,7 @@ fail: return r; } -static int setup_volatile_overlay( - const char *directory, - bool userns, uid_t uid_shift, uid_t uid_range, - const char *selinux_apifs_context) { +static int setup_volatile_overlay(const char *directory, uid_t uid_shift, const char *selinux_apifs_context) { _cleanup_free_ char *buf = NULL, *escaped_directory = NULL, *escaped_upper = NULL, *escaped_work = NULL; char template[] = "/tmp/nspawn-volatile-XXXXXX"; @@ -1158,19 +1145,19 @@ finish: int setup_volatile_mode( const char *directory, VolatileMode mode, - bool userns, uid_t uid_shift, uid_t uid_range, + uid_t uid_shift, const char *selinux_apifs_context) { switch (mode) { case VOLATILE_YES: - return setup_volatile_yes(directory, userns, uid_shift, uid_range, selinux_apifs_context); + return setup_volatile_yes(directory, uid_shift, selinux_apifs_context); case VOLATILE_STATE: - return setup_volatile_state(directory, userns, uid_shift, uid_range, selinux_apifs_context); + return setup_volatile_state(directory, uid_shift, selinux_apifs_context); case VOLATILE_OVERLAY: - return setup_volatile_overlay(directory, userns, uid_shift, uid_range, selinux_apifs_context); + return setup_volatile_overlay(directory, uid_shift, selinux_apifs_context); default: return 0; diff --git a/src/nspawn/nspawn-mount.h b/src/nspawn/nspawn-mount.h index 08d3e68f291..aabc9e29bef 100644 --- a/src/nspawn/nspawn-mount.h +++ b/src/nspawn/nspawn-mount.h @@ -54,9 +54,9 @@ int inaccessible_mount_parse(CustomMount **l, size_t *n, const char *s); int mount_all(const char *dest, MountSettingsMask mount_settings, uid_t uid_shift, const char *selinux_apifs_context); int mount_sysfs(const char *dest, MountSettingsMask mount_settings); -int mount_custom(const char *dest, CustomMount *mounts, size_t n, bool userns, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context, MountSettingsMask mount_settings); +int mount_custom(const char *dest, CustomMount *mounts, size_t n, uid_t uid_shift, const char *selinux_apifs_context, MountSettingsMask mount_settings); -int setup_volatile_mode(const char *directory, VolatileMode mode, bool userns, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context); +int setup_volatile_mode(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 31a9a6d11f5..b85356e1ad0 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2975,8 +2975,6 @@ static int inner_child( "/", arg_custom_mounts, arg_n_custom_mounts, - false, - 0, 0, arg_selinux_apifs_context, MOUNT_NON_ROOT_ONLY | MOUNT_IN_USERNS); @@ -3364,9 +3362,7 @@ static int outer_child( r = setup_volatile_mode( directory, arg_volatile_mode, - arg_userns_mode != USER_NAMESPACE_NO, arg_uid_shift, - arg_uid_range, arg_selinux_apifs_context); if (r < 0) return r; @@ -3375,9 +3371,7 @@ static int outer_child( directory, arg_custom_mounts, arg_n_custom_mounts, - arg_userns_mode != USER_NAMESPACE_NO, arg_uid_shift, - arg_uid_range, arg_selinux_apifs_context, MOUNT_ROOT_ONLY); if (r < 0) @@ -3487,9 +3481,7 @@ static int outer_child( directory, arg_custom_mounts, arg_n_custom_mounts, - arg_userns_mode != USER_NAMESPACE_NO, arg_uid_shift, - arg_uid_range, arg_selinux_apifs_context, MOUNT_NON_ROOT_ONLY); if (r < 0) From 5530dc87f21c283cb629702cde71f30069e69820 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 7 Dec 2019 12:43:39 +0100 Subject: [PATCH 3/4] nspawn: Only bind-mount directory when necessary. --- src/nspawn/nspawn.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index b85356e1ad0..8a55ab20fa7 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3343,13 +3343,6 @@ static int outer_child( return r; directory = "/run/systemd/nspawn-root"; - - } else if (!dissected_image) { - /* Turn directory into bind mount (we need that so that we can move the bind mount to root - * later on). */ - r = mount_verbose(LOG_ERR, directory, directory, NULL, MS_BIND|MS_REC, NULL); - if (r < 0) - return r; } r = setup_pivot_root( @@ -3377,6 +3370,13 @@ static int outer_child( if (r < 0) return r; + /* Make sure we always have a mount that we can move to root later on. */ + if (!path_is_mount_point(directory, NULL, 0)) { + r = mount_verbose(LOG_ERR, directory, directory, NULL, MS_BIND|MS_REC, NULL); + 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(dissected_image, directory, arg_uid_shift, From bd6609eb11ec350218f37294808bbe06ebc538b9 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Thu, 12 Dec 2019 20:18:37 +0100 Subject: [PATCH 4/4] nspawn-mount: Use FLAGS_SET to check flags. --- src/nspawn/nspawn-mount.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 521991f412a..fa3bf77b0ed 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -415,7 +415,7 @@ int mount_sysfs(const char *dest, MountSettingsMask mount_settings) { (void) mkdir(full, 0755); - if (mount_settings & MOUNT_APPLY_APIVFS_RO) + if (FLAGS_SET(mount_settings, MOUNT_APPLY_APIVFS_RO)) extra_flags |= MS_RDONLY; r = mount_verbose(LOG_ERR, "sysfs", full, "sysfs", @@ -601,29 +601,29 @@ int mount_all(const char *dest, #endif }; - bool use_userns = (mount_settings & MOUNT_USE_USERNS); - bool netns = (mount_settings & MOUNT_APPLY_APIVFS_NETNS); - bool ro = (mount_settings & MOUNT_APPLY_APIVFS_RO); - bool in_userns = (mount_settings & MOUNT_IN_USERNS); - bool tmpfs_tmp = (mount_settings & MOUNT_APPLY_TMPFS_TMP); + bool use_userns = FLAGS_SET(mount_settings, MOUNT_USE_USERNS); + bool netns = FLAGS_SET(mount_settings, MOUNT_APPLY_APIVFS_NETNS); + bool ro = FLAGS_SET(mount_settings, MOUNT_APPLY_APIVFS_RO); + bool in_userns = FLAGS_SET(mount_settings, MOUNT_IN_USERNS); + bool tmpfs_tmp = FLAGS_SET(mount_settings, MOUNT_APPLY_TMPFS_TMP); size_t k; int r; for (k = 0; k < ELEMENTSOF(mount_table); k++) { _cleanup_free_ char *where = NULL, *options = NULL; const char *o; - bool fatal = (mount_table[k].mount_settings & MOUNT_FATAL); + bool fatal = FLAGS_SET(mount_table[k].mount_settings, MOUNT_FATAL); - if (in_userns != (bool)(mount_table[k].mount_settings & MOUNT_IN_USERNS)) + if (in_userns != FLAGS_SET(mount_table[k].mount_settings, MOUNT_IN_USERNS)) continue; - if (!netns && (bool)(mount_table[k].mount_settings & MOUNT_APPLY_APIVFS_NETNS)) + if (!netns && FLAGS_SET(mount_table[k].mount_settings, MOUNT_APPLY_APIVFS_NETNS)) continue; - if (!ro && (bool)(mount_table[k].mount_settings & MOUNT_APPLY_APIVFS_RO)) + if (!ro && FLAGS_SET(mount_table[k].mount_settings, MOUNT_APPLY_APIVFS_RO)) continue; - if (!tmpfs_tmp && (bool)(mount_table[k].mount_settings & MOUNT_APPLY_TMPFS_TMP)) + if (!tmpfs_tmp && FLAGS_SET(mount_table[k].mount_settings, MOUNT_APPLY_TMPFS_TMP)) continue; r = chase_symlinks(mount_table[k].where, dest, CHASE_NONEXISTENT|CHASE_PREFIX_ROOT, &where, NULL); @@ -922,13 +922,13 @@ int mount_custom( for (i = 0; i < n; i++) { CustomMount *m = mounts + i; - if ((mount_settings & MOUNT_IN_USERNS) != m->in_userns) + if (FLAGS_SET(mount_settings, MOUNT_IN_USERNS) != m->in_userns) continue; - if (mount_settings & MOUNT_ROOT_ONLY && !path_equal(m->destination, "/")) + if (FLAGS_SET(mount_settings, MOUNT_ROOT_ONLY) && !path_equal(m->destination, "/")) continue; - if (mount_settings & MOUNT_NON_ROOT_ONLY && path_equal(m->destination, "/")) + if (FLAGS_SET(mount_settings, MOUNT_NON_ROOT_ONLY) && path_equal(m->destination, "/")) continue; switch (m->type) {