linux-kernel - [PATCH 2/2] comedi: comedi_bond: Check for loops when bonding devices

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251027153748.4569-3-abbotti@mev.co.uk>
Date: Mon, 27 Oct 2025 15:25:03 +0000
From: Ian Abbott <abbotti@....co.uk>
To: linux-kernel@...r.kernel.org
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Ian Abbott <abbotti@....co.uk>,
	H Hartley Sweeten <hsweeten@...ionengravers.com>
Subject: [PATCH 2/2] comedi: comedi_bond: Check for loops when bonding devices

The "comedi_bond" driver allows a composite COMEDI device to be built up
from the subdevices of other COMEDI devices, although it currently only
supports digital I/O subdevices.  Although it checks that it is not
trying to bind to itself, it is possible to end up with a cycle of
"comedi_bond" devices bound to each other.  For example:

1. Configure /dev/comedi0 to use some COMEDI hardware device with
   digital I/O subdevices, but not a "comedi_bond" device.
2. Configure /dev/comedi1 as a "comedi_bond" device bound to
   /dev/comedi0.
3. Unconfigure /dev/comedi0 and reconfigure it as a "comedi_bond" device
   bound to /dev/comedi1.

Now we have /dev/comedi0 and /dev/comedi1 bound in a cycle.  When an
operation is performed on the digital I/O subdevice of /dev/comedi0 for
example, it will try and perform the operation on /dev/comedi1, which
will try and perform the operation on /dev/comedi0.  The task will end
up deadlocked trying to lock /dev/comedi0's mutex which it has already
locked.

I discovered that possibility while investigating fix sysbot crash
https://syzkaller.appspot.com/bug?extid=4a6138c17a47937dcea1 ("possible
deadlock in comedi_do_insn"), but I think that report may be a false
positive.

To avoid that, replace the calls to `comedi_open()` and `comedi_close()`
in "kcomedilib" with calls to `comedi_open_from()` and
`comedi_close_from()`.  These take an extra parameter that indicates the
COMEDI minor device number from which the open or close is being
performed.  `comedi_open_from()` will refuse to open the device if doing
so would result in a cycle.  The cycle detection depends on the extra
parameter having the correct value for this device and also for existing
devices in the chain.

Signed-off-by: Ian Abbott <abbotti@....co.uk>
---
 drivers/comedi/drivers/comedi_bond.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/comedi/drivers/comedi_bond.c b/drivers/comedi/drivers/comedi_bond.c
index 78c39fa84177..30650fa36fff 100644
--- a/drivers/comedi/drivers/comedi_bond.c
+++ b/drivers/comedi/drivers/comedi_bond.c
@@ -205,7 +205,7 @@ static int do_dev_config(struct comedi_device *dev, struct comedi_devconfig *it)
 		snprintf(file, sizeof(file), "/dev/comedi%d", minor);
 		file[sizeof(file) - 1] = 0;

-		d = comedi_open(file);
+		d = comedi_open_from(file, dev->minor);

 		if (!d) {
 			dev_err(dev->class_dev,
@@ -326,7 +326,7 @@ static void bonding_detach(struct comedi_device *dev)
 			if (!bdev)
 				continue;
 			if (!test_and_set_bit(bdev->minor, devs_closed))
-				comedi_close(bdev->dev);
+				comedi_close_from(bdev->dev, dev->minor);
 			kfree(bdev);
 		}
 		kfree(devpriv->devs);
-- 
2.51.0