OpenVZ Forum - RDF feed
type: channel
items: [patch 0/9] mount ownership and unprivileged mount syscall (v6)
items: [patch 2/9] unprivileged mounts: allow unprivileged umount
items: [patch 4/9] unprivileged mounts: propagate error values from clone_mnt
items: [patch 6/9] unprivileged mounts: allow unprivileged mounts
items: [patch 8/9] unprivileged mounts: propagation: inherit owner from parent
items: [patch 9/9] unprivileged mounts: add "no submounts" flag
items: Re: [patch 6/9] unprivileged mounts: allow unprivileged mounts
items: Re: [patch 2/9] unprivileged mounts: allow unprivileged umount
items: Re: [patch 4/9] unprivileged mounts: propagate error values from clone_mnt
items: Re: [patch 8/9] unprivileged mounts: propagation: inherit owner from parent
items: Re: [patch 9/9] unprivileged mounts: add "no submounts" flag

[patch 0/9] mount ownership and unprivileged mount syscall (v6)
type: item
description: After much sitting in -mm, Andrew dropped this patchset due to conflicts with other stuff. It would be nice, if it could be reviewed in time for 2.6.26 (2.6.25 is closed as far as I understand). v5 -> v6: - update to latest -mm - preliminary util-linux-ng support (will post right after this series) Thanks, Miklos -- _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Miklos Szeredi
date: 2008-01-08T11:35:02-00:00

[patch 2/9] unprivileged mounts: allow unprivileged umount
type: item
description: From: Miklos Szeredi <mszeredi@suse.cz> The owner doesn't need sysadmin capabilities to call umount(). Similar behavior as umount(8) on mounts having "user=UID" option in /etc/mtab. The difference is that umount also checks /etc/fstab, presumably to exclude another mount on the same mountpoint. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- Index: linux/fs/namespace.c =================================================================== --- linux.orig/fs/namespace.c 2008-01-03 20:52:38.000000000 +0100 +++ linux/fs/namespace.c 2008-01-03 21:14:16.000000000 +0100 @@ -894,6 +894,27 @@ static int do_umount(struct vfsmount *mn return retval; } +static bool is_mount_owner(struct vfsmount *mnt, uid_t uid) +{ + return (mnt->mnt_flags & MNT_USER) && mnt->mnt_uid == uid; +} + +/* + * umount is permitted for + * - sysadmin + * - mount owner, if not forced umount + */ +static bool permit_umount(struct vfsmount *mnt, int flags) +{ + if (capable(CAP_SYS_ADMIN)) + return true; + + if (flags & MNT_FORCE) + return false; + + return is_mount_owner(mnt, current->fsuid); +} + /* * Now umount can handle mount points as well as block devices. * This is important for filesystems which use unnamed block devices. @@ -917,7 +938,7 @@ asmlinkage long sys_umount(char __user * goto dput_and_out; retval = -EPERM; - if (!capable(CAP_SYS_ADMIN)) + if (!permit_umount(nd.path.mnt, flags)) goto dput_and_out; retval = do_umount(nd.path.mnt, flags); -- _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Miklos Szeredi
date: 2008-01-08T11:35:04-00:00

[patch 4/9] unprivileged mounts: propagate error values from clone_mnt
type: item
description: From: Miklos Szeredi <mszeredi@suse.cz> Allow clone_mnt() to return errors other than ENOMEM. This will be used for returning a different error value when the number of user mounts goes over the limit. Fix copy_tree() to return EPERM for unbindable mounts. Don't propagate further from dup_mnt_ns() as that copy_tree() can only fail with -ENOMEM. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- Index: linux/fs/namespace.c =================================================================== --- linux.orig/fs/namespace.c 2008-01-04 13:47:09.000000000 +0100 +++ linux/fs/namespace.c 2008-01-04 13:47:49.000000000 +0100 @@ -512,41 +512,42 @@ static struct vfsmount *clone_mnt(struct struct super_block *sb = old->mnt_sb; struct vfsmount *mnt = alloc_vfsmnt(old->mnt_devname); - if (mnt) { - mnt->mnt_flags = old->mnt_flags; - atomic_inc(&sb->s_active); - mnt->mnt_sb = sb; - mnt->mnt_root = dget(root); - mnt->mnt_mountpoint = mnt->mnt_root; - mnt->mnt_parent = mnt; - - /* don't copy the MNT_USER flag */ - mnt->mnt_flags &= ~MNT_USER; - if (flag & CL_SETUSER) - set_mnt_user(mnt); - - if (flag & CL_SLAVE) { - list_add(&mnt->mnt_slave, &old->mnt_slave_list); - mnt->mnt_master = old; - CLEAR_MNT_SHARED(mnt); - } else if (!(flag & CL_PRIVATE)) { - if ((flag & CL_PROPAGATION) || IS_MNT_SHARED(old)) - list_add(&mnt->mnt_share, &old->mnt_share); - if (IS_MNT_SLAVE(old)) - list_add(&mnt->mnt_slave, &old->mnt_slave); - mnt->mnt_master = old->mnt_master; - } - if (flag & CL_MAKE_SHARED) - set_mnt_shared(mnt); + if (!mnt) + return ERR_PTR(-ENOMEM); - /* stick the duplicate mount on the same expiry list - * as the original if that was on one */ - if (flag & CL_EXPIRE) { - spin_lock(&vfsmount_lock); - if (!list_empty(&old->mnt_expire)) - list_add(&mnt->mnt_expire, &old->mnt_expire); - spin_unlock(&vfsmount_lock); - } + mnt->mnt_flags = old->mnt_flags; + atomic_inc(&sb->s_active); + mnt->mnt_sb = sb; + mnt->mnt_root = dget(root); + mnt->mnt_mountpoint = mnt->mnt_root; + mnt->mnt_parent = mn…
creator: Miklos Szeredi
date: 2008-01-08T11:35:06-00:00

[patch 6/9] unprivileged mounts: allow unprivileged mounts
type: item
description: From: Miklos Szeredi <mszeredi@suse.cz> Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. For "safe" filesystems also allow unprivileged forced unmounting. Move subtype handling from do_kern_mount() into do_new_mount(). All other callers are kernel-internal and do not need subtype support. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- Index: linux/fs/namespace.c =================================================================== --- linux.orig/fs/namespace.c 2008-01-03 21:20:11.000000000 +0100 +++ linux/fs/namespace.c 2008-01-03 21:21:06.000000000 +0100 @@ -960,14 +960,16 @@ static bool is_mount_owner(struct vfsmou /* * umount is permitted for * - sysadmin - * - mount owner, if not forced umount + * - mount owner + * o if not forced umount, + * o if forced umount, and filesystem is "safe" */ static bool permit_umount(struct vfsmount *mnt, int flags) { if (capable(CAP_SYS_ADMIN)) return true; - if (flags & MNT_FORCE) + if ((flags & MNT_FORCE) && !(mnt->mnt_sb->s_type->fs_flags & FS_SAFE)) return false; return is_mount_owner(mnt, current->fsuid); @@ -1025,13 +1027,17 @@ asmlinkage long sys_oldumount(char __use * - mountpoint is not a symlink * - mountpoint is in a mount owned by the user */ -static bool permit_mount(struct nameidata *nd, int *flags) +static bool permit_mount(struct nameidata *nd, struct file_system_type *type, + int *flags) { struct inode *inode = nd->path.dentry->d_inode; if (capable(CAP_SYS_ADMIN)) return true; + if (type && !(type->fs_flags & FS_SAFE)) + return false; + if (S_ISLNK(inode->i_mode)) return false; @@ -1285,7 +1291,7 @@ static int do_loopback(struct nameidata struct vfsmount *mnt = NULL; int err; - if (!permit_mount(nd, &flags)) + if (!permit_mount(nd, NULL, &flags)) return -EPERM; if (!old_name || !*old_name) return -…
creator: Miklos Szeredi
date: 2008-01-08T11:35:08-00:00

[patch 8/9] unprivileged mounts: propagation: inherit owner from parent
type: item
description: From: Miklos Szeredi <mszeredi@suse.cz> 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 <mszeredi@suse.cz> --- 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->mnt_flags & MNT_USER); - mnt->mnt_uid = current->fsuid; + mnt->mnt_uid = owner; mnt->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->fsuid); spin_lock(&vfsmount_lock); nr_user_mounts++; spin_unlock(&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->mnt_sb; struct vfsmount *mnt; @@ -554,7 +554,10 @@ static struct vfsmount *clone_mnt(struct /* don't copy the MNT_USER flag */ mnt->mnt_flags &= ~MNT_USER; if (flag & CL_SETUSER) - __set_mnt_user(mnt); + __set_mnt_user(mnt, owner); + + if (flag & CL_NOSUID) + mnt->mnt_flags |= MNT_NOSUID; if (flag & CL_SLAVE) { list_add(&mnt->mnt_slave, &old->mnt_slave_list); @@ -1060,7 +1063,7 @@ static int lives_below_in_same_fs(struct } struct vfsmount *copy_tree(struct vfsmount *mnt, struct dentry *dentry, - int flag) + int flag, uid_t owner)…
creator: Miklos Szeredi
date: 2008-01-08T11:35:10-00:00

[patch 9/9] unprivileged mounts: add "no submounts" flag
type: item
description: From: Miklos Szeredi <mszeredi@suse.cz> Add a new mount flag "nomnt", which denies submounts for the owner. This would be useful, if we want to support traditional /etc/fstab based user mounts. In this case mount(8) would still have to be suid-root, to check the mountpoint against the user/users flag in /etc/fstab, but /etc/mtab would no longer be mandatory for storing the actual owner of the mount. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- Index: linux/fs/namespace.c =================================================================== --- linux.orig/fs/namespace.c 2008-01-04 13:49:52.000000000 +0100 +++ linux/fs/namespace.c 2008-01-04 13:50:28.000000000 +0100 @@ -694,6 +694,7 @@ static int show_vfsmnt(struct seq_file * { MNT_NOATIME, ",noatime" }, { MNT_NODIRATIME, ",nodiratime" }, { MNT_RELATIME, ",relatime" }, + { MNT_NOMNT, ",nomnt" }, { 0, NULL } }; struct proc_fs_info *fs_infop; @@ -1044,6 +1045,9 @@ static bool permit_mount(struct nameidat if (S_ISLNK(inode->i_mode)) return false; + if (nd->path.mnt->mnt_flags & MNT_NOMNT) + return false; + if (!is_mount_owner(nd->path.mnt, current->fsuid)) return false; @@ -1888,9 +1892,11 @@ long do_mount(char *dev_name, char *dir_ mnt_flags |= MNT_RELATIME; if (flags & MS_RDONLY) mnt_flags |= MNT_READONLY; + if (flags & MS_NOMNT) + mnt_flags |= MNT_NOMNT; - flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | - MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT); + flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | MS_NOATIME | + MS_NODIRATIME | MS_RELATIME | MS_KERNMOUNT | MS_NOMNT); /* ... and get the mountpoint */ retval = path_lookup(dir_name, LOOKUP_FOLLOW, &nd); Index: linux/include/linux/fs.h =================================================================== --- linux.orig/include/linux/fs.h 2008-01-04 13:49:12.000000000 +0100 +++ linux/include/linux/fs.h 2008-01-04 13:49:58.000000000 +0100 @@ -130,6 +130,7 @@ extern int dir_notify_enable; #define MS_KERNMOUNT (1<<22) /* this is a kern_mou…
creator: Miklos Szeredi
date: 2008-01-08T11:35:11-00:00

Re: [patch 6/9] unprivileged mounts: allow unprivileged mounts
type: item
description: On Tue, Jan 08, 2008 at 12:35:08PM +0100, Miklos Szeredi wrote: > Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of > this filesystem may not constitute a security problem. > > Since most filesystems haven't been designed with unprivileged mounting in > mind, a thorough audit is needed before setting this flag. > > For "safe" filesystems also allow unprivileged forced unmounting. What about to list "safe" filesystems anywhere in /proc/fs/ ? I think it's very important information for admins. Note, your patch for mount(8) is always trying to use unprivileged mount(2) for non-root users. It's overkill when unprivileged mounts are supported for bind mounts and fuse only. It would be nice to check if FS is "safe" before switch to unprivileged mode. The "safe" definition is also very subjective and it depends on your level of paranoia. There should be a way (e.g. /proc) how control and modify the list of "safe" filesystems. For example I have no problem to mark cifs as "safe" for my home server. Karel -- Karel Zak <kzak@redhat.com> _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Karel Zak
date: 2008-01-09T11:11:20-00:00

Re: [patch 6/9] unprivileged mounts: allow unprivileged mounts
type: item
description: > On Tue, Jan 08, 2008 at 12:35:08PM +0100, Miklos Szeredi wrote: > > Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of > > this filesystem may not constitute a security problem. > > > > Since most filesystems haven't been designed with unprivileged mounting in > > mind, a thorough audit is needed before setting this flag. > > > > For "safe" filesystems also allow unprivileged forced unmounting. > > What about to list "safe" filesystems anywhere in /proc/fs/ ? I think > it's very important information for admins. Makes sense. I'll cook up something. > Note, your patch for mount(8) is always trying to use unprivileged > mount(2) for non-root users. It's overkill when unprivileged mounts are > supported for bind mounts and fuse only. It would be nice to check > if FS is "safe" before switch to unprivileged mode. I think the little gain in performance is not worth the added complexity. Especially if the added complexity is in the privileged part, and itself can be a source of security holes. > The "safe" definition is also very subjective and it depends on your > level of paranoia. There should be a way (e.g. /proc) how control and > modify the list of "safe" filesystems. For example I have no problem > to mark cifs as "safe" for my home server. OK, also makes some sense. Pavel's examples do point out that fuse isn't as safe as I'd like it to be, so perhaps it would make sense to default to just bind mounts being allowed, and having to explicity enable unprivileged fuse mounts with a sysctl or whatever. Miklos _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Miklos Szeredi
date: 2008-01-09T12:41:40-00:00

Re: [patch 2/9] unprivileged mounts: allow unprivileged umount
type: item
description: Quoting Miklos Szeredi (miklos@szeredi.hu): > From: Miklos Szeredi <mszeredi@suse.cz> > > The owner doesn't need sysadmin capabilities to call umount(). > > Similar behavior as umount(8) on mounts having "user=UID" option in /etc/mtab. > The difference is that umount also checks /etc/fstab, presumably to exclude > another mount on the same mountpoint. > > Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> Acked-by: Serge Hallyn <serue@us.ibm.com> > --- > > Index: linux/fs/namespace.c > =================================================================== > --- linux.orig/fs/namespace.c 2008-01-03 20:52:38.000000000 +0100 > +++ linux/fs/namespace.c 2008-01-03 21:14:16.000000000 +0100 > @@ -894,6 +894,27 @@ static int do_umount(struct vfsmount *mn > return retval; > } > > +static bool is_mount_owner(struct vfsmount *mnt, uid_t uid) > +{ > + return (mnt->mnt_flags & MNT_USER) && mnt->mnt_uid == uid; > +} > + > +/* > + * umount is permitted for > + * - sysadmin > + * - mount owner, if not forced umount > + */ > +static bool permit_umount(struct vfsmount *mnt, int flags) > +{ > + if (capable(CAP_SYS_ADMIN)) > + return true; > + > + if (flags & MNT_FORCE) > + return false; > + > + return is_mount_owner(mnt, current->fsuid); > +} > + > /* > * Now umount can handle mount points as well as block devices. > * This is important for filesystems which use unnamed block devices. > @@ -917,7 +938,7 @@ asmlinkage long sys_umount(char __user * > goto dput_and_out; > > retval = -EPERM; > - if (!capable(CAP_SYS_ADMIN)) > + if (!permit_umount(nd.path.mnt, flags)) > goto dput_and_out; > > retval = do_umount(nd.path.mnt, flags); > > -- _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: serue
date: 2008-01-14T21:48:41-00:00

Re: [patch 4/9] unprivileged mounts: propagate error values from clone_mnt
type: item
description: Quoting Miklos Szeredi (miklos@szeredi.hu): > From: Miklos Szeredi <mszeredi@suse.cz> > > Allow clone_mnt() to return errors other than ENOMEM. This will be used for > returning a different error value when the number of user mounts goes over the > limit. > > Fix copy_tree() to return EPERM for unbindable mounts. > > Don't propagate further from dup_mnt_ns() as that copy_tree() can only fail > with -ENOMEM. I see what you're saying, but it just seems like it's bound to be more confusing this way. What's the reason to insist on doing this? To force people to think about it as a form of documentation? Still, > Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> Acked-by: Serge Hallyn <serue@us.ibm.com> > --- > > Index: linux/fs/namespace.c > =================================================================== > --- linux.orig/fs/namespace.c 2008-01-04 13:47:09.000000000 +0100 > +++ linux/fs/namespace.c 2008-01-04 13:47:49.000000000 +0100 > @@ -512,41 +512,42 @@ static struct vfsmount *clone_mnt(struct > struct super_block *sb = old->mnt_sb; > struct vfsmount *mnt = alloc_vfsmnt(old->mnt_devname); > > - if (mnt) { > - mnt->mnt_flags = old->mnt_flags; > - atomic_inc(&sb->s_active); > - mnt->mnt_sb = sb; > - mnt->mnt_root = dget(root); > - mnt->mnt_mountpoint = mnt->mnt_root; > - mnt->mnt_parent = mnt; > - > - /* don't copy the MNT_USER flag */ > - mnt->mnt_flags &= ~MNT_USER; > - if (flag & CL_SETUSER) > - set_mnt_user(mnt); > - > - if (flag & CL_SLAVE) { > - list_add(&mnt->mnt_slave, &old->mnt_slave_list); > - mnt->mnt_master = old; > - CLEAR_MNT_SHARED(mnt); > - } else if (!(flag & CL_PRIVATE)) { > - if ((flag & CL_PROPAGATION) || IS_MNT_SHARED(old)) > - list_add(&mnt->mnt_share, &old->mnt_share); > - if (IS_MNT_SLAVE(old)) > - list_add(&mnt->mnt_slave, &old->mnt_slave); > - mnt->mnt_master = old->mnt_master; > - } > - if (flag & CL_MAKE_SHARED) > - set_mnt_shared(mnt); > + if (!mnt) > + return ERR_PTR(-ENOMEM); > > - /* stick the duplicate mount on the same expiry list >…
creator: serue
date: 2008-01-14T22:23:05-00:00

Re: [patch 6/9] unprivileged mounts: allow unprivileged mounts
type: item
description: Quoting Miklos Szeredi (miklos@szeredi.hu): > From: Miklos Szeredi <mszeredi@suse.cz> > > Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of > this filesystem may not constitute a security problem. > > Since most filesystems haven't been designed with unprivileged mounting in > mind, a thorough audit is needed before setting this flag. > > For "safe" filesystems also allow unprivileged forced unmounting. > > Move subtype handling from do_kern_mount() into do_new_mount(). All > other callers are kernel-internal and do not need subtype support. > > Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> This patch itself doesn't assign FS_SAFE to any filesystems, so presuming that there is such a thing as an fs safe for users to mount, and/or users sign their systems away through a sysctl, this patch in itself appears right. Acked-by: Serge Hallyn <serue@us.ibm.com> > --- > > Index: linux/fs/namespace.c > =================================================================== > --- linux.orig/fs/namespace.c 2008-01-03 21:20:11.000000000 +0100 > +++ linux/fs/namespace.c 2008-01-03 21:21:06.000000000 +0100 > @@ -960,14 +960,16 @@ static bool is_mount_owner(struct vfsmou > /* > * umount is permitted for > * - sysadmin > - * - mount owner, if not forced umount > + * - mount owner > + * o if not forced umount, > + * o if forced umount, and filesystem is "safe" > */ > static bool permit_umount(struct vfsmount *mnt, int flags) > { > if (capable(CAP_SYS_ADMIN)) > return true; > > - if (flags & MNT_FORCE) > + if ((flags & MNT_FORCE) && !(mnt->mnt_sb->s_type->fs_flags & FS_SAFE)) > return false; > > return is_mount_owner(mnt, current->fsuid); > @@ -1025,13 +1027,17 @@ asmlinkage long sys_oldumount(char __use > * - mountpoint is not a symlink > * - mountpoint is in a mount owned by the user > */ > -static bool permit_mount(struct nameidata *nd, int *flags) > +static bool permit_mount(struct nameidata *nd, struct file_system_type *type, > + int *flags) > { > struct i…
creator: serue
date: 2008-01-14T22:58:26-00:00

Re: [patch 8/9] unprivileged mounts: propagation: inherit owner from parent
type: item
description: Quoting Miklos Szeredi (miklos@szeredi.hu): > From: Miklos Szeredi <mszeredi@suse.cz> > > 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. What about nodev? thanks, -serge > > 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 <mszeredi@suse.cz> > --- > > 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->mnt_flags & MNT_USER); > - mnt->mnt_uid = current->fsuid; > + mnt->mnt_uid = owner; > mnt->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->fsuid); > spin_lock(&vfsmount_lock); > nr_user_mounts++; > spin_unlock(&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->mnt_sb; > struct vfsmount *mnt; > @@ -554,7 +554,10 @@ static struct vfsmount *clone_mnt(struct > /* don't copy the MNT_USER flag */ > mnt->mnt_flags &= ~MNT_USER; > if (flag & CL_SETUSER) > - __set_mnt_user(mnt); > + __set_mnt_user(mnt, owner); > + > + if (flag & CL_NOSUID) > + mnt->mnt_flags |= MNT_NOSUID; > > if (flag & CL_SLAVE) { > list_add(&mnt->mnt_slave,…
creator: serue
date: 2008-01-14T23:13:40-00:00

Re: [patch 9/9] unprivileged mounts: add "no submounts" flag
type: item
description: Quoting Miklos Szeredi (miklos@szeredi.hu): > From: Miklos Szeredi <mszeredi@suse.cz> > > Add a new mount flag "nomnt", which denies submounts for the owner. > This would be useful, if we want to support traditional /etc/fstab > based user mounts. > > In this case mount(8) would still have to be suid-root, to check the > mountpoint against the user/users flag in /etc/fstab, but /etc/mtab > would no longer be mandatory for storing the actual owner of the > mount. Ah, I see, so the floppy drive could be mounted as a MNT_NOMNT but MNT_USER mount with mnt_owner set. Makes sense. I'd ask for a better name than 'nomnt', but I can't think of one myself. > Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> Acked-by: Serge Hallyn <serue@us.ibm.com> > --- > > Index: linux/fs/namespace.c > =================================================================== > --- linux.orig/fs/namespace.c 2008-01-04 13:49:52.000000000 +0100 > +++ linux/fs/namespace.c 2008-01-04 13:50:28.000000000 +0100 > @@ -694,6 +694,7 @@ static int show_vfsmnt(struct seq_file * > { MNT_NOATIME, ",noatime" }, > { MNT_NODIRATIME, ",nodiratime" }, > { MNT_RELATIME, ",relatime" }, > + { MNT_NOMNT, ",nomnt" }, > { 0, NULL } > }; > struct proc_fs_info *fs_infop; > @@ -1044,6 +1045,9 @@ static bool permit_mount(struct nameidat > if (S_ISLNK(inode->i_mode)) > return false; > > + if (nd->path.mnt->mnt_flags & MNT_NOMNT) > + return false; > + > if (!is_mount_owner(nd->path.mnt, current->fsuid)) > return false; > > @@ -1888,9 +1892,11 @@ long do_mount(char *dev_name, char *dir_ > mnt_flags |= MNT_RELATIME; > if (flags & MS_RDONLY) > mnt_flags |= MNT_READONLY; > + if (flags & MS_NOMNT) > + mnt_flags |= MNT_NOMNT; > > - flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | > - MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT); > + flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | MS_NOATIME | > + MS_NODIRATIME | MS_RELATIME | MS_KERNMOUNT | MS_NOMNT); > > /* ... and get the mountpoint */ > retval = path_l…
creator: serue
date: 2008-01-14T23:39:10-00:00

Re: [patch 4/9] unprivileged mounts: propagate error values from clone_mnt
type: item
description: > Quoting Miklos Szeredi (miklos@szeredi.hu): > > From: Miklos Szeredi <mszeredi@suse.cz> > > > > Allow clone_mnt() to return errors other than ENOMEM. This will be used for > > returning a different error value when the number of user mounts goes over the > > limit. > > > > Fix copy_tree() to return EPERM for unbindable mounts. > > > > Don't propagate further from dup_mnt_ns() as that copy_tree() can only fail > > with -ENOMEM. > > I see what you're saying, but it just seems like it's bound to be more > confusing this way. Ah yes, this is indeed confusing. Last time dup_mnt_ns() returned a namespace pointer or NULL. But now I see it returns an ERR_PTR(error) instead, which means it's cleaner to just propagate the error value. I'll fix this. Thanks, Miklos _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Miklos Szeredi
date: 2008-01-15T10:15:55-00:00

Re: [patch 8/9] unprivileged mounts: propagation: inherit owner from parent
type: item
description: > Quoting Miklos Szeredi (miklos@szeredi.hu): > > From: Miklos Szeredi <mszeredi@suse.cz> > > > > 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. > > What about nodev? 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 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Miklos Szeredi
date: 2008-01-15T10:39:08-00:00

Re: [patch 9/9] unprivileged mounts: add "no submounts" flag
type: item
description: > Quoting Miklos Szeredi (miklos@szeredi.hu): > > From: Miklos Szeredi <mszeredi@suse.cz> > > > > Add a new mount flag "nomnt", which denies submounts for the owner. > > This would be useful, if we want to support traditional /etc/fstab > > based user mounts. > > > > In this case mount(8) would still have to be suid-root, to check the > > mountpoint against the user/users flag in /etc/fstab, but /etc/mtab > > would no longer be mandatory for storing the actual owner of the > > mount. > > Ah, I see, so the floppy drive could be mounted as a MNT_NOMNT but > MNT_USER mount with mnt_owner set. Makes sense. I'd ask for a better > name than 'nomnt', but I can't think of one myself. Me neither. Thanks for the review, Serge! Miklos _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Miklos Szeredi
date: 2008-01-15T10:41:10-00:00

Re: [patch 9/9] unprivileged mounts: add "no submounts" flag
type: item
description: On Jan 15, 2008 8:41 AM, Miklos Szeredi <miklos@szeredi.hu> wrote: > > Quoting Miklos Szeredi (miklos@szeredi.hu): > > > From: Miklos Szeredi <mszeredi@suse.cz> > > > > > > Add a new mount flag "nomnt", which denies submounts for the owner. > > > This would be useful, if we want to support traditional /etc/fstab > > > based user mounts. > > > > > > In this case mount(8) would still have to be suid-root, to check the > > > mountpoint against the user/users flag in /etc/fstab, but /etc/mtab > > > would no longer be mandatory for storing the actual owner of the > > > mount. > > > > Ah, I see, so the floppy drive could be mounted as a MNT_NOMNT but > > MNT_USER mount with mnt_owner set. Makes sense. I'd ask for a better > > name than 'nomnt', but I can't think of one myself. > > Me neither. > Why not "nosubmnt"? -- A. C. Censi accensi [em] gmail [ponto] com accensi [em] montreal [ponto] com [ponto] br _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: accensi
date: 2008-01-15T10:53:42-00:00

Re: [patch 9/9] unprivileged mounts: add "no submounts" flag
type: item
description: > Why not "nosubmnt"? Why not indeed. Maybe I should try to use my brain sometime. Thanks, Miklos _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Miklos Szeredi
date: 2008-01-15T10:58:27-00:00

Re: [patch 9/9] unprivileged mounts: add "no submounts" flag
type: item
description: Quoting Miklos Szeredi (miklos@szeredi.hu): > > Why not "nosubmnt"? > > Why not indeed. Maybe I should try to use my brain sometime. Well it really should have 'user' or 'unpriv' in the name somewhere. 'nosubmnt' is more confusing than 'nomnt' because it no submounts really sounds like a reasonable thing in itself... But I never win naming arguments, so I accept that I have poor naming judgement :) -serge _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: serue
date: 2008-01-15T13:47:19-00:00

Re: [patch 8/9] unprivileged mounts: propagation: inherit owner from parent
type: item
description: Quoting Miklos Szeredi (miklos@szeredi.hu): > > Quoting Miklos Szeredi (miklos@szeredi.hu): > > > From: Miklos Szeredi <mszeredi@suse.cz> > > > > > > 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. > > > > What about nodev? > > 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. 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 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: serue
date: 2008-01-15T14:21:15-00:00

Re: [patch 8/9] unprivileged mounts: propagation: inherit owner from parent
type: item
description: > > > > 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. > > > > > > What about nodev? > > > > 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. > > 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? Right. > 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? 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 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Miklos Szeredi
date: 2008-01-15T14:37:37-00:00

Re: [patch 8/9] unprivileged mounts: propagation: inherit owner from parent
type: item
description: Quoting Miklos Szeredi (miklos@szeredi.hu): > > > > > 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. > > > > > > > > What about nodev? > > > > > > 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. > > > > 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? > > Right. > > > 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? > > I assume, that the floppy and cdrom are already mounted with > nosuid,nodev. Yeah, of course, what I'm saying is no different whether the upper mount is a user mount or not. You're right. > 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. 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 pur…
creator: serue
date: 2008-01-15T14:59:19-00:00

Re: [patch 9/9] unprivileged mounts: add "no submounts" flag
type: item
description: > > > Why not "nosubmnt"? > > > > Why not indeed. Maybe I should try to use my brain sometime. > > Well it really should have 'user' or 'unpriv' in the name > somewhere. 'nosubmnt' is more confusing than 'nomnt' because > it no submounts really sounds like a reasonable thing in > itself... I slept on it, and I still think 'nosubmnt' might be the best compromise. Obviously the superuser has privileges, that override what is normally allowed, and we don't find it strange when a read-only file is happily being written by root. It may feel wrong in the context of mounts, because we are so used to mounts being privileged-only. Objections? Once this goes in, it will stay the same forever, so now is the time to express any doubts... Thanks, Miklos _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Miklos Szeredi
date: 2008-01-16T09:43:10-00:00