[patch] vfs: fix vfs_rename_dir for FS_RENAME_DOES_D_MOVE filesystems - Kernel

This is a discussion on [patch] vfs: fix vfs_rename_dir for FS_RENAME_DOES_D_MOVE filesystems - Kernel ; From: Miklos Szeredi 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 ...

+ Reply to Thread
Results 1 to 2 of 2

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

  1. [patch] vfs: fix vfs_rename_dir for FS_RENAME_DOES_D_MOVE filesystems

    From: Miklos Szeredi

    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
    CC: Zach Brown
    Signed-off-by: Miklos Szeredi
    ---
    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)
    --
    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/

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

    On Mon, Jul 21, 2008 at 01:41:47PM +0200, Miklos Szeredi wrote:
    > From: Miklos Szeredi
    >
    > 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
    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...
    --
    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/

+ Reply to Thread