login
Header Space

 
 

Re: [patch 8/9] unprivileged mounts: propagation: inherit owner from parent

Previous thread: [patch 2/9] unprivileged mounts: allow unprivileged umount by Miklos Szeredi on Tuesday, January 8, 2008 - 7:35 am. (2 messages)

Next thread: [patch 4/9] unprivileged mounts: propagate error values from clone_mnt by Miklos Szeredi on Tuesday, January 8, 2008 - 7:35 am. (3 messages)
To: <akpm@...>, <hch@...>, <serue@...>, <viro@...>, <ebiederm@...>, <kzak@...>
Cc: <linux-fsdevel@...>, <linux-kernel@...>, <containers@...>, <util-linux-ng@...>
Date: Tuesday, January 8, 2008 - 7:35 am

From: Miklos Szeredi &lt;mszeredi@suse.cz&gt;

On mount propagation, let the owner of the clone be inherited from the
parent into which it has been propagated.  Also if the parent has the
"nosuid" flag, set this flag for the child as well.

This makes sense for example, when propagation is set up from the
initial namespace into a per-user namespace, where some or all of the
mounts may be owned by the user.

Signed-off-by: Miklos Szeredi &lt;mszeredi@suse.cz&gt;
---

Index: linux/fs/namespace.c
===================================================================
--- linux.orig/fs/namespace.c	2008-01-04 13:48:14.000000000 +0100
+++ linux/fs/namespace.c	2008-01-04 13:49:52.000000000 +0100
@@ -500,10 +500,10 @@ static int reserve_user_mount(void)
 	return err;
 }
 
-static void __set_mnt_user(struct vfsmount *mnt)
+static void __set_mnt_user(struct vfsmount *mnt, uid_t owner)
 {
 	BUG_ON(mnt-&gt;mnt_flags &amp; MNT_USER);
-	mnt-&gt;mnt_uid = current-&gt;fsuid;
+	mnt-&gt;mnt_uid = owner;
 	mnt-&gt;mnt_flags |= MNT_USER;
 
 	if (!capable(CAP_SETUID))
@@ -514,7 +514,7 @@ static void __set_mnt_user(struct vfsmou
 
 static void set_mnt_user(struct vfsmount *mnt)
 {
-	__set_mnt_user(mnt);
+	__set_mnt_user(mnt, current-&gt;fsuid);
 	spin_lock(&amp;vfsmount_lock);
 	nr_user_mounts++;
 	spin_unlock(&amp;vfsmount_lock);
@@ -530,7 +530,7 @@ static void clear_mnt_user(struct vfsmou
 }
 
 static struct vfsmount *clone_mnt(struct vfsmount *old, struct dentry *root,
-					int flag)
+					int flag, uid_t owner)
 {
 	struct super_block *sb = old-&gt;mnt_sb;
 	struct vfsmount *mnt;
@@ -554,7 +554,10 @@ static struct vfsmount *clone_mnt(struct
 	/* don't copy the MNT_USER flag */
 	mnt-&gt;mnt_flags &amp;= ~MNT_USER;
 	if (flag &amp; CL_SETUSER)
-		__set_mnt_user(mnt);
+		__set_mnt_user(mnt, owner);
+
+	if (flag &amp; CL_NOSUID)
+		mnt-&gt;mnt_flags |= MNT_NOSUID;
 
 	if (flag &amp; CL_SLAVE) {
 		list_add(&amp;mnt-&gt;mnt_slave, &amp;old-&gt;mnt_slave_list);
@@ -1060,7 +1063,7...
To: Miklos Szeredi <miklos@...>
Cc: <akpm@...>, <hch@...>, <serue@...>, <viro@...>, <ebiederm@...>, <kzak@...>, <linux-fsdevel@...>, <linux-kernel@...>, <containers@...>, <util-linux-ng@...>
Date: Monday, January 14, 2008 - 7:13 pm

What about nodev?

thanks,
--
To: <serue@...>
Cc: <miklos@...>, <akpm@...>, <hch@...>, <serue@...>, <viro@...>, <ebiederm@...>, <kzak@...>, <linux-fsdevel@...>, <linux-kernel@...>, <containers@...>, <util-linux-ng@...>
Date: Tuesday, January 15, 2008 - 6:39 am

Hmm, I think the nosuid thing is meant to prevent suid mounts being
introduced into a "suidless" namespace.  This doesn't apply to dev
mounts, which are quite safe in a suidless environment, as long as the
user is not able to create devices.  But that should be taken care of
by capability tests.

I'll update the description.

Thanks,
Miklos
--
To: Miklos Szeredi <miklos@...>
Cc: <serue@...>, <akpm@...>, <hch@...>, <viro@...>, <ebiederm@...>, <kzak@...>, <linux-fsdevel@...>, <linux-kernel@...>, <containers@...>, <util-linux-ng@...>
Date: Tuesday, January 15, 2008 - 10:21 am

Hmm,

Part of me wants to say the safest thing for now would be to refuse
mounts propagation from non-user mounts to user mounts.

I assume you're thinking about a fully user-mounted chroot, where
the user woudl still want to be able to stick in a cdrom and have
it automounted under /mnt/cdrom, propagated from the root mounts ns?

But then are there no devices which the user could create on a floppy
while inserted into his own laptop, owned by his own uid, then insert
into this machine, and use the device under the auto-mounted /dev/floppy
to gain inappropriate access?

-serge
--
To: <serue@...>
Cc: <miklos@...>, <serue@...>, <akpm@...>, <hch@...>, <viro@...>, <ebiederm@...>, <kzak@...>, <linux-fsdevel@...>, <linux-kernel@...>, <containers@...>, <util-linux-ng@...>
Date: Tuesday, January 15, 2008 - 10:37 am

I assume, that the floppy and cdrom are already mounted with
nosuid,nodev.

The problem case is I think is if a sysadmin does some mounting in the
initial namespace, and this is propagated into the fully user-mounted
namespace (or chroot), so that a mount with suid binaries slips in.
Which is bad, because the user may be able rearange the namespace, to
trick the suid program to something it should not do.

OTOH, a mount with devices can't be abused this way, since it is not
possible to gain privileges to files/devices just by rearanging the
mounts.

Miklos
--
To: Miklos Szeredi <miklos@...>
Cc: <serue@...>, <akpm@...>, <hch@...>, <viro@...>, <ebiederm@...>, <kzak@...>, <linux-fsdevel@...>, <linux-kernel@...>, <containers@...>, <util-linux-ng@...>
Date: Tuesday, January 15, 2008 - 10:59 am

Yeah, of course, what I'm saying is no different whether the upper mount

And really this shouldn't be an issue at all - the usermount chroot
would be set up under something like /share/hallyn/root, so the admin
would have to purposely set up propagation into that tree, so this

Thanks for humoring me,

-serge
--
Previous thread: [patch 2/9] unprivileged mounts: allow unprivileged umount by Miklos Szeredi on Tuesday, January 8, 2008 - 7:35 am. (2 messages)

Next thread: [patch 4/9] unprivileged mounts: propagate error values from clone_mnt by Miklos Szeredi on Tuesday, January 8, 2008 - 7:35 am. (3 messages)
speck-geostationary