diff options
author | Marios Makassikis <mmakassikis@freebox.fr> | 2023-10-14 12:48:25 +0900 |
---|---|---|
committer | Steve French <stfrench@microsoft.com> | 2023-10-22 19:06:27 -0500 |
commit | 807252f028c59b9a3bac4d62ad84761548c10f11 (patch) | |
tree | 49f027d9c279230e648df87c9c28fb9278ff5040 /fs/smb | |
parent | 3354db668808d5b6d7c5e0cb19ff4c9da4bb5e58 (diff) |
ksmbd: fix recursive locking in vfs helpers
Running smb2.rename test from Samba smbtorture suite against a kernel built
with lockdep triggers a "possible recursive locking detected" warning.
This is because mnt_want_write() is called twice with no mnt_drop_write()
in between:
-> ksmbd_vfs_mkdir()
-> ksmbd_vfs_kern_path_create()
-> kern_path_create()
-> filename_create()
-> mnt_want_write()
-> mnt_want_write()
Fix this by removing the mnt_want_write/mnt_drop_write calls from vfs
helpers that call kern_path_create().
Full lockdep trace below:
============================================
WARNING: possible recursive locking detected
6.6.0-rc5 #775 Not tainted
--------------------------------------------
kworker/1:1/32 is trying to acquire lock:
ffff888005ac83f8 (sb_writers#5){.+.+}-{0:0}, at: ksmbd_vfs_mkdir+0xe1/0x410
but task is already holding lock:
ffff888005ac83f8 (sb_writers#5){.+.+}-{0:0}, at: filename_create+0xb6/0x260
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(sb_writers#5);
lock(sb_writers#5);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by kworker/1:1/32:
#0: ffff8880064e4138 ((wq_completion)ksmbd-io){+.+.}-{0:0}, at: process_one_work+0x40e/0x980
#1: ffff888005b0fdd0 ((work_completion)(&work->work)){+.+.}-{0:0}, at: process_one_work+0x40e/0x980
#2: ffff888005ac83f8 (sb_writers#5){.+.+}-{0:0}, at: filename_create+0xb6/0x260
#3: ffff8880057ce760 (&type->i_mutex_dir_key#3/1){+.+.}-{3:3}, at: filename_create+0x123/0x260
Cc: stable@vger.kernel.org
Fixes: 40b268d384a2 ("ksmbd: add mnt_want_write to ksmbd vfs functions")
Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Diffstat (limited to 'fs/smb')
-rw-r--r-- | fs/smb/server/vfs.c | 23 |
1 files changed, 3 insertions, 20 deletions
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index 2e37939a8ba0..1053127f71ad 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -173,10 +173,6 @@ int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode) return err; } - err = mnt_want_write(path.mnt); - if (err) - goto out_err; - mode |= S_IFREG; err = vfs_create(mnt_idmap(path.mnt), d_inode(path.dentry), dentry, mode, true); @@ -186,9 +182,7 @@ int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode) } else { pr_err("File(%s): creation failed (err:%d)\n", name, err); } - mnt_drop_write(path.mnt); -out_err: done_path_create(&path, dentry); return err; } @@ -219,10 +213,6 @@ int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode) return err; } - err = mnt_want_write(path.mnt); - if (err) - goto out_err2; - idmap = mnt_idmap(path.mnt); mode |= S_IFDIR; err = vfs_mkdir(idmap, d_inode(path.dentry), dentry, mode); @@ -233,21 +223,19 @@ int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode) dentry->d_name.len); if (IS_ERR(d)) { err = PTR_ERR(d); - goto out_err1; + goto out_err; } if (unlikely(d_is_negative(d))) { dput(d); err = -ENOENT; - goto out_err1; + goto out_err; } ksmbd_vfs_inherit_owner(work, d_inode(path.dentry), d_inode(d)); dput(d); } -out_err1: - mnt_drop_write(path.mnt); -out_err2: +out_err: done_path_create(&path, dentry); if (err) pr_err("mkdir(%s): creation failed (err:%d)\n", name, err); @@ -665,16 +653,11 @@ int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname, goto out3; } - err = mnt_want_write(newpath.mnt); - if (err) - goto out3; - err = vfs_link(oldpath.dentry, mnt_idmap(newpath.mnt), d_inode(newpath.dentry), dentry, NULL); if (err) ksmbd_debug(VFS, "vfs_link failed err %d\n", err); - mnt_drop_write(newpath.mnt); out3: done_path_create(&newpath, dentry); |