From: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
To: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] userns: allow privileged user to operate locked mount
Date: Fri, 15 Nov 2013 08:47:09 +0800 [thread overview]
Message-ID: <52856F0D.8090206@cn.fujitsu.com> (raw)
In-Reply-To: <874n7e4k78.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
On 11/15/2013 07:50 AM, Eric W. Biederman wrote:
> Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> writes:
>
>> Privileged user should have rights to mount/umount/move
>> these even locked mount.
>
> Hmm. This is pretty much a can't happen case, as the only exist in mount
> namespaces where the global root isn't the root. How are you getting
> into this situation? Using setns() ?
>
Before, priviged user can use setns to set his mount namespace to the
container's mount namespace, and change container's mount directly.
this patch just gives back host the control of container.
> Why would we even care?
>
> As implemented this patch does not handle nested user namespaces and
> that really worries me at a semantic level.
>
> We don't want to design cases where we can create containers in
> containers.
>
> Eric
>
>
>> Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
>> ---
>> fs/namespace.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index da5c494..7097fc7 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -1297,6 +1297,11 @@ static inline bool may_mount(void)
>> return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
>> }
>>
>> +static inline bool may_mount_lock(struct mount *mnt)
>> +{
>> + return !(mnt->mnt.mnt_flags & MNT_LOCKED) || capable(CAP_SYS_ADMIN);
>> +}
>> +
>> /*
>> * Now umount can handle mount points as well as block devices.
>> * This is important for filesystems which use unnamed block devices.
>> @@ -1330,7 +1335,7 @@ SYSCALL_DEFINE2(umount, char __user *, name, int, flags)
>> goto dput_and_out;
>> if (!check_mnt(mnt))
>> goto dput_and_out;
>> - if (mnt->mnt.mnt_flags & MNT_LOCKED)
>> + if (!may_mount_lock(mnt))
>> goto dput_and_out;
>>
>> retval = do_umount(mnt, flags);
>> @@ -1768,7 +1773,8 @@ static int do_loopback(struct path *path, const char *old_name,
>> if (!check_mnt(parent) || !check_mnt(old))
>> goto out2;
>>
>> - if (!recurse && has_locked_children(old, old_path.dentry))
>> + if (!recurse && has_locked_children(old, old_path.dentry) &&
>> + !capable(CAP_SYS_ADMIN))
>> goto out2;
>>
>> if (recurse)
>> @@ -1895,7 +1901,7 @@ static int do_move_mount(struct path *path, const char *old_name)
>> if (!check_mnt(p) || !check_mnt(old))
>> goto out1;
>>
>> - if (old->mnt.mnt_flags & MNT_LOCKED)
>> + if (!may_mount_lock(old))
>> goto out1;
>>
>> err = -EINVAL;
>> @@ -2679,7 +2685,7 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
>> goto out4;
>> if (!check_mnt(root_mnt) || !check_mnt(new_mnt))
>> goto out4;
>> - if (new_mnt->mnt.mnt_flags & MNT_LOCKED)
>> + if (!may_mount_lock(new_mnt))
>> goto out4;
>> error = -ENOENT;
>> if (d_unlinked(new.dentry))
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
WARNING: multiple messages have this Message-ID (diff)
From: Gao feng <gaofeng@cn.fujitsu.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: containers@lists.linux-foundation.org,
linux-fsdevel@vger.kernel.org, serge@hallyn.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] userns: allow privileged user to operate locked mount
Date: Fri, 15 Nov 2013 08:47:09 +0800 [thread overview]
Message-ID: <52856F0D.8090206@cn.fujitsu.com> (raw)
In-Reply-To: <874n7e4k78.fsf@xmission.com>
On 11/15/2013 07:50 AM, Eric W. Biederman wrote:
> Gao feng <gaofeng@cn.fujitsu.com> writes:
>
>> Privileged user should have rights to mount/umount/move
>> these even locked mount.
>
> Hmm. This is pretty much a can't happen case, as the only exist in mount
> namespaces where the global root isn't the root. How are you getting
> into this situation? Using setns() ?
>
Before, priviged user can use setns to set his mount namespace to the
container's mount namespace, and change container's mount directly.
this patch just gives back host the control of container.
> Why would we even care?
>
> As implemented this patch does not handle nested user namespaces and
> that really worries me at a semantic level.
>
> We don't want to design cases where we can create containers in
> containers.
>
> Eric
>
>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> ---
>> fs/namespace.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index da5c494..7097fc7 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -1297,6 +1297,11 @@ static inline bool may_mount(void)
>> return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
>> }
>>
>> +static inline bool may_mount_lock(struct mount *mnt)
>> +{
>> + return !(mnt->mnt.mnt_flags & MNT_LOCKED) || capable(CAP_SYS_ADMIN);
>> +}
>> +
>> /*
>> * Now umount can handle mount points as well as block devices.
>> * This is important for filesystems which use unnamed block devices.
>> @@ -1330,7 +1335,7 @@ SYSCALL_DEFINE2(umount, char __user *, name, int, flags)
>> goto dput_and_out;
>> if (!check_mnt(mnt))
>> goto dput_and_out;
>> - if (mnt->mnt.mnt_flags & MNT_LOCKED)
>> + if (!may_mount_lock(mnt))
>> goto dput_and_out;
>>
>> retval = do_umount(mnt, flags);
>> @@ -1768,7 +1773,8 @@ static int do_loopback(struct path *path, const char *old_name,
>> if (!check_mnt(parent) || !check_mnt(old))
>> goto out2;
>>
>> - if (!recurse && has_locked_children(old, old_path.dentry))
>> + if (!recurse && has_locked_children(old, old_path.dentry) &&
>> + !capable(CAP_SYS_ADMIN))
>> goto out2;
>>
>> if (recurse)
>> @@ -1895,7 +1901,7 @@ static int do_move_mount(struct path *path, const char *old_name)
>> if (!check_mnt(p) || !check_mnt(old))
>> goto out1;
>>
>> - if (old->mnt.mnt_flags & MNT_LOCKED)
>> + if (!may_mount_lock(old))
>> goto out1;
>>
>> err = -EINVAL;
>> @@ -2679,7 +2685,7 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
>> goto out4;
>> if (!check_mnt(root_mnt) || !check_mnt(new_mnt))
>> goto out4;
>> - if (new_mnt->mnt.mnt_flags & MNT_LOCKED)
>> + if (!may_mount_lock(new_mnt))
>> goto out4;
>> error = -ENOENT;
>> if (d_unlinked(new.dentry))
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
next prev parent reply other threads:[~2013-11-15 0:47 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-13 7:27 [PATCH] userns: allow privileged user to operate locked mount Gao feng
2013-11-13 7:27 ` Gao feng
[not found] ` <1384327663-12032-1-git-send-email-gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2013-11-14 23:50 ` Eric W. Biederman
2013-11-14 23:50 ` Eric W. Biederman
[not found] ` <874n7e4k78.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2013-11-15 0:47 ` Gao feng [this message]
2013-11-15 0:47 ` Gao feng
[not found] ` <52856F0D.8090206-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2013-11-15 1:38 ` Eric W. Biederman
2013-11-15 1:38 ` Eric W. Biederman
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=52856F0D.8090206@cn.fujitsu.com \
--to=gaofeng-bthxqxjhjhxqfuhtdcdx3a@public.gmane.org \
--cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
--cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
--cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
/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.