| From 0de7d9cd07a2671fa6089173bccc0b2afe6b93ee Mon Sep 17 00:00:00 2001 |
| From: Nikita Zhandarovich <n.zhandarovich@fintech.ru> |
| Date: Thu, 23 Oct 2025 16:22:32 +0300 |
| Subject: comedi: check device's attached status in compat ioctls |
| |
| From: Nikita Zhandarovich <n.zhandarovich@fintech.ru> |
| |
| 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> |
| --- |
| drivers/comedi/comedi_fops.c | 42 ++++++++++++++++++++++++++++++++++++------ |
| 1 file changed, 36 insertions(+), 6 deletions(-) |
| |
| --- a/drivers/comedi/comedi_fops.c |
| +++ b/drivers/comedi/comedi_fops.c |
| @@ -2966,7 +2966,12 @@ static int compat_chaninfo(struct file * |
| 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; |
| } |
| @@ -2987,7 +2992,12 @@ static int compat_rangeinfo(struct file |
| 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; |
| } |
| @@ -3063,7 +3073,12 @@ static int compat_cmd(struct file *file, |
| return rc; |
| |
| mutex_lock(&dev->mutex); |
| - rc = do_cmd_ioctl(dev, &cmd, ©, file); |
| + if (!dev->attached) { |
| + dev_dbg(dev->class_dev, "no driver attached\n"); |
| + rc = -ENODEV; |
| + } else { |
| + rc = do_cmd_ioctl(dev, &cmd, ©, file); |
| + } |
| mutex_unlock(&dev->mutex); |
| if (copy) { |
| /* Special case: copy cmd back to user. */ |
| @@ -3088,7 +3103,12 @@ static int compat_cmdtest(struct file *f |
| return rc; |
| |
| mutex_lock(&dev->mutex); |
| - rc = do_cmdtest_ioctl(dev, &cmd, ©, file); |
| + if (!dev->attached) { |
| + dev_dbg(dev->class_dev, "no driver attached\n"); |
| + rc = -ENODEV; |
| + } else { |
| + rc = do_cmdtest_ioctl(dev, &cmd, ©, file); |
| + } |
| mutex_unlock(&dev->mutex); |
| if (copy) { |
| err = put_compat_cmd(compat_ptr(arg), &cmd); |
| @@ -3148,7 +3168,12 @@ static int compat_insnlist(struct file * |
| } |
| |
| 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; |
| @@ -3167,7 +3192,12 @@ static int compat_insn(struct file *file |
| 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; |
| } |