[PATCH OLK-6.6 0/4] fs: Backport four namespace fix patches

Al Viro (4): path_overmount(): avoid false negatives fix propagation graph breakage by MOVE_MOUNT_SET_GROUP move_mount(2) do_change_type(): refuse to operate on unmounted/not ours mounts clone_private_mnt(): make sure that caller has CAP_SYS_ADMIN in the right userns fs/namespace.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) -- 2.34.3

From: Al Viro <viro@zeniv.linux.org.uk> mainline inclusion from mainline-v6.16-rc1 commit 5f31c549382bcddbbd754c72c5433b19420d485d category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/ICGF26 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- Holding namespace_sem is enough to make sure that result remains valid. It is *not* enough to avoid false negatives from __lookup_mnt(). Mounts can be unhashed outside of namespace_sem (stuck children getting detached on final mntput() of lazy-umounted mount) and having an unrelated mount removed from the hash chain while we traverse it may end up with false negative from __lookup_mnt(). We need to sample and recheck the seqlock component of mount_lock... Bug predates the introduction of path_overmount() - it had come from the code in finish_automount() that got abstracted into that helper. Reviewed-by: Christian Brauner <brauner@kernel.org> Fixes: 26df6034fdb2 ("fix automount/automount race properly") Fixes: 6ac392815628 ("fs: allow to mount beneath top mount") Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> --- fs/namespace.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index dd2313f41034..8977588be826 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -3012,22 +3012,29 @@ static int do_set_group(struct path *from_path, struct path *to_path) * @path: path to check * * Check if path is overmounted, i.e., if there's a mount on top of * @path->mnt with @path->dentry as mountpoint. * - * Context: This function expects namespace_lock() to be held. + * Context: namespace_sem must be held at least shared. + * MUST NOT be called under lock_mount_hash() (there one should just + * call __lookup_mnt() and check if it returns NULL). * Return: If path is overmounted true is returned, false if not. */ static inline bool path_overmounted(const struct path *path) { + unsigned seq = read_seqbegin(&mount_lock); + bool no_child; + rcu_read_lock(); - if (unlikely(__lookup_mnt(path->mnt, path->dentry))) { - rcu_read_unlock(); - return true; - } + no_child = !__lookup_mnt(path->mnt, path->dentry); rcu_read_unlock(); - return false; + if (need_seqretry(&mount_lock, seq)) { + read_seqlock_excl(&mount_lock); + no_child = !__lookup_mnt(path->mnt, path->dentry); + read_sequnlock_excl(&mount_lock); + } + return unlikely(!no_child); } /** * can_move_mount_beneath - check that we can mount beneath the top mount * @from: mount to mount beneath -- 2.34.3

From: Al Viro <viro@zeniv.linux.org.uk> mainline inclusion from mainline-v6.16-rc1 commit d8cc0362f918d020ca1340d7694f07062dc30f36 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/ICGF26 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- 9ffb14ef61ba "move_mount: allow to add a mount into an existing group" breaks assertions on ->mnt_share/->mnt_slave. For once, the data structures in question are actually documented. Documentation/filesystem/sharedsubtree.rst: All vfsmounts in a peer group have the same ->mnt_master. If it is non-NULL, they form a contiguous (ordered) segment of slave list. do_set_group() puts a mount into the same place in propagation graph as the old one. As the result, if old mount gets events from somewhere and is not a pure event sink, new one needs to be placed next to the old one in the slave list the old one's on. If it is a pure event sink, we only need to make sure the new one doesn't end up in the middle of some peer group. "move_mount: allow to add a mount into an existing group" ends up putting the new one in the beginning of list; that's definitely not going to be in the middle of anything, so that's fine for case when old is not marked shared. In case when old one _is_ marked shared (i.e. is not a pure event sink), that breaks the assumptions of propagation graph iterators. Put the new mount next to the old one on the list - that does the right thing in "old is marked shared" case and is just as correct as the current behaviour if old is not marked shared (kudos to Pavel for pointing that out - my original suggested fix changed behaviour in the "nor marked" case, which complicated things for no good reason). Reviewed-by: Christian Brauner <brauner@kernel.org> Fixes: 9ffb14ef61ba ("move_mount: allow to add a mount into an existing group") Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> --- fs/namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/namespace.c b/fs/namespace.c index 8977588be826..e01e255a5818 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2987,11 +2987,11 @@ static int do_set_group(struct path *from_path, struct path *to_path) goto out; if (IS_MNT_SLAVE(from)) { struct mount *m = from->mnt_master; - list_add(&to->mnt_slave, &m->mnt_slave_list); + list_add(&to->mnt_slave, &from->mnt_slave); to->mnt_master = m; } if (IS_MNT_SHARED(from)) { to->mnt_group_id = from->mnt_group_id; -- 2.34.3

From: Al Viro <viro@zeniv.linux.org.uk> mainline inclusion from mainline-v6.16-rc1 commit 12f147ddd6de7382dad54812e65f3f08d05809fc category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/ICGF26 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- Ensure that propagation settings can only be changed for mounts located in the caller's mount namespace. This change aligns permission checking with the rest of mount(2). Reviewed-by: Christian Brauner <brauner@kernel.org> Fixes: 07b20889e305 ("beginning of the shared-subtree proper") Reported-by: "Orlando, Noah" <Noah.Orlando@deshaw.com> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> --- fs/namespace.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/namespace.c b/fs/namespace.c index e01e255a5818..c28b26211d22 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2549,10 +2549,14 @@ static int do_change_type(struct path *path, int ms_flags) type = flags_to_propagation_type(ms_flags); if (!type) return -EINVAL; namespace_lock(); + if (!check_mnt(mnt)) { + err = -EINVAL; + goto out_unlock; + } if (type == MS_SHARED) { err = invent_group_ids(mnt, recurse); if (err) goto out_unlock; } -- 2.34.3

From: Al Viro <viro@zeniv.linux.org.uk> mainline inclusion from mainline-v6.16-rc1 commit c28f922c9dcee0e4876a2c095939d77fe7e15116 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/ICGF26 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- What we want is to verify there is that clone won't expose something hidden by a mount we wouldn't be able to undo. "Wouldn't be able to undo" may be a result of MNT_LOCKED on a child, but it may also come from lacking admin rights in the userns of the namespace mount belongs to. clone_private_mnt() checks the former, but not the latter. There's a number of rather confusing CAP_SYS_ADMIN checks in various userns during the mount, especially with the new mount API; they serve different purposes and in case of clone_private_mnt() they usually, but not always end up covering the missing check mentioned above. Reviewed-by: Christian Brauner <brauner@kernel.org> Reported-by: "Orlando, Noah" <Noah.Orlando@deshaw.com> Fixes: 427215d85e8d ("ovl: prevent private clone if bind mount is not allowed") Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Conflicts: fs/namespace.c [Conflicts with mainline commit db04662e2f4f ("fs: allow detached mounts in clone_private_mount()") and commit 1f282cdc1d21 ("fs/fhandle.c: fix a race in call of has_locked_children()").] Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> --- fs/namespace.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs/namespace.c b/fs/namespace.c index c28b26211d22..5a74d92dec6b 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2102,10 +2102,15 @@ struct vfsmount *clone_private_mount(const struct path *path) goto invalid; if (!check_mnt(old_mnt)) goto invalid; + if (!ns_capable(old_mnt->mnt_ns->user_ns, CAP_SYS_ADMIN)) { + up_read(&namespace_sem); + return ERR_PTR(-EPERM); + } + if (has_locked_children(old_mnt, path->dentry)) goto invalid; new_mnt = clone_mnt(old_mnt, path->dentry, CL_PRIVATE); up_read(&namespace_sem); -- 2.34.3

反馈: 您发送到kernel@openeuler.org的补丁/补丁集,已成功转换为PR! PR链接地址: https://gitee.com/openeuler/kernel/pulls/16884 邮件列表地址:https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/IEB... FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/16884 Mailing list address: https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/IEB...
participants (2)
-
patchwork bot
-
Wang Zhaolong