[linux-cifs-client] Re: [PATCH] cifs: when renaming don't try to unlink negative dentry

[Jeff Layton]

On Fri, 17 Apr 2009 21:31:50 -0500
Steve French <smfrench at gmail.com> wrote:

> On Fri, Apr 17, 2009 at 6:02 PM, Jeff Layton <jlayton at redhat.com> wrote:
> > On Fri, 17 Apr 2009 16:16:23 -0500
> > Steve French <smfrench at gmail.com> wrote:
> >
> >> I merged this, adding the CC: stable, but think we need to also
> >> consistently check for inode == NULL in cifs_unlink (we only check in
> >> two branches now)
> >>
> >> Any objections if I also add the following check:
> >>
> >
> > I think it would be preferable to just check once for inode==NULL in
> > cifs_unlink near the top and BUG() if it is. Note that vfs_unlink takes
> > the i_mutex on this before calling the .unlink inode op, so we're
> > guaranteed that the d_inode won't be NULL from that codepath.
> >
> > I think if we get an unlink on a negative dentry then we should
> > probably consider that a BUG().
> >
> > Other .unlink ops also seem to assume that you can't call .unlink with
> > a negative dentry.
> 
> I am more worried about internal calls to unlink from cifs slipping
> through with inode null.  If we check in one branch we should check in
> the other, or as you suggest move it to the top
> 

Yep, internal callers are my worry too. That's why I think a BUG() is
appropriate to help catch this when it occurs. Maybe something like
this patch?

-- 
Jeff Layton <jlayton at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cifs-don-t-allow-cifs_unlink-to-be-called-on-negati.patch
Type: text/x-patch
Size: 1897 bytes
Desc: not available
Url : http://lists.samba.org/archive/linux-cifs-client/attachments/20090418/916ed513/0001-cifs-don-t-allow-cifs_unlink-to-be-called-on-negati.bin