Lockdep reports a circular dependency between card_mutex and misc_mtx:
misc_open() holds misc_mtx and calls ->open(): misc_mtx -> nosy_open()
add_card()/remove_card() hold card_mutex and (de)register the miscdev: card_mutex -> misc_register()/misc_deregister() -> misc_mtx
nosy_open() only used card_mutex to map the minor to the pcilynx instance by scanning card_list. However, misc_open() already sets file->private_data to the struct miscdevice of the opened minor, so we can obtain the pcilynx pointer via container_of() and take a kref, without taking card_mutex in the open path.
This removes the misc_mtx -> card_mutex edge and breaks the cycle.
The crash info is below： ====================================================== WARNING: possible circular locking dependency detected 5.18.0-rc1 #1 Not tainted ------------------------------------------------------ syz-executor.1/1374 is trying to acquire lock: ffffffff8f8bb4e8 (card_mutex#2){+.+.}-{3:3}, at: nosy_open+0x55/0x480
but task is already holding lock: ffffffff8ef78a88 (misc_mtx){+.+.}-{3:3}, at: misc_open+0x5a/0x3f0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is: -> #1 (misc_mtx){+.+.}-{3:3}: __mutex_lock_common+0x138/0x2450 mutex_lock_nested+0x17/0x20 misc_register+0x95/0x490 foo_misc_register+0x3a/0x50 add_card+0xd38/0x1250 local_pci_probe+0x140/0x200 pci_device_probe+0x345/0x770 really_probe+0x2d1/0x990 __driver_probe_device+0x1a7/0x270 driver_probe_device+0x54/0x370 __driver_attach+0x430/0x590 bus_for_each_dev+0x125/0x190 bus_add_driver+0x32c/0x530 driver_register+0x309/0x410 do_one_initcall+0x104/0x250 do_initcall_level+0x102/0x132 do_initcalls+0x46/0x74 kernel_init_freeable+0x28f/0x393 kernel_init+0x14/0x1a0 ret_from_fork+0x22/0x30 -> #0 (card_mutex#2){+.+.}-{3:3}: __lock_acquire+0x3607/0x7930 lock_acquire+0xff/0x2d0 __mutex_lock_common+0x138/0x2450 mutex_lock_nested+0x17/0x20 nosy_open+0x55/0x480 misc_open+0x363/0x3f0 chrdev_open+0x407/0x490 do_dentry_open+0x5b4/0xc20 path_openat+0x1d7a/0x2300 do_filp_open+0x1cb/0x3e0 do_sys_openat2+0x96/0x350 __x64_sys_openat+0x186/0x1b0 do_syscall_64+0x43/0x90 entry_SYSCALL_64_after_hwframe+0x44/0xae other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1 ---- ---- lock(misc_mtx); lock(card_mutex#2); lock(misc_mtx); lock(card_mutex#2);
*** DEADLOCK ***
1 lock held by syz-executor.1/1374: #0: ffffffff8ef78a88 (misc_mtx){+.+.}-{3:3}, at: misc_open+0x5a/0x3f0
stack backtrace: CPU: 0 PID: 1374 Comm: syz-executor.1 Not tainted 5.18.0-rc1 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x5a/0x74 check_noncircular+0x223/0x2d0 __lock_acquire+0x3607/0x7930 lock_acquire+0xff/0x2d0 __mutex_lock_common+0x138/0x2450 mutex_lock_nested+0x17/0x20 nosy_open+0x55/0x480 misc_open+0x363/0x3f0 chrdev_open+0x407/0x490 do_dentry_open+0x5b4/0xc20 path_openat+0x1d7a/0x2300 do_filp_open+0x1cb/0x3e0 do_sys_openat2+0x96/0x350 __x64_sys_openat+0x186/0x1b0 do_syscall_64+0x43/0x90 entry_SYSCALL_64_after_hwframe+0x44/0xae RIP: 0033:0x7fbaab4abbcd Code: 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007fbaaac1cbe8 EFLAGS: 00000246 ORIG_RAX: 0000000000000101 RAX: ffffffffffffffda RBX: 00007fbaab5c9f80 RCX: 00007fbaab4abbcd RDX: 0000000000062803 RSI: 0000000020003080 RDI: ffffffffffffff9c RBP: 00007fbaab519edb R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 R13: 00007ffe62540a6f R14: 00007ffe62540c10 R15: 00007fbaaac1cd80 </TASK> ====================================================== Fixes: 424d66cedae8 ("firewire: nosy: fix device shutdown with active client") Cc: stable@vger.kernel.org Signed-off-by: Guangshuo Li lgs201920130244@gmail.com --- drivers/firewire/nosy.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c index b0d671db178a..726d5f15ff08 100644 --- a/drivers/firewire/nosy.c +++ b/drivers/firewire/nosy.c @@ -263,19 +263,13 @@ set_phy_reg(struct pcilynx *lynx, int addr, int val) static int nosy_open(struct inode *inode, struct file *file) { - int minor = iminor(inode); + struct miscdevice *misc = file->private_data; struct client *client; - struct pcilynx *tmp, *lynx = NULL; + struct pcilynx *lynx;
- mutex_lock(&card_mutex); - list_for_each_entry(tmp, &card_list, link) - if (tmp->misc.minor == minor) { - lynx = lynx_get(tmp); - break; - } - mutex_unlock(&card_mutex); - if (lynx == NULL) - return -ENODEV; + /* misc_open() set file->private_data to the miscdevice already. */ + lynx = container_of(misc, struct pcilynx, misc); + lynx_get(lynx);
client = kmalloc(sizeof *client, GFP_KERNEL); if (client == NULL)