From dba4fa891040e1280569b4b9b8fc5b61ddbaabcf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 31 Oct 2023 16:10:32 +0100 Subject: [PATCH] nspawn: make sure idmapped logic works if DDI contains only /usr/ tree If we have a DDI that contains only a /usr/ tree (and which is thus combined with a tmpfs for root on boot) we previously would try to apply idmapping to the tmpfs, but not the /usr/ mount. That's broken of course. Fix this by applying it to both trees. --- src/nspawn/nspawn-mount.c | 2 +- src/nspawn/nspawn.c | 17 ++++++++- src/shared/dissect-image.c | 2 +- src/shared/mount-util.c | 71 ++++++++++++++++++++++++++------------ src/shared/mount-util.h | 4 +-- 5 files changed, 69 insertions(+), 27 deletions(-) diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 76556426621..470f477f22c 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -828,7 +828,7 @@ static int mount_bind(const char *dest, CustomMount *m, uid_t uid_shift, uid_t u } if (idmapping != REMOUNT_IDMAPPING_NONE) { - r = remount_idmap(where, uid_shift, uid_range, source_st.st_uid, idmapping); + r = remount_idmap(STRV_MAKE(where), uid_shift, uid_range, source_st.st_uid, idmapping); if (r < 0) return log_error_errno(r, "Failed to map ids for bind mount %s: %m", where); } diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 37a75134f92..b704db6fca9 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3828,8 +3828,23 @@ 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; - r = remount_idmap(directory, arg_uid_shift, arg_uid_range, UID_INVALID, REMOUNT_IDMAPPING_HOST_ROOT); + dirs[i++] = (char*) directory; + + if (dissected_image && dissected_image->partitions[PARTITION_USR].found) { + usr_subtree = path_join(directory, "/usr"); + if (!usr_subtree) + return log_oom(); + + dirs[i++] = usr_subtree; + } + + dirs[i] = NULL; + + r = remount_idmap(dirs, arg_uid_shift, arg_uid_range, 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 * can't really distinguish this nicely, nor do we have any guarantees about the diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 58a87431a9b..ef23c4a255a 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -2026,7 +2026,7 @@ static int mount_partition( (void) fs_grow(node, -EBADF, p); if (userns_fd >= 0) { - r = remount_idmap_fd(p, userns_fd); + r = remount_idmap_fd(STRV_MAKE(p), userns_fd); if (r < 0) return r; } diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index b4bee7af857..a7493a38147 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -1359,41 +1359,68 @@ int make_userns(uid_t uid_shift, uid_t uid_range, uid_t owner, RemountIdmapping } int remount_idmap_fd( - const char *p, + char **paths, int userns_fd) { - _cleanup_close_ int mount_fd = -EBADF; int r; - assert(p); assert(userns_fd >= 0); - /* Clone the mount point */ - mount_fd = open_tree(-1, p, OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC); - if (mount_fd < 0) - return log_debug_errno(errno, "Failed to open tree of mounted filesystem '%s': %m", p); + /* This remounts all specified paths with the specified userns as idmap. It will do so in in the + * order specified in the strv: the expectation is that the top-level directories are at the + * beginning, and nested directories in the right, so that the tree can be built correctly from left + * to right. */ - /* Set the user namespace mapping attribute on the cloned mount point */ - if (mount_setattr(mount_fd, "", AT_EMPTY_PATH | AT_RECURSIVE, - &(struct mount_attr) { - .attr_set = MOUNT_ATTR_IDMAP, - .userns_fd = userns_fd, - }, sizeof(struct mount_attr)) < 0) - return log_debug_errno(errno, "Failed to change bind mount attributes for '%s': %m", p); + size_t n = strv_length(paths); + if (n == 0) /* Nothing to do? */ + return 0; - /* Remove the old mount point */ - r = umount_verbose(LOG_DEBUG, p, UMOUNT_NOFOLLOW); - if (r < 0) - return r; + int *mount_fds = NULL; + size_t n_mounts_fds = 0; + + CLEANUP_ARRAY(mount_fds, n_mounts_fds, close_many_and_free); + + mount_fds = new(int, n); + if (!mount_fds) + return log_oom_debug(); + + for (size_t i = 0; i < n; i++) { + int mntfd; + + /* Clone the mount point */ + mntfd = mount_fds[n_mounts_fds] = open_tree(-EBADF, paths[i], OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC); + if (mount_fds[n_mounts_fds] < 0) + return log_debug_errno(errno, "Failed to open tree of mounted filesystem '%s': %m", paths[i]); - /* And place the cloned version in its place */ - if (move_mount(mount_fd, "", -1, p, MOVE_MOUNT_F_EMPTY_PATH) < 0) - return log_debug_errno(errno, "Failed to attach UID mapped mount to '%s': %m", p); + n_mounts_fds++; + + /* Set the user namespace mapping attribute on the cloned mount point */ + if (mount_setattr(mntfd, "", AT_EMPTY_PATH, + &(struct mount_attr) { + .attr_set = MOUNT_ATTR_IDMAP, + .userns_fd = userns_fd, + }, sizeof(struct mount_attr)) < 0) + return log_debug_errno(errno, "Failed to change bind mount attributes for clone of '%s': %m", paths[i]); + } + + for (size_t i = n; i > 0; i--) { /* Unmount the paths right-to-left */ + /* Remove the old mount points now that we have a idmapped mounts as replacement for all of them */ + r = umount_verbose(LOG_DEBUG, paths[i-1], UMOUNT_NOFOLLOW); + if (r < 0) + return r; + } + + for (size_t i = 0; i < n; i++) { /* Mount the replacement mounts left-to-right */ + /* And place the cloned version in its place */ + log_debug("Mounting idmapped fs to '%s'", paths[i]); + if (move_mount(mount_fds[i], "", -EBADF, paths[i], MOVE_MOUNT_F_EMPTY_PATH) < 0) + return log_debug_errno(errno, "Failed to attach UID mapped mount to '%s': %m", paths[i]); + } return 0; } -int remount_idmap(const char *p, uid_t uid_shift, uid_t uid_range, uid_t owner, RemountIdmapping idmapping) { +int remount_idmap(char **p, uid_t uid_shift, uid_t uid_range, uid_t owner, RemountIdmapping idmapping) { _cleanup_close_ int userns_fd = -EBADF; userns_fd = make_userns(uid_shift, uid_range, owner, idmapping); diff --git a/src/shared/mount-util.h b/src/shared/mount-util.h index f06fd6de8c6..ef311049008 100644 --- a/src/shared/mount-util.h +++ b/src/shared/mount-util.h @@ -124,8 +124,8 @@ typedef enum RemountIdmapping { } RemountIdmapping; int make_userns(uid_t uid_shift, uid_t uid_range, uid_t owner, RemountIdmapping idmapping); -int remount_idmap_fd(const char *p, int userns_fd); -int remount_idmap(const char *p, uid_t uid_shift, uid_t uid_range, uid_t owner, RemountIdmapping idmapping); +int remount_idmap_fd(char **p, int userns_fd); +int remount_idmap(char **p, uid_t uid_shift, uid_t uid_range, uid_t owner, RemountIdmapping idmapping); int bind_mount_submounts( const char *source, -- 2.47.3