Re: unprivileged mounts git tree

Serge E. Hallyn
2008-09-04T17:49:39Z

Quoting Miklos Szeredi (miklos@szeredi.hu):
> On Thu, 4 Sep 2008, Serge E. Hallyn wrote:
> > Quoting Miklos Szeredi (miklos@szeredi.hu):
> > > On Thu, 04 Sep 2008, Miklos Szeredi wrote:
> > > > On Thu, 4 Sep 2008, Serge E. Hallyn wrote:
> > > > > Are you going to revert the change forcing CL_SLAVE for
> > > > > !capable(CAP_SYS_ADMIN)? I don't think we want that - I think that
> > > > > *within* a set of user mounts, propagation should be safe, right?
> > > > >
> > > > > Will you be able to do this soon? If not, should we just do the part
> > > > > returning -EPERM when turning a shared mount into a user mount?
> > > >
> > > > OK, let's do that first and the tricky part (propagation vs. user
> > > > mounts) later. Will push after I've tested it.
> > >
> > > Here it is:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git unprivileged-mounts
> >
> > but you're still doing
> >
> > if (IS_MNT_SHARED(old_nd.path.mnt) && !capable(CAP_SYS_ADMIN))
> > goto out;
> >
> > shouldn't it be something like
> >
> > if (IS_MNT_SHARED(old_nd.path.mnt) && (old_nd.path.mnt & MNT_USER))
> > goto out;
> >
> > ?
>
> Why would that be an error? There's no real security gain to be had
> from restricting a privileged user, but could cause a lot of
> annoyance. If we think this is dangerous, then protection should be
> built into mount(8) with an option to override. But not into the
> kernel, IMO.

We disagree on that. But can we agree that the check you added is wrong?
There is no reason why a user mount should not be able to do shared
mounts, is there? So should the check above just go away then?

-serge