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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.