Re: [PATCH] exportfs: fix incorrect EACCES in reconnect_path()

["J. Bruce Fields" <bfields@fieldses.org>]

On Tue, May 06, 2008 at 10:35:46AM +1000, Neil Brown wrote:
> On Monday May 5, bfields@fieldses.org wrote:
> > On Mon, May 05, 2008 at 09:22:49AM +1000, Neil Brown wrote:
> > > 
> > > Now it could be argued that this permission test is really a dumb idea
> > > that buys nothing and costs much.  And if you were to queue a patch to
> > > get rid of it, I doubt you would get any objections .... certainly not
> > > from me :-)
> > 
> > Dumb idea or not, it looks like it's explicitly documented in
> > exports(5):
> > 
> > 	" subtree  checking  is  also  used to make sure that files
> > 	inside directories to which only root has access can only be
> > 	accessed if  the  filesystem is exported with no_root_squash
> > 	(see below), even if the file itself allows more general
> > 	access."
> > 
> > So as much as I'd like to I'm not comfortable silently turning off that
> > check.
> 
> Ack.
> 
> > 
> > I suppose we could choose to acquire those capabilities only in the
> > no_subtree_check case.
> 
> If only it were that easy ;-)
> 
> reconnect_path potentially requires both 'r' and 'x' permission on
> parent directories.  'r' to be able to read the directory to find the
> name of the object being reconnected, and 'x' to do the lookup which
> effects the reconnect.
> 
> To fix the current bug properly, reconnect_path still needs to bypass
> normal permission checks even when subtree_check is in effect, so it
> can be sure of getting read permission on the parent directory.

OK, but why not just forget the subtree_check case?  It would be just
another item on the "reasons not to use subtree_check" list.

If a fix for the subtree checking case were easy (or if someone else had
the time to do a very careful job of it), then fine, but maybe we should
just fix the easy case and leave the subtree checking as is for now.

--b.

> 
> There is another way .... but it would need careful consideration.
> 
> While the dentry returned by exportfs_decode_fh (for a directory) must
> be connected in the dcache tree, it does *not* need to have a correct
> name.  All that is needed is that d_parent is correct  (this is used,
> as mentioned before, to correctly lock directory renames).
> 
> We can leave the dentry unhashed but with a correct d_parent pointer.
> If the directory is ever access by name, d_slice_alias will be called
> and this will update the name in the dentry to be correct.
> 
> We could then get rid of exportfs_get_name and the call to
> lookup_one_len, and add some dcache magic after the ->get_parent call
> to make 'pd' an anonymous child of 'ppd'.
> 
> Some matching changes to d_splice_alias should finish the task.
> 
> Does this seem sane to anyone else?  Is it worth a try?
> 
> 
> NeilBrown