From 49eb36596b72e568d4b9b785cd4544e58c89bdee Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Apr 2018 17:03:45 +0200 Subject: [PATCH 1/6] fs-util: add new CHASE_STEP flag to chase_symlinks() If the flag is set only a single step of the normalization is executed, and the resulting path is returned. This allows callers to normalize piecemeal, taking into account every single intermediary path of the normalization. --- src/basic/fs-util.c | 63 +++++++++++++++++++++++++++++++++++++++-- src/basic/fs-util.h | 1 + src/test/test-fs-util.c | 45 ++++++++++++++++++++++++++++- 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index bfdd9282f2..c0ba1f18d1 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -594,6 +594,9 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, if ((flags & (CHASE_NONEXISTENT|CHASE_OPEN)) == (CHASE_NONEXISTENT|CHASE_OPEN)) return -EINVAL; + if ((flags & (CHASE_STEP|CHASE_OPEN)) == (CHASE_STEP|CHASE_OPEN)) + return -EINVAL; + if (isempty(path)) return -EINVAL; @@ -615,13 +618,34 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, * Suggested usage: whenever you want to canonicalize a path, use this function. Pass the absolute path you got * as-is: fully qualified and relative to your host's root. Optionally, specify the root parameter to tell this * function what to do when encountering a symlink with an absolute path as directory: prefix it by the - * specified path. */ + * specified path. + * + * There are three ways to invoke this function: + * + * 1. Without CHASE_STEP or CHASE_OPEN: in this case the path is resolved and the normalized path is returned + * in `ret`. The return value is < 0 on error. If CHASE_NONEXISTENT is also set 0 is returned if the file + * doesn't exist, > 0 otherwise. If CHASE_NONEXISTENT is not set >= 0 is returned if the destination was + * found, -ENOENT if it doesn't. + * + * 2. With CHASE_OPEN: in this case the destination is opened after chasing it as O_PATH and this file + * descriptor is returned as return value. This is useful to open files relative to some root + * directory. Note that the returned O_PATH file descriptors must be converted into a regular one (using + * fd_reopen() or such) before it can be used for reading/writing. CHASE_OPEN may not be combined with + * CHASE_NONEXISTENT. + * + * 3. With CHASE_STEP: in this case only a single step of the normalization is executed, i.e. only the first + * symlink or ".." component of the path is resolved, and the resulting path is returned. This is useful if + * a caller wants to trace the a path through the file system verbosely. Returns < 0 on error, > 0 if the + * path is fully normalized, and == 0 for each normalization step. This may be combined with + * CHASE_NONEXISTENT, in which case 1 is returned when a component is not found. + * + * */ /* A root directory of "/" or "" is identical to none */ if (noop_root(original_root)) original_root = NULL; - if (!original_root && !ret && (flags & (CHASE_NONEXISTENT|CHASE_NO_AUTOFS|CHASE_SAFE|CHASE_OPEN)) == CHASE_OPEN) { + if (!original_root && !ret && (flags & (CHASE_NONEXISTENT|CHASE_NO_AUTOFS|CHASE_SAFE|CHASE_OPEN|CHASE_STEP)) == CHASE_OPEN) { /* Shortcut the CHASE_OPEN case if the caller isn't interested in the actual path and has no root set * and doesn't care about any of the other special features we provide either. */ r = open(path, O_PATH|O_CLOEXEC); @@ -718,6 +742,9 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, free_and_replace(done, parent); + if (flags & CHASE_STEP) + goto chased_one; + fd_parent = openat(fd, "..", O_CLOEXEC|O_NOFOLLOW|O_PATH); if (fd_parent < 0) return -errno; @@ -834,6 +861,9 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, free(buffer); todo = buffer = joined; + if (flags & CHASE_STEP) + goto chased_one; + continue; } @@ -872,7 +902,36 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, return TAKE_FD(fd); } + if (flags & CHASE_STEP) + return 1; + return exists; + +chased_one: + + if (ret) { + char *c; + + if (done) { + if (todo) { + c = strjoin(done, todo); + if (!c) + return -ENOMEM; + } else + c = TAKE_PTR(done); + } else { + if (todo) + c = strdup(todo); + else + c = strdup("/"); + if (!c) + return -ENOMEM; + } + + *ret = c; + } + + return 0; } int chase_symlinks_and_open( diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index d50ac7f972..16f20cecdf 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -75,6 +75,7 @@ enum { CHASE_SAFE = 1U << 3, /* If set, return EPERM if we ever traverse from unprivileged to privileged files or directories */ CHASE_OPEN = 1U << 4, /* If set, return an O_PATH object to the final component */ CHASE_TRAIL_SLASH = 1U << 5, /* If set, any trailing slash will be preserved */ + CHASE_STEP = 1U << 6, /* If set, just execute a single step of the normalization */ }; int chase_symlinks(const char *path_with_prefix, const char *root, unsigned flags, char **ret); diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 284167cff2..9e9650fced 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -24,7 +24,7 @@ #include "util.h" static void test_chase_symlinks(void) { - _cleanup_free_ char *result = NULL; + _cleanup_free_ char *result = NULL, *z = NULL, *w = NULL; char temp[] = "/tmp/test-chase.XXXXXX"; const char *top, *p, *pslash, *q, *qslash; int r, pfd; @@ -271,6 +271,49 @@ static void test_chase_symlinks(void) { assert_se(sd_id128_equal(a, b)); } + /* Test CHASE_ONE */ + + p = strjoina(temp, "/start"); + r = chase_symlinks(p, NULL, CHASE_STEP, &result); + assert_se(r == 0); + p = strjoina(temp, "/top/dot/dotdota"); + assert_se(streq(p, result)); + result = mfree(result); + + r = chase_symlinks(p, NULL, CHASE_STEP, &result); + assert_se(r == 0); + p = strjoina(temp, "/top/./dotdota"); + assert_se(streq(p, result)); + result = mfree(result); + + r = chase_symlinks(p, NULL, CHASE_STEP, &result); + assert_se(r == 0); + p = strjoina(temp, "/top/../a"); + assert_se(streq(p, result)); + result = mfree(result); + + r = chase_symlinks(p, NULL, CHASE_STEP, &result); + assert_se(r == 0); + p = strjoina(temp, "/a"); + assert_se(streq(p, result)); + result = mfree(result); + + r = chase_symlinks(p, NULL, CHASE_STEP, &result); + assert_se(r == 0); + p = strjoina(temp, "/b"); + assert_se(streq(p, result)); + result = mfree(result); + + r = chase_symlinks(p, NULL, CHASE_STEP, &result); + assert_se(r == 0); + assert_se(streq("/usr", result)); + result = mfree(result); + + r = chase_symlinks("/usr", NULL, CHASE_STEP, &result); + assert_se(r > 0); + assert_se(streq("/usr", result)); + result = mfree(result); + assert_se(rm_rf(temp, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0); } From f10f4215b5a92bf3c1c84deccad51fdff65dfa9e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Apr 2018 17:06:12 +0200 Subject: [PATCH 2/6] fs-util: export how many iterations chase_symlinks() executes at max --- src/basic/fs-util.c | 2 +- src/basic/fs-util.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index c0ba1f18d1..3dc194ca6e 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -582,7 +582,7 @@ static bool safe_transition(const struct stat *a, const struct stat *b) { int chase_symlinks(const char *path, const char *original_root, unsigned flags, char **ret) { _cleanup_free_ char *buffer = NULL, *done = NULL, *root = NULL; _cleanup_close_ int fd = -1; - unsigned max_follow = 32; /* how many symlinks to follow before giving up and returning ELOOP */ + unsigned max_follow = CHASE_SYMLINKS_MAX; /* how many symlinks to follow before giving up and returning ELOOP */ struct stat previous_stat; bool exists = true; char *todo; diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 16f20cecdf..e8362b92a5 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -78,6 +78,9 @@ enum { CHASE_STEP = 1U << 6, /* If set, just execute a single step of the normalization */ }; +/* How many iterations to execute before returning -ELOOP */ +#define CHASE_SYMLINKS_MAX 32 + int chase_symlinks(const char *path_with_prefix, const char *root, unsigned flags, char **ret); int chase_symlinks_and_open(const char *path, const char *root, unsigned chase_flags, int open_flags, char **ret_path); From c9ef8573be8e170fc0166d58406c4f9805fa323e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Apr 2018 10:14:25 +0200 Subject: [PATCH 3/6] namespace: don't consider raw image read-only if /home in it is writable --- src/core/namespace.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index e138d3ba02..0cce2b4584 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1105,7 +1105,9 @@ int setup_namespace( if (root_image) { dissect_image_flags |= DISSECT_IMAGE_REQUIRE_ROOT; - if (protect_system == PROTECT_SYSTEM_STRICT && strv_isempty(read_write_paths)) + if (protect_system == PROTECT_SYSTEM_STRICT && + protect_home != PROTECT_HOME_NO && + strv_isempty(read_write_paths)) dissect_image_flags |= DISSECT_IMAGE_READ_ONLY; r = loop_device_make_by_path(root_image, From f8b64b5723528f68a01a3682c74033901917275e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Apr 2018 11:03:21 +0200 Subject: [PATCH 4/6] namespace: split out calls to normalize mount entry list into new function --- src/core/namespace.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 0cce2b4584..ec6b0a5ae7 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1066,6 +1066,18 @@ static unsigned namespace_calculate_mounts( (namespace_info_mount_apivfs(root_directory, ns_info) ? ELEMENTSOF(apivfs_table) : 0); } +static void normalize_mounts(const char *root_directory, MountEntry *mounts, unsigned *n_mounts) { + assert(n_mounts); + assert(mounts || *n_mounts == 0); + + qsort_safe(mounts, *n_mounts, sizeof(MountEntry), mount_path_compare); + + drop_duplicates(mounts, n_mounts); + drop_outside_root(root_directory, mounts, n_mounts); + drop_inaccessible(mounts, n_mounts); + drop_nop(mounts, n_mounts); +} + int setup_namespace( const char* root_directory, const char* root_image, @@ -1250,12 +1262,7 @@ int setup_namespace( if (r < 0) goto finish; - qsort(mounts, n_mounts, sizeof(MountEntry), mount_path_compare); - - drop_duplicates(mounts, &n_mounts); - drop_outside_root(root, mounts, &n_mounts); - drop_inaccessible(mounts, &n_mounts); - drop_nop(mounts, &n_mounts); + normalize_mounts(root_directory, mounts, &n_mounts); } if (unshare(CLONE_NEWNS) < 0) { From e8717862737bbf8f5388cdfe068265e0aec0fe68 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Apr 2018 11:12:49 +0200 Subject: [PATCH 5/6] namespace: improve logging when creating mount source nodes --- src/core/namespace.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index ec6b0a5ae7..32d8ca63ef 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -948,14 +948,22 @@ static int apply_mount( /* Hmm, either the source or the destination are missing. Let's see if we can create the destination, then try again */ - if (stat(what, &st) >= 0) { + if (stat(what, &st) < 0) + log_debug_errno(errno, "Mount point source '%s' is not accessible: %m", what); + else { + int q; (void) mkdir_parents(mount_entry_path(m), 0755); if (S_ISDIR(st.st_mode)) - try_again = mkdir(mount_entry_path(m), 0755) >= 0; + q = mkdir(mount_entry_path(m), 0755) < 0 ? -errno : 0; else - try_again = touch(mount_entry_path(m)) >= 0; + q = touch(mount_entry_path(m)); + + if (q < 0) + log_debug_errno(q, "Failed to create destination mount point node '%s': %m", mount_entry_path(m)); + else + try_again = true; } } From 088696fe29e1eadeced40a5a2b277570f0955164 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Apr 2018 10:14:43 +0200 Subject: [PATCH 6/6] namespace: rework how we resolve symlinks in mount points Before this patch we'd resolve all symlinks of bind mounts and other mount points to establish for a service in advance, and only then start mounting them. This is problematic, if symlink chains jump around between directories in a namespace tree, so that to resolve a specific symlink chain we need to establish another mount already. A typical case where this happens is if /etc/resolv.conf is a symlink to some file in /run: in that case we'd normally resolve and mount /etc/resolv.conf early on, but that's broken, as to do this properly we'd need to resolve /etc/resolv.conf first, then figure out that /run needs to be mounted before we can proceed, and thus reorder the order in which we apply mounts dynamically. With this change, whenever we are about to apply a mount, we'll do a single step of the symlink normalization process, patch the mount entry accordingly, and then sort the list of mounts to establish again, taking the new path into account. This means that we can correctly deal with the example above: we might start with wanting to mount /etc/resolv.conf early, but after resolving it to the path in /run/ we'd push it to the end of the list, ensuring that /run is mounted first. (Note that this also fixes another bug: we were following symlinks on the bind mount source relative to the root directory of the service, rather than of the host. That's wrong though as we explicitly document tha the source of bind mounts is always on the host.) --- src/core/namespace.c | 124 +++++++++++++++++++++++++++++-------------- 1 file changed, 85 insertions(+), 39 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 32d8ca63ef..cb34a79859 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -61,12 +61,14 @@ typedef struct MountEntry { bool ignore:1; /* Ignore if path does not exist? */ bool has_prefix:1; /* Already is prefixed by the root dir? */ bool read_only:1; /* Shall this mount point be read-only? */ + bool applied:1; /* Already applied */ char *path_malloc; /* Use this instead of 'path_const' if we had to allocate memory */ const char *source_const; /* The source path, for bind mounts */ char *source_malloc; const char *options_const;/* Mount options for tmpfs */ char *options_malloc; unsigned long flags; /* Mount flags used by EMPTY_DIR and TMPFS. Do not include MS_RDONLY here, but please use read_only. */ + unsigned n_followed; } MountEntry; /* If MountAPIVFS= is used, let's mount /sys and /proc into the it, but only as a fallback if the user hasn't mounted @@ -410,7 +412,6 @@ static int mount_path_compare(const void *a, const void *b) { /* If the paths are equal, check the mode */ if (p->mode < q->mode) return -1; - if (p->mode > q->mode) return 1; @@ -454,8 +455,10 @@ static void drop_duplicates(MountEntry *m, unsigned *n) { for (f = m, t = m, previous = NULL; f < m + *n; f++) { /* The first one wins (which is the one with the more restrictive mode), see mount_path_compare() - * above. */ - if (previous && path_equal(mount_entry_path(f), mount_entry_path(previous))) { + * above. Note that we only drop duplicates that haven't been mounted yet. */ + if (previous && + path_equal(mount_entry_path(f), mount_entry_path(previous)) && + !f->applied && !previous->applied) { log_debug("%s is duplicate.", mount_entry_path(f)); previous->read_only = previous->read_only || mount_entry_read_only(f); /* Propagate the read-only flag to the remaining entry */ mount_entry_done(f); @@ -817,36 +820,37 @@ static int mount_tmpfs(const MountEntry *m) { return 1; } -static int mount_entry_chase( +static int follow_symlink( const char *root_directory, - const MountEntry *m, - const char *path, - bool chase_nonexistent, - char **location) { + MountEntry *m) { - char *chased; + _cleanup_free_ char *target = NULL; int r; - assert(m); + /* Let's chase symlinks, but only one step at a time. That's because depending where the symlink points we + * might need to change the order in which we mount stuff. Hence: let's normalize piecemeal, and do one step at + * a time by specifying CHASE_STEP. This function returns 0 if we resolved one step, and > 0 if we reached the + * end and already have a fully normalized name. */ - /* Since mount() will always follow symlinks and we need to take the different root directory into account we - * chase the symlinks on our own first. This is called for the destination path, as well as the source path (if - * that applies). The result is stored in "location". */ - - r = chase_symlinks(path, root_directory, CHASE_TRAIL_SLASH | (chase_nonexistent ? CHASE_NONEXISTENT : 0), &chased); - if (r == -ENOENT && m->ignore) { - log_debug_errno(r, "Path %s does not exist, ignoring.", path); - return 0; - } + r = chase_symlinks(mount_entry_path(m), root_directory, CHASE_STEP|CHASE_NONEXISTENT, &target); if (r < 0) - return log_debug_errno(r, "Failed to follow symlinks on %s: %m", path); + return log_debug_errno(r, "Failed to chase symlinks '%s': %m", mount_entry_path(m)); + if (r > 0) /* Reached the end, nothing more to resolve */ + return 1; - log_debug("Followed symlinks %s → %s.", path, chased); + if (m->n_followed >= CHASE_SYMLINKS_MAX) { /* put a boundary on things */ + log_debug("Symlink loop on '%s'.", mount_entry_path(m)); + return -ELOOP; + } - free(*location); - *location = chased; + log_debug("Followed mount entry path symlink %s → %s.", mount_entry_path(m), target); - return 1; + free_and_replace(m->path_malloc, target); + m->has_prefix = true; + + m->n_followed ++; + + return 0; } static int apply_mount( @@ -859,10 +863,6 @@ static int apply_mount( assert(m); - r = mount_entry_chase(root_directory, m, mount_entry_path(m), !IN_SET(m->mode, INACCESSIBLE, READONLY, READWRITE), &m->path_malloc); - if (r <= 0) - return r; - log_debug("Applying namespace mount on %s", mount_entry_path(m)); switch (m->mode) { @@ -875,8 +875,12 @@ static int apply_mount( * inaccessible path. */ (void) umount_recursive(mount_entry_path(m), 0); - if (lstat(mount_entry_path(m), &target) < 0) + if (lstat(mount_entry_path(m), &target) < 0) { + if (errno == ENOENT && m->ignore) + return 0; + return log_debug_errno(errno, "Failed to lstat() %s to determine what to mount over it: %m", mount_entry_path(m)); + } what = mode_to_inaccessible_node(target.st_mode); if (!what) { @@ -889,6 +893,8 @@ static int apply_mount( case READONLY: case READWRITE: r = path_is_mount_point(mount_entry_path(m), root_directory, 0); + if (r == -ENOENT && m->ignore) + return 0; if (r < 0) return log_debug_errno(r, "Failed to determine whether %s is already a mount point: %m", mount_entry_path(m)); if (r > 0) /* Nothing to do here, it is already a mount. We just later toggle the MS_RDONLY bit for the mount point if needed. */ @@ -901,16 +907,29 @@ static int apply_mount( rbind = false; _fallthrough_; - case BIND_MOUNT_RECURSIVE: - /* Also chase the source mount */ + case BIND_MOUNT_RECURSIVE: { + _cleanup_free_ char *chased = NULL; - r = mount_entry_chase(root_directory, m, mount_entry_source(m), false, &m->source_malloc); - if (r <= 0) - return r; + /* Since mount() will always follow symlinks we chase the symlinks on our own first. Note that bind + * mount source paths are always relative to the host root, hence we pass NULL as root directory to + * chase_symlinks() here. */ + + r = chase_symlinks(mount_entry_source(m), NULL, CHASE_TRAIL_SLASH, &chased); + if (r == -ENOENT && m->ignore) { + log_debug_errno(r, "Path %s does not exist, ignoring.", mount_entry_source(m)); + return 0; + } + if (r < 0) + return log_debug_errno(r, "Failed to follow symlinks on %s: %m", mount_entry_source(m)); + + log_debug("Followed source symlinks %s → %s.", mount_entry_source(m), chased); + + free_and_replace(m->source_malloc, chased); what = mount_entry_source(m); make = true; break; + } case EMPTY_DIR: case TMPFS: @@ -1340,11 +1359,38 @@ int setup_namespace( goto finish; } - /* First round, add in all special mounts we need */ - for (m = mounts; m < mounts + n_mounts; ++m) { - r = apply_mount(root, m); - if (r < 0) - goto finish; + /* First round, establish all mounts we need */ + for (;;) { + bool again = false; + + for (m = mounts; m < mounts + n_mounts; ++m) { + + if (m->applied) + continue; + + r = follow_symlink(root, m); + if (r < 0) + goto finish; + if (r == 0) { + /* We hit a symlinked mount point. The entry got rewritten and might point to a + * very different place now. Let's normalize the changed list, and start from + * the beginning. After all to mount the entry at the new location we might + * need some other mounts first */ + again = true; + break; + } + + r = apply_mount(root, m); + if (r < 0) + goto finish; + + m->applied = true; + } + + if (!again) + break; + + normalize_mounts(root_directory, mounts, &n_mounts); } /* Create a blacklist we can pass to bind_mount_recursive() */