linux-kernel - [PATCH] comedi: check device's attached status in compat ioctls

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>] [day] [month] [year] [list]
Message-ID: <20251020174125.150608-1-n.zhandarovich@fintech.ru>
Date: Mon, 20 Oct 2025 20:41:22 +0300
From: Nikita Zhandarovich <n.zhandarovich@...tech.ru>
To: Ian Abbott <abbotti@....co.uk>, H Hartley Sweeten
	<hsweeten@...ionengravers.com>
CC: Nikita Zhandarovich <n.zhandarovich@...tech.ru>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, <linux-kernel@...r.kernel.org>,
	<syzbot+ab8008c24e84adee93ff@...kaller.appspotmail.com>,
	<lvc-project@...uxtesting.org>
Subject: [PATCH] comedi: check device's attached status in compat ioctls

Syzbot identified an issue [1] that crashes kernel, seemingly due to
unexistent callback dev->get_valid_routes(). By all means, this should
not occur as said callback must always be set to
get_zero_valid_routes() in __comedi_device_postconfig().

As the crash seems to appear exclusively in i386 kernels, at least,
judging from [1] reports, the blame lies with compat versions
of standard IOCTL handlers. Several of them are modified and
do not use comedi_unlocked_ioctl(). While functionality of these
ioctls essentially copy their original versions, they do not
have required sanity check for device's attached status. This,
in turn, leads to a possibility of calling select IOCTLs on a
device that has not been properly setup, even via COMEDI_DEVCONFIG.

Doing so on unconfigured devices means that several crucial steps
are missed, for instance, specifying dev->get_valid_routes()
callback.

Fix this somewhat crudely by ensuring device's attached status before
performing any ioctls, improving logic consistency between modern
and compat functions.

[1] Syzbot report:
BUG: kernel NULL pointer dereference, address: 0000000000000000
...
CR2: ffffffffffffffd6 CR3: 000000006c717000 CR4: 0000000000352ef0
Call Trace:
 <TASK>
 get_valid_routes drivers/comedi/comedi_fops.c:1322 [inline]
 parse_insn+0x78c/0x1970 drivers/comedi/comedi_fops.c:1401
 do_insnlist_ioctl+0x272/0x700 drivers/comedi/comedi_fops.c:1594
 compat_insnlist drivers/comedi/comedi_fops.c:3208 [inline]
 comedi_compat_ioctl+0x810/0x990 drivers/comedi/comedi_fops.c:3273
 __do_compat_sys_ioctl fs/ioctl.c:695 [inline]
 __se_compat_sys_ioctl fs/ioctl.c:638 [inline]
 __ia32_compat_sys_ioctl+0x242/0x370 fs/ioctl.c:638
 do_syscall_32_irqs_on arch/x86/entry/syscall_32.c:83 [inline]
...

Reported-by: syzbot+ab8008c24e84adee93ff@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ab8008c24e84adee93ff
Fixes: 3fbfd2223a27 ("comedi: get rid of compat_alloc_user_space() mess in COMEDI_CHANINFO compat")
Cc: stable@...r.kernel.org
Signed-off-by: Nikita Zhandarovich <n.zhandarovich@...tech.ru>
---
Compile-tested only due to my QEMU setup currently being broken.
Also, there is no decent syzkaller repro for this problem so
testing this is a little tricky.

P.S. 'Fixes' tag is technically not correct, as each compat ioctl
that requires special handling arguments-wise has been modified
in separate commit. I've opted not to mention each and every one
of them.

P.P.S. While these multiple identical checks look abhorrent, I've
decided against changing current approach to compat functions.

 drivers/comedi/comedi_fops.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/comedi/comedi_fops.c b/drivers/comedi/comedi_fops.c
index 7e2f2b1a1c36..123e9af2ed44 100644
--- a/drivers/comedi/comedi_fops.c
+++ b/drivers/comedi/comedi_fops.c
@@ -3023,6 +3023,11 @@ static int compat_chaninfo(struct file *file, unsigned long arg)
 	chaninfo.rangelist = compat_ptr(chaninfo32.rangelist);
 
 	mutex_lock(&dev->mutex);
+	if (!dev->attached) {
+		dev_dbg(dev->class_dev, "no driver attached\n");
+		mutex_unlock(&dev->mutex);
+		return -ENODEV;
+	}
 	err = do_chaninfo_ioctl(dev, &chaninfo);
 	mutex_unlock(&dev->mutex);
 	return err;
@@ -3044,6 +3049,11 @@ static int compat_rangeinfo(struct file *file, unsigned long arg)
 	rangeinfo.range_ptr = compat_ptr(rangeinfo32.range_ptr);
 
 	mutex_lock(&dev->mutex);
+	if (!dev->attached) {
+		dev_dbg(dev->class_dev, "no driver attached\n");
+		mutex_unlock(&dev->mutex);
+		return -ENODEV;
+	}
 	err = do_rangeinfo_ioctl(dev, &rangeinfo);
 	mutex_unlock(&dev->mutex);
 	return err;
@@ -3120,6 +3130,11 @@ static int compat_cmd(struct file *file, unsigned long arg)
 		return rc;
 
 	mutex_lock(&dev->mutex);
+	if (!dev->attached) {
+		dev_dbg(dev->class_dev, "no driver attached\n");
+		mutex_unlock(&dev->mutex);
+		return -ENODEV;
+	}
 	rc = do_cmd_ioctl(dev, &cmd, &copy, file);
 	mutex_unlock(&dev->mutex);
 	if (copy) {
@@ -3145,6 +3160,11 @@ static int compat_cmdtest(struct file *file, unsigned long arg)
 		return rc;
 
 	mutex_lock(&dev->mutex);
+	if (!dev->attached) {
+		dev_dbg(dev->class_dev, "no driver attached\n");
+		mutex_unlock(&dev->mutex);
+		return -ENODEV;
+	}
 	rc = do_cmdtest_ioctl(dev, &cmd, &copy, file);
 	mutex_unlock(&dev->mutex);
 	if (copy) {
@@ -3205,6 +3225,12 @@ static int compat_insnlist(struct file *file, unsigned long arg)
 	}
 
 	mutex_lock(&dev->mutex);
+	if (!dev->attached) {
+		dev_dbg(dev->class_dev, "no driver attached\n");
+		mutex_unlock(&dev->mutex);
+		kfree(insns);
+		return -ENODEV;
+	}
 	rc = do_insnlist_ioctl(dev, insns, insnlist32.n_insns, file);
 	mutex_unlock(&dev->mutex);
 	kfree(insns);
@@ -3224,6 +3250,11 @@ static int compat_insn(struct file *file, unsigned long arg)
 		return rc;
 
 	mutex_lock(&dev->mutex);
+	if (!dev->attached) {
+		dev_dbg(dev->class_dev, "no driver attached\n");
+		mutex_unlock(&dev->mutex);
+		return -ENODEV;
+	}
 	rc = do_insn_ioctl(dev, &insn, file);
 	mutex_unlock(&dev->mutex);
 	return rc;