Re: [PATCH] CIFS: Fix NT_STATUS_ACCESS_DENIED for mounts with prefixpath option

[Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Wed, 31 Aug 2011 23:00:45 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:

> 2011/8/31 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> > On Wed, 31 Aug 2011 22:27:25 +0400
> > Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> >
> >> 2011/8/31 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> >> > I think you need to make sure you're holding the i_mutex of the parent
> >> > for this. Most of these sorts of dentry operations require it.
> >> >
> >>
> >> In the case of this patch, we can have a negative parent - so, can't
> >> hold i_mutex here. It seems to me that we need extra cifs mutex for
> >> dealing this it.
> >>
> >
> > In the event of a positive parent, how do you intend to ensure that the
> > dentry is not instantiated via other codepaths (lookup or readdir?)
> 
> If dentry is negative we don't have it reachable from userspace (cifs
> mount point always starts with a positive one) - no lookups and
> readdirs from here. If I am mistaken, point me to it, please.
> 

If the parent dentry is negative, you mean. Ok, Good point...

Ok, so you're proposing to wrap this check in a big mutex to prevent
this race when the parent dentry is negative. In the event that it's
positive though, you'll need to make sure that you take the parent's
i_mutex.

I don't have a great feel for this sort of dcache trickery, but this
whole scheme sounds like it's going to break a lot of assumptions about
how the dcache works. In particular having negative dentries with
children sounds very problematic.

Would it be better to simply make the initial dentry in this case be a
disconnected or root dentry (see DCACHE_DISCONNECTED), and then later
"graft" it into a larger tree if the parent comes to life?

> > It's not clear to me though that this is really a problem though in any
> > case. In the event that you "re-instantiate" the dentry, the old inode
> > will just end up being put (and likely eventually freed). Sure, the
> > inode number might change, but that's life with noserverino.
> >
> > --
> > Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >
> 


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>