[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <E1Jcnm7-00020F-7o@pomaz-ex.szeredi.hu>
Date: Fri, 21 Mar 2008 21:23:39 +0100
From: Miklos Szeredi <miklos@...redi.hu>
To: viro@...IV.linux.org.uk
CC: miklos@...redi.hu, haveblue@...ibm.com,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
neilb@...e.de, akpm@...ux-foundation.org, hch@...radead.org,
linux-security-module@...r.kernel.org, jmorris@...ei.org
Subject: Re: r-o bind in nfsd
> > > > > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists