[patch] vfs: fix vfs_rename_dir for FS_RENAME_DOES_D_MOVE filesystems

[patch] vfs: fix vfs_rename_dir for FS_RENAME_DOES_D_MOVE filesystems

[Miklos Szeredi]

From: Miklos Szeredi <mszeredi@suse.cz>

vfs_rename_dir() doesn't properly account for filesystems with
FS_RENAME_DOES_D_MOVE.  If new_dentry has a target inode attached, it
unhashes the new_dentry prior to the rename() iop and rehashes it
after, but doesn't account for the possibility that rename() may have
swapped {old,new}_dentry.  For FS_RENAME_DOES_D_MOVE filesystems, it
rehashes new_dentry (now the old renamed-from name, which d_move()
expected to go away), such that a subsequent lookup will find it.

This was caught by the recently posted POSIX fstest suite, rename/10.t 
test 62 (and others) on ceph.

Fix by not rehashing the new dentry.  Rehashing would only make sense
if the rename failed (which should happen extremely rarely), but we
cannot handle that case correctly 100% of the time anyway, so...

Reported-by: Sage Weil <sage@newdream.net>
CC: Zach Brown <zach.brown@oracle.com>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/namei.c |    2 --
 1 file changed, 2 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-07-21 09:46:07.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-07-21 11:56:01.000000000 +0200
@@ -2574,8 +2574,6 @@ static int vfs_rename_dir(struct inode *
 		if (!error)
 			target->i_flags |= S_DEAD;
 		mutex_unlock(&target->i_mutex);
-		if (d_unhashed(new_dentry))
-			d_rehash(new_dentry);
 		dput(new_dentry);
 	}
 	if (!error)

Re: [patch] vfs: fix vfs_rename_dir for FS_RENAME_DOES_D_MOVE filesystems

[Al Viro]

On Mon, Jul 21, 2008 at 01:41:47PM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> vfs_rename_dir() doesn't properly account for filesystems with
> FS_RENAME_DOES_D_MOVE.  If new_dentry has a target inode attached, it
> unhashes the new_dentry prior to the rename() iop and rehashes it
> after, but doesn't account for the possibility that rename() may have
> swapped {old,new}_dentry.  For FS_RENAME_DOES_D_MOVE filesystems, it
> rehashes new_dentry (now the old renamed-from name, which d_move()
> expected to go away), such that a subsequent lookup will find it.
> 
> This was caught by the recently posted POSIX fstest suite, rename/10.t 
> test 62 (and others) on ceph.
> 
> Fix by not rehashing the new dentry.  Rehashing would only make sense
> if the rename failed (which should happen extremely rarely), but we
> cannot handle that case correctly 100% of the time anyway, so...

Lovely.  AFAICS, that's a fallout from
commit 349457ccf2592c14bdf13b6706170ae2e94931b1
Author: Mark Fasheh <mark.fasheh@oracle.com>
Date:   Fri Sep 8 14:22:21 2006 -0700

    [PATCH] Allow file systems to manually d_move() inside of ->rename()

that had allowed that crap for directories.  Note that d_rehash() used
to be needed (d_move() would unhash the source otherwise) and d_move()
used to be unconditional until the changeset above.

It's _probably_ OK now, but I'd really like to think about NFS behaviour.
There are subtle traps in that area.

BTW, failing rename() is trivial - just have a non-empty target...