nfsfh.c:nfsd_findparent lookup("..") failure fix in 2.4.4 - xfs related?

nfsfh.c:nfsd_findparent lookup("..") failure fix in 2.4.4 - xfs related?

[Peter Wächtler]

The following diff was made in 2.4.4.
 
diff -u --recursive --new-file v2.4.4/linux/fs/nfsd/nfsfh.c linux/fs/nfsd/nfsfh.c
--- v2.4.4/linux/fs/nfsd/nfsfh.c        Fri Feb  9 11:29:44 2001
+++ linux/fs/nfsd/nfsfh.c       Sat May 19 17:47:55 2001
@@ -244,6 +245,11 @@
         */
        pdentry = child->d_inode->i_op->lookup(child->d_inode, tdentry);
        d_drop(tdentry); /* we never want ".." hashed */
+       if (!pdentry && tdentry->d_inode == NULL) {
+               /* File system cannot find ".." ... sad but possible */
+               dput(tdentry);
+               pdentry = ERR_PTR(-EINVAL);
+       }


Umh. How is it possible to have a valid dentry which has no parent?
Even "/.." is linked to "/."

Is this xfs related? At least it was triggered on 2.4.3-xfs with
exported xfs filesystems.

Re: nfsfh.c:nfsd_findparent lookup("..") failure fix in 2.4.4 - xfs related?

[Trond Myklebust]

>>>>> " " == Peter Wächtler <pwaechtler@loewe-komp.de> writes:

     > The following diff was made in 2.4.4.  diff -u --recursive
     > --new-file v2.4.4/linux/fs/nfsd/nfsfh.c linux/fs/nfsd/nfsfh.c
     > --- v2.4.4/linux/fs/nfsd/nfsfh.c Fri Feb 9 11:29:44 2001
     > +++ linux/fs/nfsd/nfsfh.c Sat May 19 17:47:55 2001
     > @@ -244,6 +245,11 @@
     >          */
     >         pdentry = child->d_inode->i_op->lookup(child->d_inode,
     >         tdentry); d_drop(tdentry); /* we never want ".." hashed
     >         */
     > + if (!pdentry && tdentry->d_inode == NULL) {
     > + /* File system cannot find ".." ... sad but possible */
     > + dput(tdentry);
     > + pdentry = ERR_PTR(-EINVAL);
     > + }


     > Umh. How is it possible to have a valid dentry which has no
     > parent?  Even "/.." is linked to "/."

Who said these are *valid* dentries?

There's no way you can fit full path information into an NFS
filehandle, so when the client passes such an object to nfsd, the
latter sometimes has to make so-called 'disconnected' dentries. These
are dentries which contain no path information, and are basically just
providing a dentry alias for the inode.

The code you are looking at attempts to call the filesystem in order
to lookup a file named '..' from just such a disconnected dentry. The
purpose being to actually recreate the path, and hence convert the
dentry into a valid one...

Cheers,
  Trond

Re: nfsfh.c:nfsd_findparent lookup("..") failure fix in 2.4.4 - xfs related?

[Neil Brown]

On Thursday October 18, pwaechtler@loewe-komp.de wrote:
> The following diff was made in 2.4.4.
>  
> diff -u --recursive --new-file v2.4.4/linux/fs/nfsd/nfsfh.c linux/fs/nfsd/nfsfh.c
> --- v2.4.4/linux/fs/nfsd/nfsfh.c        Fri Feb  9 11:29:44 2001
> +++ linux/fs/nfsd/nfsfh.c       Sat May 19 17:47:55 2001
> @@ -244,6 +245,11 @@
>          */
>         pdentry = child->d_inode->i_op->lookup(child->d_inode, tdentry);
>         d_drop(tdentry); /* we never want ".." hashed */
> +       if (!pdentry && tdentry->d_inode == NULL) {
> +               /* File system cannot find ".." ... sad but possible */
> +               dput(tdentry);
> +               pdentry = ERR_PTR(-EINVAL);
> +       }
> 
> 
> Umh. How is it possible to have a valid dentry which has no parent?
> Even "/.." is linked to "/."

Well.  The filesystem could be corrupted, and there may not be a ".."
entry in the directory.  Or the filesystem code might  be broken in
some way.  Or just maying it doesn't support "..".
No other part of the linux kernel ever asks a filesystem to do a
lookup of "..".  All other accesses are caught by the VFS layer and
converted to dentry->d_parent.
So it would not be too surprising if some filesystems didn't handle
".." properly.
isofs, for example, doesn't support ".." at all.  I think the code
appears to try, but is simply wrong.

> 
> Is this xfs related? At least it was triggered on 2.4.3-xfs with
> exported xfs filesystems.

The code was put in because problems were reported with xfs.  How ever
it is correct to have this code anyway to protect nfsd from
corrupt, incomplete, or broken filesystems.

NeilBrown


> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/