lkml.org 
[lkml]   [2008]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: r-o bind in nfsd
From
Date
> > > > > And having the vfsmount available within vfs_...() functions means,
> > > > > that the mnt_want_write() check can be moved inside, which means that
> > > > > callers get simpler and less likely to be buggy. Those are all
> > > > > advantages IMO, regardless of any security module issues.
> > > >
> > > > Or we can introduce another set of exported functions (path_mkdir(),
> > > > ...), and leave vfs_...() alone. And then the only question is if
> > > > LSM's can live with ordering change.
> > >
> > > I really don't see the point of new helpers; especially since one doesn't
> > > have to _have_ vfsmount to use the old ones and since we don't have a lot
> > > of users of each of those to start with.
> >
> > Traditionally we have syscalls, and nfsd. Both of them want the
> > security checks, and I think nfsd wants the read-only mount checking
> > as well, but I'm not entirely sure. Maybe we can handle that by just
> > making nfsd acquire a write-ref on the mount and keep it while it's
> > exported.
> >
> > Then there's ecryptfs and unionfs, which probably need neither, but it
> > wouldn't hurt to do them anyway.
> >
> > Still, even if there are only two callers, then moving stuff to up
> > doesn't make any sense. Passing down a struct path is free for the
> > syscall case, it doesn't consume any stack space or extra CPU. Do
> > please tell, why would that be such a bad thing?
>
> Because we'd been that way before; see the shitpiles around ->lookup()
> getting nameidata, etc. You'll end up with some callers passing NULL
> as ->mnt since they don't have anything better to pass, some stuff
> called *from* the damn thing caring to check for ->mnt being NULL,
> some stuff not caring about what ->mnt is at all and some assuming
> that it's not NULL. Which will lead to exploding combinations that
> won't be noticed until somebody steps into such config.

Right, we do want to prevent that happening.

And for example moving read-only mount checks inside vfs_...() would
ensure that.

> As for the vfsmount-dependent checks (and any kind of MAC, while we are
> at it)... They belong to callers, exactly because different callers may
> want different (amount of) checks.

And we end up random callers forgetting some of the checks, like we
have now with nfsd. Not good at all.

I think it's still a lot better all the checks are always done, even
if not strictly necessary for a certain caller, than if the caller has
to make sure the necessary ones do get done.

Assuming of course, that all valid users _do_ have the vfsmount
available, which I think is true. If you have a counterexample,
please let us know.

If not all (but most) callers have the vfsmount available, then a new
helper makes sense.

If there was only one caller which needed a certain check, then moving
that into the caller would be the right thing of course. But that's
not the case here.

Miklos


\
 
 \ /
  Last update: 2008-03-21 21:27    [W:0.046 / U:3.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site