From 28faeda47f75dd9f34e36c48237cb376a1dd889c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 Nov 2017 20:48:46 +0100 Subject: [PATCH 01/10] path-util: when checking systemd versions, check both lib and lib64 We need to check both to be compatible with multilib images. --- src/basic/path-util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 3bde1d1e01b..056fa7d6332 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -882,7 +882,9 @@ int systemd_installation_has_version(const char *root, unsigned minimal_version) * for Gentoo which does a merge without making /lib a symlink. */ "lib/systemd/libsystemd-shared-*.so\0" - "usr/lib/systemd/libsystemd-shared-*.so\0") { + "lib64/systemd/libsystemd-shared-*.so\0" + "usr/lib/systemd/libsystemd-shared-*.so\0" + "usr/lib64/systemd/libsystemd-shared-*.so\0") { _cleanup_strv_free_ char **names = NULL; _cleanup_free_ char *path = NULL; From 8199d554c1d05acae2621e28e69b8efd819f789d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 Nov 2017 20:49:35 +0100 Subject: [PATCH 02/10] nspawn: figure out cgroup mode *after* mounting image If we operate on a disk image (i.e. --image=) then it's pointless to look into the mount directory before it is actually mounted to see which systemd version is running inside... Unfortunately we only mount the disk image in the child process, but the parent needs to know the cgroup mode, hence add some IPC for this purpose and communicate the cgroup mode determined from the image back to the parent. --- src/nspawn/nspawn.c | 66 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 63a55e85ac3..127229c06aa 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -320,7 +320,7 @@ static int custom_mount_check_all(void) { return 0; } -static int detect_unified_cgroup_hierarchy(const char *directory) { +static int detect_unified_cgroup_hierarchy_from_environment(void) { const char *e; int r; @@ -334,11 +334,16 @@ static int detect_unified_cgroup_hierarchy(const char *directory) { arg_unified_cgroup_hierarchy = CGROUP_UNIFIED_ALL; else arg_unified_cgroup_hierarchy = CGROUP_UNIFIED_NONE; - - return 0; } - /* Otherwise inherit the default from the host system */ + return 0; +} + +static int detect_unified_cgroup_hierarchy_from_image(const char *directory) { + int r; + + /* Let's inherit the mode to use from the host system, but let's take into consideration what systemd in the + * image actually supports. */ r = cg_all_unified(); if (r < 0) return log_error_errno(r, "Failed to determine whether we are in all unified mode."); @@ -364,6 +369,10 @@ static int detect_unified_cgroup_hierarchy(const char *directory) { } else arg_unified_cgroup_hierarchy = CGROUP_UNIFIED_NONE; + log_debug("Using %s hierarchy for container.", + arg_unified_cgroup_hierarchy == CGROUP_UNIFIED_NONE ? "legacy" : + arg_unified_cgroup_hierarchy == CGROUP_UNIFIED_SYSTEMD ? "hybrid" : "unified"); + return 0; } @@ -2522,6 +2531,7 @@ static int outer_child( int kmsg_socket, int rtnl_socket, int uid_shift_socket, + int unified_cgroup_hierarchy_socket, FDSet *fds) { pid_t pid; @@ -2608,6 +2618,24 @@ static int outer_child( log_info("Selected user namespace base " UID_FMT " and range " UID_FMT ".", arg_uid_shift, arg_uid_range); } + if (arg_unified_cgroup_hierarchy == CGROUP_UNIFIED_UNKNOWN) { + /* OK, we don't know yet which cgroup mode to use yet. Let's figure it out, and tell the parent. */ + + r = detect_unified_cgroup_hierarchy_from_image(directory); + if (r < 0) + return r; + + l = send(unified_cgroup_hierarchy_socket, &arg_unified_cgroup_hierarchy, sizeof(arg_unified_cgroup_hierarchy), MSG_NOSIGNAL); + if (l < 0) + return log_error_errno(errno, "Failed to send cgroup mode: %m"); + if (l != sizeof(arg_unified_cgroup_hierarchy)) { + log_error("Short write while sending cgroup mode: %m"); + return -EIO; + } + + unified_cgroup_hierarchy_socket = safe_close(unified_cgroup_hierarchy_socket); + } + /* Turn directory into bind mount */ r = mount_verbose(LOG_ERR, directory, directory, NULL, MS_BIND|MS_REC, NULL); if (r < 0) @@ -3254,7 +3282,9 @@ static int run(int master, pid_socket_pair[2] = { -1, -1 }, uuid_socket_pair[2] = { -1, -1 }, notify_socket_pair[2] = { -1, -1 }, - uid_shift_socket_pair[2] = { -1, -1 }; + uid_shift_socket_pair[2] = { -1, -1 }, + unified_cgroup_hierarchy_socket_pair[2] = { -1, -1}; + _cleanup_close_ int notify_socket= -1; _cleanup_(barrier_destroy) Barrier barrier = BARRIER_NULL; _cleanup_(sd_event_source_unrefp) sd_event_source *notify_event_source = NULL; @@ -3307,6 +3337,10 @@ static int run(int master, if (socketpair(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0, uid_shift_socket_pair) < 0) return log_error_errno(errno, "Failed to create uid shift socket pair: %m"); + if (arg_unified_cgroup_hierarchy == CGROUP_UNIFIED_UNKNOWN) + if (socketpair(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0, unified_cgroup_hierarchy_socket_pair) < 0) + return log_error_errno(errno, "Failed to create unified cgroup socket pair: %m"); + /* Child can be killed before execv(), so handle SIGCHLD in order to interrupt * parent's blocking calls and give it a chance to call wait() and terminate. */ r = sigprocmask(SIG_UNBLOCK, &mask_chld, NULL); @@ -3335,6 +3369,7 @@ static int run(int master, uuid_socket_pair[0] = safe_close(uuid_socket_pair[0]); notify_socket_pair[0] = safe_close(notify_socket_pair[0]); uid_shift_socket_pair[0] = safe_close(uid_shift_socket_pair[0]); + unified_cgroup_hierarchy_socket_pair[0] = safe_close(unified_cgroup_hierarchy_socket_pair[0]); (void) reset_all_signal_handlers(); (void) reset_signal_mask(); @@ -3351,6 +3386,7 @@ static int run(int master, kmsg_socket_pair[1], rtnl_socket_pair[1], uid_shift_socket_pair[1], + unified_cgroup_hierarchy_socket_pair[1], fds); if (r < 0) _exit(EXIT_FAILURE); @@ -3368,6 +3404,7 @@ static int run(int master, uuid_socket_pair[1] = safe_close(uuid_socket_pair[1]); notify_socket_pair[1] = safe_close(notify_socket_pair[1]); uid_shift_socket_pair[1] = safe_close(uid_shift_socket_pair[1]); + unified_cgroup_hierarchy_socket_pair[1] = safe_close(unified_cgroup_hierarchy_socket_pair[1]); if (arg_userns_mode != USER_NAMESPACE_NO) { /* The child just let us know the UID shift it might have read from the image. */ @@ -3398,6 +3435,17 @@ static int run(int master, } } + if (arg_unified_cgroup_hierarchy == CGROUP_UNIFIED_UNKNOWN) { + /* The child let us know the support cgroup mode it might have read from the image. */ + l = recv(unified_cgroup_hierarchy_socket_pair[0], &arg_unified_cgroup_hierarchy, sizeof(arg_unified_cgroup_hierarchy), 0); + if (l < 0) + return log_error_errno(errno, "Failed to read cgroup mode: %m"); + if (l != sizeof(arg_unified_cgroup_hierarchy)) { + log_error("Short read while reading cgroup mode."); + return -EIO; + } + } + /* Wait for the outer child. */ r = wait_for_terminate_and_warn("namespace helper", *pid, NULL); if (r != 0) @@ -3736,6 +3784,10 @@ int main(int argc, char *argv[]) { if (r < 0) goto finish; + r = detect_unified_cgroup_hierarchy_from_environment(); + if (r < 0) + goto finish; + n_fd_passed = sd_listen_fds(false); if (n_fd_passed > 0) { r = fdset_new_listen_fds(&fds, false); @@ -3983,10 +4035,6 @@ int main(int argc, char *argv[]) { if (r < 0) goto finish; - r = detect_unified_cgroup_hierarchy(arg_directory); - if (r < 0) - goto finish; - interactive = isatty(STDIN_FILENO) > 0 && isatty(STDOUT_FILENO) > 0; From 1cfdbe293f0e6266fc63bd807393f0813b8119f0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 Nov 2017 20:57:17 +0100 Subject: [PATCH 03/10] cgroup: also include "cgroups.threads" in the list of files to chown Also, add "cgroups.stat". It's read-only anyway, hence its UID/GID ownership matters little, but it's probably a good idea to keep it ownership in sync with the other read-only files such as "cgroups.controllers". Also, order the list of files alphabetically. --- src/nspawn/nspawn-cgroup.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/nspawn/nspawn-cgroup.c b/src/nspawn/nspawn-cgroup.c index 1820f33c2c8..4b07b38163d 100644 --- a/src/nspawn/nspawn-cgroup.c +++ b/src/nspawn/nspawn-cgroup.c @@ -41,13 +41,15 @@ static int chown_cgroup_path(const char *path, uid_t uid_shift) { FOREACH_STRING(fn, ".", - "tasks", - "notify_on_release", - "cgroup.procs", - "cgroup.events", "cgroup.clone_children", "cgroup.controllers", - "cgroup.subtree_control") + "cgroup.events", + "cgroup.procs", + "cgroup.stat", + "cgroup.subtree_control", + "cgroup.threads", + "notify_on_release", + "tasks") if (fchownat(fd, fn, uid_shift, uid_shift, 0) < 0) log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, errno, "Failed to chown \"%s/%s\", ignoring: %m", path, fn); From bb8ad9eacaaf08078ad08e289c4cef08c7c38663 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Nov 2017 16:37:53 +0100 Subject: [PATCH 04/10] string-util: rework strextend() to optionally inset separators between each appended string This adds a new flavour of strextend(), called strextend_with_separator(), which takes an optional separator string. If specified, the separator is inserted between each appended string, as well as before the first one, but only if the original string was non-empty. This new call is particularly useful when appending new options to mount option strings and suchlike, which need to be comma-separated, and initially start out from an empty string. --- src/basic/string-util.c | 28 +++++++++++++++++++++++----- src/basic/string-util.h | 4 +++- src/test/test-string-util.c | 35 ++++++++++++++++++++++++++++++++--- 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 312e83433e4..e916000b25b 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -734,16 +734,20 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz) { return obuf; } -char *strextend(char **x, ...) { - va_list ap; - size_t f, l; +char *strextend_with_separator(char **x, const char *separator, ...) { + bool need_separator; + size_t f, l, l_separator; char *r, *p; + va_list ap; assert(x); l = f = strlen_ptr(*x); - va_start(ap, x); + need_separator = !isempty(*x); + l_separator = strlen_ptr(separator); + + va_start(ap, separator); for (;;) { const char *t; size_t n; @@ -753,22 +757,29 @@ char *strextend(char **x, ...) { break; n = strlen(t); + + if (need_separator) + n += l_separator; + if (n > ((size_t) -1) - l) { va_end(ap); return NULL; } l += n; + need_separator = true; } va_end(ap); + need_separator = !isempty(*x); + r = realloc(*x, l+1); if (!r) return NULL; p = r + f; - va_start(ap, x); + va_start(ap, separator); for (;;) { const char *t; @@ -776,10 +787,17 @@ char *strextend(char **x, ...) { if (!t) break; + if (need_separator && separator) + p = stpcpy(p, separator); + p = stpcpy(p, t); + + need_separator = true; } va_end(ap); + assert(p == r + l); + *p = 0; *x = r; diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 684af0f3e48..09a737ad376 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -179,7 +179,9 @@ char *strreplace(const char *text, const char *old_string, const char *new_strin char *strip_tab_ansi(char **p, size_t *l); -char *strextend(char **x, ...) _sentinel_; +char *strextend_with_separator(char **x, const char *separator, ...) _sentinel_; + +#define strextend(x, ...) strextend_with_separator(x, NULL, __VA_ARGS__) char *strrep(const char *s, unsigned n); diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 7a14b8efd38..07ee98465c6 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -104,9 +104,37 @@ static void test_strstrip(void) { } static void test_strextend(void) { - _cleanup_free_ char *str = strdup("0123"); - strextend(&str, "456", "78", "9", NULL); - assert_se(streq(str, "0123456789")); + _cleanup_free_ char *str = NULL; + + assert_se(strextend(&str, NULL)); + assert_se(streq_ptr(str, "")); + assert_se(strextend(&str, "", "0", "", "", "123", NULL)); + assert_se(streq_ptr(str, "0123")); + assert_se(strextend(&str, "456", "78", "9", NULL)); + assert_se(streq_ptr(str, "0123456789")); +} + +static void test_strextend_with_separator(void) { + _cleanup_free_ char *str = NULL; + + assert_se(strextend_with_separator(&str, NULL, NULL)); + assert_se(streq_ptr(str, "")); + str = mfree(str); + + assert_se(strextend_with_separator(&str, "...", NULL)); + assert_se(streq_ptr(str, "")); + assert_se(strextend_with_separator(&str, "...", NULL)); + assert_se(streq_ptr(str, "")); + str = mfree(str); + + assert_se(strextend_with_separator(&str, "xyz", "a", "bb", "ccc", NULL)); + assert_se(streq_ptr(str, "axyzbbxyzccc")); + str = mfree(str); + + assert_se(strextend_with_separator(&str, ",", "start", "", "1", "234", NULL)); + assert_se(streq_ptr(str, "start,,1,234")); + assert_se(strextend_with_separator(&str, ";", "more", "5", "678", NULL)); + assert_se(streq_ptr(str, "start,,1,234;more;5;678")); } static void test_strrep(void) { @@ -399,6 +427,7 @@ int main(int argc, char *argv[]) { test_streq_ptr(); test_strstrip(); test_strextend(); + test_strextend_with_separator(); test_strrep(); test_strappend(); test_string_has_cc(); From 2d3a5a73e0fd5283bd9de42decfd779e6a85ba53 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Nov 2017 16:46:26 +0100 Subject: [PATCH 05/10] nspawn: make sure images containing an ESP are compatible with userns -U mode In -U mode we might need to re-chown() all files and directories to match the UID shift we want for the image. That's problematic on fat partitions, such as the ESP (and which is generated by mkosi's --bootable switch), because fat of course knows no UID/GID file ownership natively. With this change we take benefit of the uid= and gid= mount options FAT knows: instead of chown()ing all files and directories we can just specify the right UID/GID to use at mount time. This beefs up the image dissection logic in two ways: 1. First of all support for mounting relevant file systems with uid=/gid= is added: when a UID is specified during mount it is used for all applicable file systems. 2. Secondly, two new mount flags are added: DISSECT_IMAGE_MOUNT_ROOT_ONLY and DISSECT_IMAGE_MOUNT_NON_ROOT_ONLY. If one is specified the mount routine will either only mount the root partition of an image, or all partitions except the root partition. This is used by nspawn: first the root partition is mounted, so that we can determine the UID shift in use so far, based on ownership of the image's root directory. Then, we mount the remaining partitions in a second go, this time with the right UID/GID information. --- src/basic/mount-util.c | 16 +++++++++++++ src/basic/mount-util.h | 1 + src/core/namespace.c | 2 +- src/dissect/dissect.c | 3 ++- src/nspawn/nspawn.c | 16 ++++++++++++- src/shared/dissect-image.c | 46 +++++++++++++++++++++++++++----------- src/shared/dissect-image.h | 16 +++++++------ 7 files changed, 77 insertions(+), 23 deletions(-) diff --git a/src/basic/mount-util.c b/src/basic/mount-util.c index a97ee00fa11..7f8b53f2b54 100644 --- a/src/basic/mount-util.c +++ b/src/basic/mount-util.c @@ -673,6 +673,22 @@ bool fstype_can_discard(const char *fstype) { "xfs"); } +bool fstype_can_uid_gid(const char *fstype) { + + /* All file systems that have a uid=/gid= mount option that fixates the owners of all files and directories, + * current and future. */ + + return STR_IN_SET(fstype, + "adfs", + "fat", + "hfs", + "hpfs", + "iso9660", + "msdos", + "ntfs", + "vfat"); +} + int repeat_unmount(const char *path, int flags) { bool done = false; diff --git a/src/basic/mount-util.h b/src/basic/mount-util.h index a19b0d9c421..a81d0192774 100644 --- a/src/basic/mount-util.h +++ b/src/basic/mount-util.h @@ -52,6 +52,7 @@ bool fstype_is_network(const char *fstype); bool fstype_is_api_vfs(const char *fstype); bool fstype_is_ro(const char *fsype); bool fstype_can_discard(const char *fstype); +bool fstype_can_uid_gid(const char *fstype); const char* mode_to_inaccessible_node(mode_t mode); diff --git a/src/core/namespace.c b/src/core/namespace.c index 218f2d8d9ac..a3262fcc4da 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1155,7 +1155,7 @@ int setup_namespace( if (root_image) { /* A root image is specified, mount it to the right place */ - r = dissected_image_mount(dissected_image, root, dissect_image_flags); + r = dissected_image_mount(dissected_image, root, UID_INVALID, dissect_image_flags); if (r < 0) goto finish; diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 0210e463720..c8fb1c80d72 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -29,6 +29,7 @@ #include "loop-util.h" #include "string-util.h" #include "strv.h" +#include "user-util.h" #include "util.h" static enum { @@ -303,7 +304,7 @@ int main(int argc, char *argv[]) { if (r < 0) goto finish; - r = dissected_image_mount(m, arg_path, arg_flags); + r = dissected_image_mount(m, arg_path, UID_INVALID, arg_flags); if (r < 0) { log_error_errno(r, "Failed to mount image: %m"); goto finish; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 127229c06aa..dfe48bcf56a 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2582,7 +2582,13 @@ static int outer_child( return r; if (dissected_image) { - r = dissected_image_mount(dissected_image, directory, DISSECT_IMAGE_DISCARD_ON_LOOP|(arg_read_only ? DISSECT_IMAGE_READ_ONLY : 0)); + /* If we are operating on a disk image, then mount its root directory now, but leave out the rest. We + * can read the UID shift from it if we need to. Further down we'll mount the rest, but then with the + * uid shift known. That way we can mount VFAT file systems shifted to the right place right away. This + * makes sure ESP partitions and userns are compatible. */ + + r = dissected_image_mount(dissected_image, directory, arg_uid_shift, + DISSECT_IMAGE_MOUNT_ROOT_ONLY|DISSECT_IMAGE_DISCARD_ON_LOOP|(arg_read_only ? DISSECT_IMAGE_READ_ONLY : 0)); if (r < 0) return r; } @@ -2618,6 +2624,14 @@ static int outer_child( log_info("Selected user namespace base " UID_FMT " and range " UID_FMT ".", arg_uid_shift, arg_uid_range); } + 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, + DISSECT_IMAGE_MOUNT_NON_ROOT_ONLY|DISSECT_IMAGE_DISCARD_ON_LOOP|(arg_read_only ? DISSECT_IMAGE_READ_ONLY : 0)); + if (r < 0) + return r; + } + if (arg_unified_cgroup_hierarchy == CGROUP_UNIFIED_UNKNOWN) { /* OK, we don't know yet which cgroup mode to use yet. Let's figure it out, and tell the parent. */ diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 5c69b87c00f..8789fd163ab 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -49,6 +49,7 @@ #include "string-util.h" #include "strv.h" #include "udev-util.h" +#include "user-util.h" #include "xattr-util.h" int probe_filesystem(const char *node, char **ret_fstype) { @@ -686,10 +687,11 @@ static int mount_partition( DissectedPartition *m, const char *where, const char *directory, + uid_t uid_shift, DissectImageFlags flags) { - const char *p, *options = NULL, *node, *fstype; - _cleanup_free_ char *chased = NULL; + _cleanup_free_ char *chased = NULL, *options = NULL; + const char *p, *node, *fstype; bool rw; int r; @@ -720,13 +722,26 @@ static int mount_partition( /* If requested, turn on discard support. */ if (fstype_can_discard(fstype) && ((flags & DISSECT_IMAGE_DISCARD) || - ((flags & DISSECT_IMAGE_DISCARD_ON_LOOP) && is_loop_device(m->node)))) - options = "discard"; + ((flags & DISSECT_IMAGE_DISCARD_ON_LOOP) && is_loop_device(m->node)))) { + options = strdup("discard"); + if (!options) + return -ENOMEM; + } + + if (uid_is_valid(uid_shift) && uid_shift != 0 && fstype_can_uid_gid(fstype)) { + _cleanup_free_ char *uid_option = NULL; + + if (asprintf(&uid_option, "uid=" UID_FMT ",gid=" GID_FMT, uid_shift, (gid_t) uid_shift) < 0) + return -ENOMEM; + + if (!strextend_with_separator(&options, ",", uid_option, NULL)) + return -ENOMEM; + } return mount_verbose(LOG_DEBUG, node, p, fstype, MS_NODEV|(rw ? 0 : MS_RDONLY), options); } -int dissected_image_mount(DissectedImage *m, const char *where, DissectImageFlags flags) { +int dissected_image_mount(DissectedImage *m, const char *where, uid_t uid_shift, DissectImageFlags flags) { int r; assert(m); @@ -735,15 +750,20 @@ int dissected_image_mount(DissectedImage *m, const char *where, DissectImageFlag if (!m->partitions[PARTITION_ROOT].found) return -ENXIO; - r = mount_partition(m->partitions + PARTITION_ROOT, where, NULL, flags); + if ((flags & DISSECT_IMAGE_MOUNT_NON_ROOT_ONLY) == 0) { + r = mount_partition(m->partitions + PARTITION_ROOT, where, NULL, uid_shift, flags); + if (r < 0) + return r; + } + + if ((flags & DISSECT_IMAGE_MOUNT_ROOT_ONLY)) + return 0; + + r = mount_partition(m->partitions + PARTITION_HOME, where, "/home", uid_shift, flags); if (r < 0) return r; - r = mount_partition(m->partitions + PARTITION_HOME, where, "/home", flags); - if (r < 0) - return r; - - r = mount_partition(m->partitions + PARTITION_SRV, where, "/srv", flags); + r = mount_partition(m->partitions + PARTITION_SRV, where, "/srv", uid_shift, flags); if (r < 0) return r; @@ -761,7 +781,7 @@ int dissected_image_mount(DissectedImage *m, const char *where, DissectImageFlag r = dir_is_empty(p); if (r > 0) { - r = mount_partition(m->partitions + PARTITION_ESP, where, mp, flags); + r = mount_partition(m->partitions + PARTITION_ESP, where, mp, uid_shift, flags); if (r < 0) return r; } @@ -1254,7 +1274,7 @@ int dissected_image_acquire_metadata(DissectedImage *m) { if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) < 0) _exit(EXIT_FAILURE); - r = dissected_image_mount(m, t, DISSECT_IMAGE_READ_ONLY); + r = dissected_image_mount(m, t, UID_INVALID, DISSECT_IMAGE_READ_ONLY); if (r < 0) _exit(EXIT_FAILURE); diff --git a/src/shared/dissect-image.h b/src/shared/dissect-image.h index 7c7ce46015b..53a1554a287 100644 --- a/src/shared/dissect-image.h +++ b/src/shared/dissect-image.h @@ -62,15 +62,17 @@ static inline int PARTITION_VERITY_OF(int p) { } typedef enum DissectImageFlags { - DISSECT_IMAGE_READ_ONLY = 1, - DISSECT_IMAGE_DISCARD_ON_LOOP = 2, /* Turn on "discard" if on a loop device and file system supports it */ - DISSECT_IMAGE_DISCARD = 4, /* Turn on "discard" if file system supports it, on all block devices */ - DISSECT_IMAGE_DISCARD_ON_CRYPTO = 8, /* Turn on "discard" also on crypto devices */ + DISSECT_IMAGE_READ_ONLY = 1 << 0, + DISSECT_IMAGE_DISCARD_ON_LOOP = 1 << 1, /* Turn on "discard" if on a loop device and file system supports it */ + DISSECT_IMAGE_DISCARD = 1 << 2, /* Turn on "discard" if file system supports it, on all block devices */ + DISSECT_IMAGE_DISCARD_ON_CRYPTO = 1 << 3, /* Turn on "discard" also on crypto devices */ DISSECT_IMAGE_DISCARD_ANY = DISSECT_IMAGE_DISCARD_ON_LOOP | DISSECT_IMAGE_DISCARD | DISSECT_IMAGE_DISCARD_ON_CRYPTO, - DISSECT_IMAGE_GPT_ONLY = 16, /* Only recognize images with GPT partition tables */ - DISSECT_IMAGE_REQUIRE_ROOT = 32, /* Don't accept disks without root partition */ + DISSECT_IMAGE_GPT_ONLY = 1 << 4, /* Only recognize images with GPT partition tables */ + DISSECT_IMAGE_REQUIRE_ROOT = 1 << 5, /* Don't accept disks without root partition */ + DISSECT_IMAGE_MOUNT_ROOT_ONLY = 1 << 6, /* Mount only the root partition */ + DISSECT_IMAGE_MOUNT_NON_ROOT_ONLY = 1 << 7, /* Mount only non-root partitions */ } DissectImageFlags; struct DissectedImage { @@ -94,7 +96,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(DissectedImage*, dissected_image_unref); int dissected_image_decrypt(DissectedImage *m, const char *passphrase, const void *root_hash, size_t root_hash_size, DissectImageFlags flags, DecryptedImage **ret); int dissected_image_decrypt_interactively(DissectedImage *m, const char *passphrase, const void *root_hash, size_t root_hash_size, DissectImageFlags flags, DecryptedImage **ret); -int dissected_image_mount(DissectedImage *m, const char *dest, DissectImageFlags flags); +int dissected_image_mount(DissectedImage *m, const char *dest, uid_t uid_shift, DissectImageFlags flags); int dissected_image_acquire_metadata(DissectedImage *m); From de54e02d5e2d4735edface6505b49b99b6bacc85 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Nov 2017 17:58:00 +0100 Subject: [PATCH 06/10] nspawn: when in hybrid mode, chown() both the legacy and the unified hierarchy to the root in the container If user namespacing is used, let's make sure that the root user in the container gets access to both /sys/fs/cgroup/systemd and /sys/fs/cgroup/unified. This matches similar logic in cg_set_access(). --- src/nspawn/nspawn-cgroup.c | 15 ++++++++++++++- src/nspawn/nspawn-cgroup.h | 2 +- src/nspawn/nspawn.c | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/nspawn/nspawn-cgroup.c b/src/nspawn/nspawn-cgroup.c index 4b07b38163d..d51585a6524 100644 --- a/src/nspawn/nspawn-cgroup.c +++ b/src/nspawn/nspawn-cgroup.c @@ -57,7 +57,7 @@ static int chown_cgroup_path(const char *path, uid_t uid_shift) { return 0; } -int chown_cgroup(pid_t pid, uid_t uid_shift) { +int chown_cgroup(pid_t pid, CGroupUnified unified_requested, uid_t uid_shift) { _cleanup_free_ char *path = NULL, *fs = NULL; int r; @@ -73,6 +73,19 @@ int chown_cgroup(pid_t pid, uid_t uid_shift) { if (r < 0) return log_error_errno(r, "Failed to chown() cgroup %s: %m", fs); + if (unified_requested == CGROUP_UNIFIED_SYSTEMD) { + _cleanup_free_ char *lfs = NULL; + /* Always propagate access rights from unified to legacy controller */ + + r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, NULL, &lfs); + if (r < 0) + return log_error_errno(r, "Failed to get file system path for container cgroup: %m"); + + r = chown_cgroup_path(lfs, uid_shift); + if (r < 0) + return log_error_errno(r, "Failed to chown() cgroup %s: %m", lfs); + } + return 0; } diff --git a/src/nspawn/nspawn-cgroup.h b/src/nspawn/nspawn-cgroup.h index e66a8548431..3855e5b4eaa 100644 --- a/src/nspawn/nspawn-cgroup.h +++ b/src/nspawn/nspawn-cgroup.h @@ -25,6 +25,6 @@ #include "cgroup-util.h" -int chown_cgroup(pid_t pid, uid_t uid_shift); +int chown_cgroup(pid_t pid, CGroupUnified unified_requested, uid_t uid_shift); int sync_cgroup(pid_t pid, CGroupUnified unified_requested, uid_t uid_shift); int create_subcgroup(pid_t pid, CGroupUnified unified_requested); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index dfe48bcf56a..ccf5418ceb6 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3619,7 +3619,7 @@ static int run(int master, return r; } - r = chown_cgroup(*pid, arg_uid_shift); + r = chown_cgroup(*pid, arg_unified_cgroup_hierarchy, arg_uid_shift); if (r < 0) return r; From f2d566b7280ea09a624671c2c4aad432a9976244 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Nov 2017 19:36:52 +0100 Subject: [PATCH 07/10] tests: make TEST-10-ISSUE-2467 also run in nspawn No reason not to run it in nspawn, and it's easier to debug than the qemu version. --- test/TEST-10-ISSUE-2467/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/TEST-10-ISSUE-2467/test.sh b/test/TEST-10-ISSUE-2467/test.sh index e7eb1cb303c..e027c9d81d2 100755 --- a/test/TEST-10-ISSUE-2467/test.sh +++ b/test/TEST-10-ISSUE-2467/test.sh @@ -3,7 +3,6 @@ # ex: ts=8 sw=4 sts=4 et filetype=sh set -e TEST_DESCRIPTION="https://github.com/systemd/systemd/issues/2467" -TEST_NO_NSPAWN=1 . $TEST_BASE_DIR/test-functions SKIP_INITRD=yes @@ -49,6 +48,7 @@ EOF setup_testsuite ) || return 1 + setup_nspawn_root # mask some services that we do not want to run in these tests ln -s /dev/null $initdir/etc/systemd/system/systemd-hwdb-update.service From 9b45c2bf02a43e3e1b42de1ab0c3fe29c64dc5f5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Nov 2017 19:37:32 +0100 Subject: [PATCH 08/10] tests: don't use "netcat" for testing TEST-10-ISSUE-2467 Apparently there are a myriad of netcat implementations around, and they all behave slightly differently. The one I have on my Fedora 27 installation will cause a failure when invoked as "nc -U" on an AF_UNIX socket whose connections are immediately disconnected, thus causing the test to fail. Let's avoid all ambiguities in this regard, and drop usage of netcat altoegther. Instead let's use a FIFO in the file system, which we can connect to with only shell commands, and is hence much simpler and more reliable to test with. The actual test is supposed to validate that PID 1 doesn't hang when activation of a socket-activated service fails, hence which transport mechanism is used ultimately doesn't matter, as long as we activate the service, and we do here... --- test/TEST-10-ISSUE-2467/test.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/TEST-10-ISSUE-2467/test.sh b/test/TEST-10-ISSUE-2467/test.sh index e027c9d81d2..2f95e9062da 100755 --- a/test/TEST-10-ISSUE-2467/test.sh +++ b/test/TEST-10-ISSUE-2467/test.sh @@ -18,7 +18,7 @@ test_setup() { eval $(udevadm info --export --query=env --name=${LOOPDEV}p2) setup_basic_environment - dracut_install nc true rm + dracut_install true rm # setup the testsuite service cat >$initdir/etc/systemd/system/testsuite.service <<'EOF' @@ -28,13 +28,15 @@ After=multi-user.target [Service] Type=oneshot -ExecStart=/bin/sh -e -x -c 'rm -f /tmp/nonexistent; systemctl start test.socket; echo a | nc -U /run/test.ctl; >/testok' +StandardOutput=tty +StandardError=tty +ExecStart=/bin/sh -e -x -c 'rm -f /tmp/nonexistent; systemctl start test.socket; echo > /run/test.ctl; >/testok' TimeoutStartSec=10s EOF cat >$initdir/etc/systemd/system/test.socket <<'EOF' [Socket] -ListenStream=/run/test.ctl +ListenFIFO=/run/test.ctl EOF cat > $initdir/etc/systemd/system/test.service <<'EOF' From 8fe9c8d9200f1b5e1bdd5f7bdf38d6a37b7412e7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Nov 2017 19:42:15 +0100 Subject: [PATCH 09/10] test: add a simple script that runs all our integration tests one after the other --- test/run-integration-tests.sh | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100755 test/run-integration-tests.sh diff --git a/test/run-integration-tests.sh b/test/run-integration-tests.sh new file mode 100755 index 00000000000..3ece46c7717 --- /dev/null +++ b/test/run-integration-tests.sh @@ -0,0 +1,45 @@ +#!/bin/bash -e + +if ! test -d ../build ; then + echo "Expected build directory in ../build, but couldn't find it." >&2 + exit 1 +fi + +ninja -C ../build + +declare -A results + +RESULT=0 +FAILURES=0 + +for TEST in TEST-??-* ; do + echo -e "\n--x-- Starting $TEST --x--" + set +e + make -C "$TEST" BUILD_DIR=$(pwd)/../build clean setup run + RESULT=$? + set -e + echo "--x-- Result of $TEST: $RESULT --x--" + + results["$TEST"]="$RESULT" + + [ "$RESULT" -ne "0" ] && FAILURES=$(($FAILURES+1)) +done + +echo "" + +for TEST in ${!results[@]}; do + RESULT="${results[$TEST]}" + if [ "$RESULT" -eq "0" ] ; then + echo "$TEST: SUCCESS" + else + echo "$TEST: FAIL" + fi +done | sort + +if [ "$FAILURES" -eq 0 ] ; then + echo -e "\nALL PASSED" +else + echo -e "\nTOTAL FAILURES: $FAILURES" +fi + +exit "$FAILURES" From 6c782b7a47ad9a8dcabd7218a43626293694c27b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Nov 2017 20:11:52 +0100 Subject: [PATCH 10/10] fstab-generator: port some code to strextend_with_separator() --- src/fstab-generator/fstab-generator.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index 68e7c23cbea..c0886547e08 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -820,19 +820,13 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat return log_oom(); } else if (streq(key, "rootflags")) { - char *o; if (proc_cmdline_value_missing(key, value)) return 0; - o = arg_root_options ? - strjoin(arg_root_options, ",", value) : - strdup(value); - if (!o) + if (!strextend_with_separator(&arg_root_options, ",", value, NULL)) return log_oom(); - free(arg_root_options); - arg_root_options = o; } else if (streq(key, "roothash")) { if (proc_cmdline_value_missing(key, value)) @@ -858,20 +852,13 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat return log_oom(); } else if (streq(key, "mount.usrflags")) { - char *o; if (proc_cmdline_value_missing(key, value)) return 0; - o = arg_usr_options ? - strjoin(arg_usr_options, ",", value) : - strdup(value); - if (!o) + if (!strextend_with_separator(&arg_usr_options, ",", value, NULL)) return log_oom(); - free(arg_usr_options); - arg_usr_options = o; - } else if (streq(key, "rw") && !value) arg_root_rw = true; else if (streq(key, "ro") && !value)