Re: Rename dir on server can cause client to get ESTALE - this time with PATCH

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 4492 bytes --]

On Mon, 14 Nov 2011 13:19:29 +1100 NeilBrown <neilb@suse.de> wrote:

> 
> hi,
>   I've run into another issue that seems to related to FS_REVAL_DOT.
> 
> The script below makes the details precise, but the essence is that if I 'cd'
> into a directory on the client, then rename it on the server, then it is
> possible that the client will start getting ESTALE when accessing '.' - even
> though the directory still exists.
> 
> The ESTALE is generated because nfs_lookup_revalidate fails on the dentry, so 
> complete_walk (in fs/namei.c) gets failure from d_revalidate() and so sets the
> status to -ESTALE.
> 
> nfs_lookup_revalidate fails because when it repeats the lookup it sees a
> different directory (as you will see the script creates a new directory with
> the old name).
> 
> I think it only makes sense to do a ->lookup revalidate of the dentry at the
> end of the path when there was a real non '.' or '..' name leading to the
> dentry.  If we were just looking up '.', we want to revalidate the inode, but
> not the dentry.
> 
> Unfortunately I cannot see how that distinction could be introduced into the
> current path-walk code.
> 
> Any ideas?
> 
> Thanks,
> NeilBrown
> 
> 
> SERVER=eli     # name of server.  ssh access required.
> DIR=/home      # directory on server to mount
> MPOINT=/mnt    # location on client to mount it.
> TMP=/neilb/tmp # path to scratch area in $DIR
> 
> sudo umount $MPOINT
> sudo mount -o vers=3 $SERVER:$DIR $MPOINT
> 
> cd /
> ssh $SERVER "rm -r $DIR$TMP/*dir*"
> ssh $SERVER "mkdir $DIR$TMP/adir"
> while [ ! -d $MPOINT$TMP/adir ];
> do echo -n . ; sleep 2;
> done
> cd $MPOINT$TMP/adir || exit
> echo "Entered directory"
> ls -la > /dev/null
> ssh $SERVER "cd $DIR$TMP; mv adir adir.moved"
> echo "Moved directory on server"
> ls -la > /dev/null
> echo -n "Waiting for move to be visible on client"
> while ls -la $MPOINT$TMP/adir >/dev/null 2>&1
> do echo -n . 
>    sleep 3
>    (cd / ; ssh $SERVER "cd $DIR$TMP; mkdir bdir ; rmdir bdir" )
> done
> echo
> echo "Make replacement directory on server"
> (cd / ; ssh $SERVER "cd $DIR$TMP; mkdir adir")
> ls -la $MPOINT$TMP/adir
> ls -la
> 


.. but answer came there none....

I've looked some more at the code and now would like to propose a patch.
This fixes it for me and feels right.

Opinions?

Thanks,
NeilBrown

From 7abb2d77b4c8d8ca340e372447467d8a47241f83 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Wed, 30 Nov 2011 18:35:13 +1100
Subject: [PATCH] nfs - handle d_revalidate of 'dot' correctly.

When d_revalidate is called on a dentry because FS_REVAL_DOT is set
it isn't really appropriate to revalidate the name.

If the path was simply ".", then the current-working-directory could
have been renamed on the server and should still be accessible as "."
even if it has a new name.

If the path was "/some/long/path/.", then the final component ("path" in
this case) has already been revalidated and there is no particular
need to do it again.

If we change nd->last_type to refer to "the last component looked at"
rather than just "the last component", then these cases can be
detected by "nd->last_type != LAST_NORM".

Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/namei.c   |    2 +-
 fs/nfs/dir.c |    9 +++++++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 5008f01..6a720f7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1434,6 +1434,7 @@ static int link_path_walk(const char *name, struct nameidata *nd)
 			}
 		}
 
+		nd->last_type = type;
 		/* remove trailing slashes? */
 		if (!c)
 			goto last_component;
@@ -1458,7 +1459,6 @@ static int link_path_walk(const char *name, struct nameidata *nd)
 
 last_component:
 		nd->last = this;
-		nd->last_type = type;
 		return 0;
 	}
 	terminate_walk(nd);
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ac28990..f62827a 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1137,6 +1137,15 @@ static int nfs_lookup_revalidate(struct dentry *dentry, struct nameidata *nd)
 	if (NFS_STALE(inode))
 		goto out_bad;
 
+	if (nd->last_type != LAST_NORM) {
+		/* name not relevant, just inode */
+		error = nfs_revalidate_inode(NFS_SERVER(inode), inode);
+		if (error)
+			goto out_bad;
+		else
+			goto out_valid;
+	}
+
 	error = -ENOMEM;
 	fhandle = nfs_alloc_fhandle();
 	fattr = nfs_alloc_fattr();
-- 
1.7.7.3





[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]