ksmbd: fix recursive locking in vfs helpers
authorMarios Makassikis <mmakassikis@freebox.fr>
Sat, 14 Oct 2023 03:48:25 +0000 (12:48 +0900)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Tue, 28 Nov 2023 17:20:01 +0000 (17:20 +0000)
commit 807252f028c59b9a3bac4d62ad84761548c10f11 upstream.

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fs/smb/server/vfs.c

index b5a5e50..9919c07 100644 (file)
@@ -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);