summaryrefslogtreecommitdiff
path: root/drivers/comedi
diff options
context:
space:
mode:
authorNikita Zhandarovich <n.zhandarovich@fintech.ru>2025-10-23 16:22:32 +0300
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2025-12-12 18:42:47 +0100
commitaac80e912de306815297a3b74f0426873ffa7dc3 (patch)
treeab9eb61473bfd1a2285117fa9fe1cafe2a6859fe /drivers/comedi
parent543f4c380c2e1f35e60528df7cb54705cda7fee3 (diff)
comedi: check device's attached status in compat ioctls
commit 0de7d9cd07a2671fa6089173bccc0b2afe6b93ee upstream. 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@syzkaller.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 <stable@kernel.org> Reviewed-by: Ian Abbott <abbotti@mev.co.uk> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru> Link: https://patch.msgid.link/20251023132234.395794-1-n.zhandarovich@fintech.ru Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/comedi')
-rw-r--r--drivers/comedi/comedi_fops.c42
1 files changed, 36 insertions, 6 deletions
diff --git a/drivers/comedi/comedi_fops.c b/drivers/comedi/comedi_fops.c
index 7e2f2b1a1c36..b2e62e04afd9 100644
--- a/drivers/comedi/comedi_fops.c
+++ b/drivers/comedi/comedi_fops.c
@@ -3023,7 +3023,12 @@ static int compat_chaninfo(struct file *file, unsigned long arg)
chaninfo.rangelist = compat_ptr(chaninfo32.rangelist);
mutex_lock(&dev->mutex);
- err = do_chaninfo_ioctl(dev, &chaninfo);
+ if (!dev->attached) {
+ dev_dbg(dev->class_dev, "no driver attached\n");
+ err = -ENODEV;
+ } else {
+ err = do_chaninfo_ioctl(dev, &chaninfo);
+ }
mutex_unlock(&dev->mutex);
return err;
}
@@ -3044,7 +3049,12 @@ static int compat_rangeinfo(struct file *file, unsigned long arg)
rangeinfo.range_ptr = compat_ptr(rangeinfo32.range_ptr);
mutex_lock(&dev->mutex);
- err = do_rangeinfo_ioctl(dev, &rangeinfo);
+ if (!dev->attached) {
+ dev_dbg(dev->class_dev, "no driver attached\n");
+ err = -ENODEV;
+ } else {
+ err = do_rangeinfo_ioctl(dev, &rangeinfo);
+ }
mutex_unlock(&dev->mutex);
return err;
}
@@ -3120,7 +3130,12 @@ static int compat_cmd(struct file *file, unsigned long arg)
return rc;
mutex_lock(&dev->mutex);
- rc = do_cmd_ioctl(dev, &cmd, &copy, file);
+ if (!dev->attached) {
+ dev_dbg(dev->class_dev, "no driver attached\n");
+ rc = -ENODEV;
+ } else {
+ rc = do_cmd_ioctl(dev, &cmd, &copy, file);
+ }
mutex_unlock(&dev->mutex);
if (copy) {
/* Special case: copy cmd back to user. */
@@ -3145,7 +3160,12 @@ static int compat_cmdtest(struct file *file, unsigned long arg)
return rc;
mutex_lock(&dev->mutex);
- rc = do_cmdtest_ioctl(dev, &cmd, &copy, file);
+ if (!dev->attached) {
+ dev_dbg(dev->class_dev, "no driver attached\n");
+ rc = -ENODEV;
+ } else {
+ rc = do_cmdtest_ioctl(dev, &cmd, &copy, file);
+ }
mutex_unlock(&dev->mutex);
if (copy) {
err = put_compat_cmd(compat_ptr(arg), &cmd);
@@ -3205,7 +3225,12 @@ static int compat_insnlist(struct file *file, unsigned long arg)
}
mutex_lock(&dev->mutex);
- rc = do_insnlist_ioctl(dev, insns, insnlist32.n_insns, file);
+ if (!dev->attached) {
+ dev_dbg(dev->class_dev, "no driver attached\n");
+ rc = -ENODEV;
+ } else {
+ rc = do_insnlist_ioctl(dev, insns, insnlist32.n_insns, file);
+ }
mutex_unlock(&dev->mutex);
kfree(insns);
return rc;
@@ -3224,7 +3249,12 @@ static int compat_insn(struct file *file, unsigned long arg)
return rc;
mutex_lock(&dev->mutex);
- rc = do_insn_ioctl(dev, &insn, file);
+ if (!dev->attached) {
+ dev_dbg(dev->class_dev, "no driver attached\n");
+ rc = -ENODEV;
+ } else {
+ rc = do_insn_ioctl(dev, &insn, file);
+ }
mutex_unlock(&dev->mutex);
return rc;
}