| Message ID | CAH2r5mt0=WRC2SgG6UZmZ32PbjZrcK4N_sZ9=WcSEar1utTmCw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [CIFS] Use FS_RENAME_DOES_D_MOVE to minimize races in rename | expand |
On Wed, Feb 26, 2020 at 8:37 AM Steve French <smfrench@gmail.com> wrote: > What sort of rename races? Is that a real/reproducible problem? > Should be safer to do the dentry move immediately after the rename > rather than later. > Looking at what "later" means, you moved d_move() before cifs_put_tlink(tlink); and shrink_dcache_parent(new_dentry); detach_mounts(new_dentry); I suppose cifs_put_tlink() is not the issue. It makes me wonder about shrink_dcache_parent()/detach_mounts() - they happen for some fs before d_move() and for some fs after d_move() I think it kind of makes sense to have them moved after dentry is unhashed anyway? (that is after d_move()). Thanks, Amir.
From c1bc75178780665716ad14b8c40a8d8e58d0f3e3 Mon Sep 17 00:00:00 2001 From: Steve French <stfrench@microsoft.com> Date: Tue, 25 Feb 2020 18:08:54 -0600 Subject: [PATCH] cifs: do d_move in rename It is safer to do the d_move closer to the rename to avoid races. Signed-off-by: Steve French <stfrench@microsoft.com> --- fs/cifs/cifsfs.c | 4 ++-- fs/cifs/inode.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index fa77fe5258b0..94e3ed4850b5 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -1018,7 +1018,7 @@ struct file_system_type cifs_fs_type = { .name = "cifs", .mount = cifs_do_mount, .kill_sb = cifs_kill_sb, - /* .fs_flags */ + .fs_flags = FS_RENAME_DOES_D_MOVE, }; MODULE_ALIAS_FS("cifs"); @@ -1027,7 +1027,7 @@ static struct file_system_type smb3_fs_type = { .name = "smb3", .mount = smb3_do_mount, .kill_sb = cifs_kill_sb, - /* .fs_flags */ + .fs_flags = FS_RENAME_DOES_D_MOVE, }; MODULE_ALIAS_FS("smb3"); MODULE_ALIAS("smb3"); diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 6543465595f6..7bb7ff110dc6 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -1835,6 +1835,8 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry, CIFSSMBClose(xid, tcon, fid.netfid); } do_rename_exit: + if (rc == 0) + d_move(from_dentry, to_dentry); cifs_put_tlink(tlink); return rc; } -- 2.20.1