| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251105115819.1083201-1-lgs201920130244@gmail.com>
Date: Wed, 5 Nov 2025 19:58:18 +0800
From: Guangshuo Li <lgs201920130244@...il.com>
To: Stefan Richter <stefanr@...6.in-berlin.de>,
linux1394-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org
Cc: Guangshuo Li <lgs201920130244@...il.com>,
stable@...r.kernel.org
Subject: [PATCH] firewire: nosy: break circular locking with misc_mtx in ->open()
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@...r.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@...il.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)
--
2.43.0
Powered by blists - more mailing lists