[PATCH] comedi: give each device's mutex its own lockdep class

[PATCH] comedi: give each device's mutex its own lockdep class

[S0CloseYetS0Far]

From: abu mohammed saleh <foroncewillbeproud@gmail.com>

The "comedi_bond" driver forwards digital I/O instructions to other
COMEDI devices via "kcomedilib".  When an INSN_BITS instruction is sent
to a bonded device, comedi_unlocked_ioctl() holds the bonded device's
mutex while comedi_do_insn() acquires the mutex of the underlying
device.  All COMEDI device mutexes are initialized in
comedi_device_init() and so share a single lockdep class, so lockdep
reports this as recursive locking:

  WARNING: possible recursive locking detected
  repro/77 is trying to acquire lock:
  ffff9d0b41f39cf0 (&dev->mutex#3){+.+.}-{4:4}, at: comedi_do_insn+0x22/0xf0
  but task is already holding lock:
  ffff9d0b41f398f0 (&dev->mutex#3){+.+.}-{4:4}, at: comedi_unlocked_ioctl+0x5f/0x1060

The two locks belong to different devices.  Since commits d1b3b9c70e11
("comedi: kcomedilib: Add loop checking variants of open and close")
and 2402f958cf3b ("comedi: comedi_bond: Check for loops when bonding
devices"), comedi_open_from() refuses to open a device if that would
form a loop of devices opening each other, so this nesting cannot
deadlock.

Register a dynamic lockdep key for each COMEDI device and use it as the
class of the device's mutex, unregistering it when the device is freed.
This silences the false positive while keeping lockdep's ordering checks
between different devices, which a mutex_lock_nested() annotation in
comedi_do_insn() would not.  It also handles chains of bonded devices,
which a single nesting subclass could not.  There are at most
COMEDI_NUM_BOARD_MINORS devices, so the number of classes is small.

The problem was found by syzbot, which can trigger it as root by
configuring a "comedi_test" device and a "comedi_bond" device bonding it
on legacy minors, then issuing INSN_BITS on the bonded device.

Tested on v7.4-rc1 (x86_64 KVM guest, defconfig + kvm_guest.config +
CONFIG_PROVE_LOCKING, gcc 15.2) with syzbot's C reproducer, booted with
comedi.comedi_num_legacy_minors=4: unpatched, lockdep reports the splat
above on every boot; patched, no lockdep reports in three runs.  Also
built with CONFIG_LOCKDEP disabled and with W=1 without new warnings.
Not tested with real COMEDI hardware.

This patch was written with the help of an AI coding assistant (Claude
Opus 5.5, Anthropic), which analyzed the syzbot report, wrote the change
and ran the tests above under the submitter's direction.

Reported-by: syzbot+acd959f649966559199c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=acd959f649966559199c
Fixes: 69e2387f3bfd ("staging: comedi: kcomedilib: protect against device detachment")
Signed-off-by: abu mohammed saleh <foroncewillbeproud@gmail.com>
---
 drivers/comedi/comedi_fops.c     | 7 +++++++
 include/linux/comedi/comedidev.h | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/comedi/comedi_fops.c b/drivers/comedi/comedi_fops.c
index 05d14839c..a488044d9 100644
--- a/drivers/comedi/comedi_fops.c
+++ b/drivers/comedi/comedi_fops.c
@@ -108,6 +108,12 @@ static void comedi_device_init(struct comedi_device *dev)
 	kref_init(&dev->refcount);
 	spin_lock_init(&dev->spinlock);
 	mutex_init(&dev->mutex);
+	/*
+	 * Give each device's mutex its own lockdep class, as one device's
+	 * mutex may be held while acquiring another's via kcomedilib.
+	 */
+	lockdep_register_key(&dev->mutex_key);
+	lockdep_set_class(&dev->mutex, &dev->mutex_key);
 	init_rwsem(&dev->attach_lock);
 	dev->minor = -1;
 }
@@ -118,6 +124,7 @@ static void comedi_dev_kref_release(struct kref *kref)
 		container_of(kref, struct comedi_device, refcount);
 
 	mutex_destroy(&dev->mutex);
+	lockdep_unregister_key(&dev->mutex_key);
 	put_device(dev->class_dev);
 	kfree(dev);
 }
diff --git a/include/linux/comedi/comedidev.h b/include/linux/comedi/comedidev.h
index 577a08f37..f6d732427 100644
--- a/include/linux/comedi/comedidev.h
+++ b/include/linux/comedi/comedidev.h
@@ -489,6 +489,10 @@ struct comedi_driver {
  *	its regions requested.
  * @spinlock: Generic spin-lock for use by the low-level driver.
  * @mutex: Generic mutex for use by the COMEDI core module.
+ * @mutex_key: Lockdep class key for @mutex.  Each COMEDI device has its own
+ *	class because a device's @mutex may be held while the @mutex of another
+ *	COMEDI device is acquired via "kcomedilib" (e.g. by the "comedi_bond"
+ *	driver).  "kcomedilib" refuses to form loops of such devices.
  * @attach_lock: &struct rw_semaphore used to guard against the COMEDI device
  *	being detached while an operation is in progress.  The down_write()
  *	operation is only allowed while @mutex is held and is used when
@@ -553,6 +557,7 @@ struct comedi_device {
 	unsigned int ioenabled:1;
 	spinlock_t spinlock;	/* generic spin-lock for low-level driver */
 	struct mutex mutex;	/* generic mutex for COMEDI core */
+	struct lock_class_key mutex_key;
 	struct rw_semaphore attach_lock;
 	struct kref refcount;
 
-- 
2.53.0