From: Alan Jenkins <alan.christopher.jenkins@gmail.com>
To: David Howells <dhowells@redhat.com>, viro@zeniv.linux.org.uk
Cc: linux-fsdevel@vger.kernel.org, torvalds@linux-foundation.org,
ebiederm@xmission.com, linux-security-module@vger.kernel.org
Subject: Re: [PATCH 04/43] separate copying and locking mount tree on cross-userns copies
Date: Wed, 20 Feb 2019 18:55:27 +0000 [thread overview]
Message-ID: <e967b416-dcef-2ce4-8775-2eb67da7d045@gmail.com> (raw)
In-Reply-To: <155059371731.12449.5751025556744658291.stgit@warthog.procyon.org.uk>
On 19/02/2019 16:28, David Howells wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
>
> Rather than having propagate_mnt() check doing unprivileged copies,
> lock them before commit_tree().
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>
> fs/namespace.c | 59 +++++++++++++++++++++++++++++++++++---------------------
> fs/pnode.c | 5 -----
> fs/pnode.h | 3 +--
> 3 files changed, 38 insertions(+), 29 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index a677b59efd74..9ed2f2930dfd 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1013,27 +1013,6 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root,
>
> mnt->mnt.mnt_flags = old->mnt.mnt_flags;
> mnt->mnt.mnt_flags &= ~(MNT_WRITE_HOLD|MNT_MARKED|MNT_INTERNAL);
> - /* Don't allow unprivileged users to change mount flags */
> - if (flag & CL_UNPRIVILEGED) {
> - mnt->mnt.mnt_flags |= MNT_LOCK_ATIME;
> -
> - if (mnt->mnt.mnt_flags & MNT_READONLY)
> - mnt->mnt.mnt_flags |= MNT_LOCK_READONLY;
> -
> - if (mnt->mnt.mnt_flags & MNT_NODEV)
> - mnt->mnt.mnt_flags |= MNT_LOCK_NODEV;
> -
> - if (mnt->mnt.mnt_flags & MNT_NOSUID)
> - mnt->mnt.mnt_flags |= MNT_LOCK_NOSUID;
> -
> - if (mnt->mnt.mnt_flags & MNT_NOEXEC)
> - mnt->mnt.mnt_flags |= MNT_LOCK_NOEXEC;
> - }
> -
> - /* Don't allow unprivileged users to reveal what is under a mount */
> - if ((flag & CL_UNPRIVILEGED) &&
> - (!(flag & CL_EXPIRE) || list_empty(&old->mnt_expire)))
> - mnt->mnt.mnt_flags |= MNT_LOCKED;
>
> atomic_inc(&sb->s_active);
> mnt->mnt.mnt_sb = sb;
> @@ -1837,6 +1816,33 @@ int iterate_mounts(int (*f)(struct vfsmount *, void *), void *arg,
> return 0;
> }
>
> +static void lock_mnt_tree(struct mount *mnt)
> +{
> + struct mount *p;
> +
> + for (p = mnt; p; p = next_mnt(p, mnt)) {
> + int flags = p->mnt.mnt_flags;
> + /* Don't allow unprivileged users to change mount flags */
> + flags |= MNT_LOCK_ATIME;
> +
> + if (flags & MNT_READONLY)
> + flags |= MNT_LOCK_READONLY;
> +
> + if (flags & MNT_NODEV)
> + flags |= MNT_LOCK_NODEV;
> +
> + if (flags & MNT_NOSUID)
> + flags |= MNT_LOCK_NOSUID;
> +
> + if (flags & MNT_NOEXEC)
> + flags |= MNT_LOCK_NOEXEC;
> + /* Don't allow unprivileged users to reveal what is under a mount */
> + if (list_empty(&p->mnt_expire))
> + flags |= MNT_LOCKED;
> + p->mnt.mnt_flags = flags;
> + }
> +}
> +
> static void cleanup_group_ids(struct mount *mnt, struct mount *end)
> {
> struct mount *p;
> @@ -1954,6 +1960,7 @@ static int attach_recursive_mnt(struct mount *source_mnt,
> struct mountpoint *dest_mp,
> struct path *parent_path)
> {
> + struct user_namespace *user_ns = current->nsproxy->mnt_ns->user_ns;
> HLIST_HEAD(tree_list);
> struct mnt_namespace *ns = dest_mnt->mnt_ns;
> struct mountpoint *smp;
> @@ -2004,6 +2011,9 @@ static int attach_recursive_mnt(struct mount *source_mnt,
> child->mnt_mountpoint);
> if (q)
> mnt_change_mountpoint(child, smp, q);
> + /* Notice when we are propagating across user namespaces */
> + if (child->mnt_parent->mnt_ns->user_ns != user_ns)
> + lock_mnt_tree(child);
> commit_tree(child);
> }
> put_mountpoint(smp);
> @@ -2941,13 +2951,18 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns,
> /* First pass: copy the tree topology */
> copy_flags = CL_COPY_UNBINDABLE | CL_EXPIRE;
> if (user_ns != ns->user_ns)
> - copy_flags |= CL_SHARED_TO_SLAVE | CL_UNPRIVILEGED;
> + copy_flags |= CL_SHARED_TO_SLAVE;
> new = copy_tree(old, old->mnt.mnt_root, copy_flags);
> if (IS_ERR(new)) {
> namespace_unlock();
> free_mnt_ns(new_ns);
> return ERR_CAST(new);
> }
> + if (user_ns != ns->user_ns) {
> + lock_mount_hash();
> + lock_mnt_tree(new);
> + unlock_mount_hash();
> + }
> new_ns->root = new;
> list_add_tail(&new_ns->list, &new->mnt_list);
>
> diff --git a/fs/pnode.c b/fs/pnode.c
> index 1100e810d855..7ea6cfb65077 100644
> --- a/fs/pnode.c
> +++ b/fs/pnode.c
> @@ -214,7 +214,6 @@ static struct mount *next_group(struct mount *m, struct mount *origin)
> }
>
> /* all accesses are serialized by namespace_sem */
> -static struct user_namespace *user_ns;
> static struct mount *last_dest, *first_source, *last_source, *dest_master;
> static struct mountpoint *mp;
> static struct hlist_head *list;
> @@ -260,9 +259,6 @@ static int propagate_one(struct mount *m)
> type |= CL_MAKE_SHARED;
> }
>
> - /* Notice when we are propagating across user namespaces */
> - if (m->mnt_ns->user_ns != user_ns)
> - type |= CL_UNPRIVILEGED;
> child = copy_tree(last_source, last_source->mnt.mnt_root, type);
> if (IS_ERR(child))
> return PTR_ERR(child);
> @@ -303,7 +299,6 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp,
> * propagate_one(); everything is serialized by namespace_sem,
> * so globals will do just fine.
> */
> - user_ns = current->nsproxy->mnt_ns->user_ns;
> last_dest = dest_mnt;
> first_source = source_mnt;
> last_source = source_mnt;
> diff --git a/fs/pnode.h b/fs/pnode.h
> index dc87e65becd2..3960a83666cf 100644
> --- a/fs/pnode.h
> +++ b/fs/pnode.h
> @@ -27,8 +27,7 @@
> #define CL_MAKE_SHARED 0x08
> #define CL_PRIVATE 0x10
> #define CL_SHARED_TO_SLAVE 0x20
> -#define CL_UNPRIVILEGED 0x40
> -#define CL_COPY_MNT_NS_FILE 0x80
> +#define CL_COPY_MNT_NS_FILE 0x40
>
> #define CL_COPY_ALL (CL_COPY_UNBINDABLE | CL_COPY_MNT_NS_FILE)
>
>
>
I can see that this covers copy_mnt_ns(). It should also cover what
will happen in future, if you pass an OPEN_TREE_CLONE fd to a process
with a different mnt_ns and mnt_ns->user_ns, and that process mounts the
fd using move_mount(). However, I can't work out how this covers mount
propagation across namespaces.
The comment "Notice when we are propagating across user namespaces" is
moved to attach_recursive_mnt(). I can't find any call to
attach_recursive_mount() inside the mount propagation code. Am I
overlooking something?
Thanks
Alan
next prev parent reply other threads:[~2019-02-20 18:55 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-19 16:27 [PATCH 00/43] VFS: Introduce filesystem context David Howells
2019-02-19 16:28 ` [PATCH 01/43] fix cgroup_do_mount() handling of failure exits David Howells
2019-02-19 16:28 ` [PATCH 02/43] cgroup: saner refcounting for cgroup_root David Howells
2019-02-19 16:28 ` [PATCH 03/43] kill kernfs_pin_sb() David Howells
2019-02-19 16:28 ` [PATCH 04/43] separate copying and locking mount tree on cross-userns copies David Howells
2019-02-20 18:55 ` Alan Jenkins [this message]
2019-02-26 15:44 ` David Howells
2019-02-26 17:45 ` Alan Jenkins
2019-02-19 16:29 ` [PATCH 05/43] saner handling of temporary namespaces David Howells
2019-02-19 16:29 ` [PATCH 06/43] vfs: Introduce fs_context, switch vfs_kern_mount() to it David Howells
2019-02-19 16:29 ` [PATCH 07/43] new helpers: vfs_create_mount(), fc_mount() David Howells
2019-02-19 16:29 ` [PATCH 08/43] teach vfs_get_tree() to handle subtype, switch do_new_mount() to it David Howells
2019-02-19 16:29 ` [PATCH 09/43] new helper: do_new_mount_fc() David Howells
2019-02-19 16:29 ` [PATCH 10/43] vfs_get_tree(): evict the call of security_sb_kern_mount() David Howells
2019-02-19 16:29 ` [PATCH 11/43] convert do_remount_sb() to fs_context David Howells
2019-03-22 11:19 ` Andreas Schwab
2019-03-22 11:25 ` David Howells
2019-03-22 13:28 ` Andreas Schwab
2019-03-22 14:00 ` Andreas Schwab
2019-02-19 16:30 ` [PATCH 12/43] fs_context flavour for submounts David Howells
2019-02-19 16:30 ` [PATCH 13/43] introduce fs_context methods David Howells
2019-02-19 16:30 ` [PATCH 14/43] vfs: Introduce logging functions David Howells
2019-02-19 16:30 ` [PATCH 15/43] vfs: Add configuration parser helpers David Howells
2019-03-03 2:53 ` Al Viro
2019-02-19 16:30 ` [PATCH 16/43] vfs: Add LSM hooks for the new mount API David Howells
2019-02-19 16:30 ` [PATCH 17/43] selinux: Implement the new mount API LSM hooks David Howells
2019-02-19 16:30 ` [PATCH 18/43] smack: Implement filesystem context security hooks David Howells
2019-02-19 16:30 ` [PATCH 19/43] vfs: Put security flags into the fs_context struct David Howells
2019-02-19 16:31 ` [PATCH 20/43] vfs: Implement a filesystem superblock creation/configuration context David Howells
2019-02-19 16:31 ` [PATCH 21/43] convenience helpers: vfs_get_super() and sget_fc() David Howells
2019-02-19 16:31 ` [PATCH 22/43] introduce cloning of fs_context David Howells
2019-02-19 16:31 ` [PATCH 23/43] procfs: Move proc_fill_super() to fs/proc/root.c David Howells
2019-02-19 16:31 ` [PATCH 24/43] proc: Add fs_context support to procfs David Howells
2019-02-19 16:31 ` [PATCH 25/43] ipc: Convert mqueue fs to fs_context David Howells
2019-02-19 16:31 ` [PATCH 26/43] cgroup: start switching " David Howells
2019-02-19 16:32 ` [PATCH 27/43] cgroup: fold cgroup1_mount() into cgroup1_get_tree() David Howells
2019-02-19 16:32 ` [PATCH 28/43] cgroup: take options parsing into ->parse_monolithic() David Howells
2019-02-19 16:32 ` [PATCH 29/43] cgroup1: switch to option-by-option parsing David Howells
2019-02-19 16:32 ` [PATCH 30/43] cgroup2: " David Howells
2019-02-19 16:32 ` [PATCH 31/43] cgroup: stash cgroup_root reference into cgroup_fs_context David Howells
2019-02-19 16:32 ` [PATCH 32/43] cgroup_do_mount(): massage calling conventions David Howells
2019-02-19 16:32 ` [PATCH 33/43] cgroup1_get_tree(): separate "get cgroup_root to use" into a separate helper David Howells
2019-02-19 16:33 ` [PATCH 34/43] cgroup: store a reference to cgroup_ns into cgroup_fs_context David Howells
2019-02-19 16:33 ` [PATCH 35/43] kernfs, sysfs, cgroup, intel_rdt: Support fs_context David Howells
2019-02-19 16:33 ` [PATCH 36/43] cpuset: Use fs_context David Howells
2019-02-19 16:33 ` [PATCH 37/43] hugetlbfs: Convert to fs_context David Howells
2019-02-19 16:33 ` [PATCH 38/43] vfs: Remove kern_mount_data() David Howells
2019-02-19 16:33 ` [PATCH 39/43] vfs: Provide documentation for new mount API David Howells
2019-02-19 16:34 ` [PATCH 40/43] vfs: Implement logging through fs_context David Howells
2019-02-19 16:34 ` [PATCH 41/43] vfs: Add some logging to the core users of the fs_context log David Howells
2019-02-19 16:34 ` [PATCH 42/43] afs: Add fs_context support David Howells
2019-02-19 16:34 ` [PATCH 43/43] afs: Use fs_context to pass parameters over automount David Howells
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e967b416-dcef-2ce4-8775-2eb67da7d045@gmail.com \
--to=alan.christopher.jenkins@gmail.com \
--cc=dhowells@redhat.com \
--cc=ebiederm@xmission.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).