Re: NFS related Oops in 2.4.[39]-xfs

Peter Wächtler
2001-10-17T13:39:29Z

Steve Lord wrote:
>
> Where did you get your kernel (the 2.4.9 version that is) this problem
> sounds familiar, but I am pretty sure we fixed this case in XFS somewhere
> between 2.4.3 and 2.4.9.
>

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);
+ }

But it would not prevent the code path 2.4.3-xfs hit.
pdentry is !=NULL and tdentry->d_inode is always NULL after d_alloc():611

Does xfs' child->d_inode->i_op->lookup(child->d_inode, tdentry);
change the contents of tdentry if the lookup("..") succeeds?

Then I suggest:

! if (!pdentry || tdentry->d_inode == NULL) {

What do you think?
BTW, when does a lookup("..") fail? Even in "/", lookup("..") returns "."

> > struct dentry *nfsd_findparent(struct dentry *child)
> > {
> > struct dentry *tdentry, *pdentry;
> > tdentry = d_alloc(child, &(const struct qstr) {"..", 2, 0});
> > if (!tdentry)
> > return ERR_PTR(-ENOMEM);
> >
> > /* I'm going to assume that if the returned dentry is different, then
> > * it is well connected. But nobody returns different dentrys do they?
> > */
> > 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);
+ }
> > if (!pdentry) {
> > /* I don't want to return a ".." dentry.
> > * I would prefer to return an unconnected "IS_ROOT" dentry,
> > * though a properly connected dentry is even better
> > */
> > /* if first or last of alias list is not tdentry, use that
> > * else make a root dentry
> > */
> > struct list_head *aliases = &tdentry->d_inode->i_dentry;
> > spin_lock(&dcache_lock);
> > if (aliases->next != aliases) { <=========== CRASH
> > pdentry = list_entry(aliases->next, struct dentry, d_