On Mon, 2007-02-05 at 19:20 -0800, Andreas Gruenbacher wrote: > On Monday 05 February 2007 11:02, Christoph Hellwig wrote: > > On Mon, Feb 05, 2007 at 10:58:26AM -0800, Trond Myklebust wrote: > > > On Mon, 2007-02-05 at 18:44 +0000, Christoph Hellwig wrote: > > > > Just FYI: Al was very opposed to the idea of passing the vfsmount to > > > > the vfs_ helpers, so you should discuss this with him. > > > > > > > > Looking at the actual patches I see you're lazy in a lot of places. > > > > Please make sure that when you introduce a vfsmount argument somewhere > > > > that it is _always_ passed and not just when it's conveniant. Yes, > > > > that's more work, but then again if you're not consistant anyone > > > > half-serious will laught at a security model using this infrasturcture. > > > > > > nfsd in particular tends to be a bit lazy about passing around vfsmount > > > info. Forcing it to do so should not be hard since the vfsmount is > > > already cached in the "struct export" (which can be found using the > > > filehandle). It will take a bit of re-engineering in order to pass that > > > information around inside the nfsd code, though. > > > > I actually have a patch to fix that. It's part of a bigger series > > that's not quite ready, but I hope to finish all of it this month. > > It's actually not hard to "fix", and nfsd would look a little less weird. But > what would this add, what do pathnames mean in the context of nfsd, and would > nfsd actually become less weird? > > On the wire, nfs transmits file handles, not filenames. The file handle > usually contains the inode numbers of the file and the containing directory. > > The code in fs/exportfs/expfs.c:find_exported_dentry() shows that fh_verify() > should return a dentry that may be connected or not, depending on the export > options and whether it's a file or directory. Together with the vfsmount from > the svc_export, we could compute a pathname. > > But there is no way to tell different hardlinks to the same inode in the same > directory from each other (both the file and directory inode are the same), > and depending on the export options, we may or may not be able to distinguish > different hardlinks across directories. Who cares? There is no way to export a partial directory, and in any case the subtree_check crap is borken beyond repair (see cross-directory renames which lead to actual changes to the filehandle - broken, broken, broken!!!!). > If the nohide or crossmnt export options are used, we might run into similar > aliasing issues with mounts (I'm not sure about this). Wrong. Those create new export points since you are crossing into a different filesystem. > So we could compute pathnames, but they wouldn't be very meaningful. Instead > of going that way, it seems more reasonable to not do pathname based access > control for the kernel kernel nfsd at all. Passing NULL as the vfsmounts does > that. With a properly configured nfsd, the areas that remote users could > access would still be well confined. Huh? Even if you don't pass in a vfsmount, you _still_ need to pass in a super_block. Inodes are only unique per filesystem. In fact, on an ideal NFS export, there is no ambiguity between the two (see above comment about subtree_check) since the entire filesystem will be exported. > The focus with this whole pathname based security stuff really is to be able > to better confine local processes. The kernel nfsd is a kernel thread, and as > such, confining it from a security point of view cannot really work, anyway. > > > > Note also that it might be nice to enforce the vfsmount argument by > > > replacing the existing dentry parameters with a struct path instead of > > > adding an extra reference to the vfsmount to existing functions. > > > > That definitly sounds like a good idea, independent of whether we want > > to pass the vfsmount in more places or not. > > Note that you can still pass a NULL vfsmount in a struct path. ...but we will have Dick Cheney track you down and shoot you if you do. The point is that you only have to check _one_ argument (the initialisation of the struct path) instead of having a check for every function argument in the vfs. Trond - 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
