Re: [PATCH] user_ns: Add support for unprivileged remount

[Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2047 bytes --]

On 2012/09/14 17:33, Eric W. Biederman wrote:
> Zhao Hongjiang <zhaohongjiang37-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> 
>> On 2012-9-14 5:26, Eric W. Biederman wrote:
>>> Zhao Hongjiang <zhaohongjiang37-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>>>
>>>> From: Zhao Hongjiang <zhaohongjiang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>>
>>>> Relax the permission checks to allow unprivileged users that have
>>>> CAP_SYS_ADMIN permissions in the user namespace referred to by the
>>>> current mount namespace to be allowed to remount filesystems.
>>>
>>> Remount in general make filesystem configuration changes not mount level
>>> changes.
>>>
>>> In general remount is not safe for unprivielged users.
>>>
>>> Do you have a use case where you need to remount a filesystem?
>>
>> As we can do a umount+mount，I don't see why remount operation is not allowed.
>> Shouldn't we add checks in remount path in the specific filesystem to ensure
>> safety instead when we enable unprivilleged mount?
> 
> But the thing is remount != mount+umount.  Remount is change lowlevel
> filesystem options.
> 
> The basic danger is if someone in the primary user namespace mounted a
> filesystem, and then we cloned that filesystem.
> 
> umounting filesystems is ok.  There reference count will drop or they
> will just unmount if the ref count goes to zero.
> 
> However mount -o remount -r /home could very easily remount everyone's
> home directory in all mount namespaces read-only by making the
> filesystem itself readonly.
> 
> That danger applies even to some extent even if the options are safe for
> us to perform at the filesystem level.
> 
> Now that doesn't mean remount is a hopeless operation.  What it does
> mean is that we need to be very carefully with enabling remounting
> of a filesystem.
> 
Hi Eric

what's you idea about the patch below.
Maybe it better to add a new fs_flags FS_USERNS_REMOUNT?
It's not a good experience that remount is disabled in container.

Thanks!

[-- Attachment #2: 0001-userns-allow-remount-filesystem-in-un-init-userns.patch --]
[-- Type: text/x-patch, Size: 1079 bytes --]

From 8c5a01c007d72c748018665d3bd27cd2bde52c0f Mon Sep 17 00:00:00 2001
From: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
Date: Thu, 17 Jan 2013 14:41:00 +0800
Subject: [PATCH] userns: allow remount filesystem in un-init userns

The proc and sysfs filesystem already enable userns support,
remounting these filesystems in un-init userns do no harm
to the host.

Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
 fs/namespace.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 55605c5..b9d83fb 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1748,7 +1748,10 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
 	struct super_block *sb = path->mnt->mnt_sb;
 	struct mount *mnt = real_mount(path->mnt);
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (sb->s_type->fs_flags & FS_USERNS_MOUNT) {
+		if (!ns_capable(mnt->mnt_ns->user_ns, CAP_SYS_ADMIN))
+			return -EPERM;
+	} else if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	if (!check_mnt(mnt))
-- 
1.7.11.7


[-- Attachment #3: Type: text/plain, Size: 205 bytes --]

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers