Skip to content

Commit

Permalink
ksmbd: fix recursive locking in vfs helpers
Browse files Browse the repository at this point in the history
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: [email protected]
Fixes: 40b268d384a2 ("ksmbd: add mnt_want_write to ksmbd vfs functions")
Signed-off-by: Marios Makassikis <[email protected]>
Signed-off-by: Namjae Jeon <[email protected]>
  • Loading branch information
Marios Makassikis authored and namjaejeon committed Oct 14, 2023
1 parent 6520b8b commit b7e1fb4
Showing 1 changed file with 3 additions and 20 deletions.
23 changes: 3 additions & 20 deletions vfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -373,10 +373,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;
#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 12, 0)
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0)
Expand All @@ -395,9 +391,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;
}
Expand Down Expand Up @@ -432,10 +426,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;

#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0)
idmap = mnt_idmap(path.mnt);
#else
Expand Down Expand Up @@ -467,21 +457,19 @@ int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode)
#endif
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);
Expand Down Expand Up @@ -1277,10 +1265,6 @@ int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname,
goto out3;
}

err = mnt_want_write(newpath.mnt);
if (err)
goto out3;

#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 12, 0)
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0)
err = vfs_link(oldpath.dentry, mnt_idmap(newpath.mnt),
Expand All @@ -1296,7 +1280,6 @@ int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname,
#endif
if (err)
ksmbd_debug(VFS, "vfs_link failed err %d\n", err);
mnt_drop_write(newpath.mnt);

out3:
done_path_create(&newpath, dentry);
Expand Down

0 comments on commit b7e1fb4

Please sign in to comment.