> > > > > 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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
