Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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@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>

authored by

Nikita Zhandarovich and committed by
Greg Kroah-Hartman
0de7d9cd 72262330

+36 -6
+36 -6
drivers/comedi/comedi_fops.c
··· 3018 3018 chaninfo.rangelist = compat_ptr(chaninfo32.rangelist); 3019 3019 3020 3020 mutex_lock(&dev->mutex); 3021 - err = do_chaninfo_ioctl(dev, &chaninfo); 3021 + if (!dev->attached) { 3022 + dev_dbg(dev->class_dev, "no driver attached\n"); 3023 + err = -ENODEV; 3024 + } else { 3025 + err = do_chaninfo_ioctl(dev, &chaninfo); 3026 + } 3022 3027 mutex_unlock(&dev->mutex); 3023 3028 return err; 3024 3029 } ··· 3044 3039 rangeinfo.range_ptr = compat_ptr(rangeinfo32.range_ptr); 3045 3040 3046 3041 mutex_lock(&dev->mutex); 3047 - err = do_rangeinfo_ioctl(dev, &rangeinfo); 3042 + if (!dev->attached) { 3043 + dev_dbg(dev->class_dev, "no driver attached\n"); 3044 + err = -ENODEV; 3045 + } else { 3046 + err = do_rangeinfo_ioctl(dev, &rangeinfo); 3047 + } 3048 3048 mutex_unlock(&dev->mutex); 3049 3049 return err; 3050 3050 } ··· 3125 3115 return rc; 3126 3116 3127 3117 mutex_lock(&dev->mutex); 3128 - rc = do_cmd_ioctl(dev, &cmd, &copy, file); 3118 + if (!dev->attached) { 3119 + dev_dbg(dev->class_dev, "no driver attached\n"); 3120 + rc = -ENODEV; 3121 + } else { 3122 + rc = do_cmd_ioctl(dev, &cmd, &copy, file); 3123 + } 3129 3124 mutex_unlock(&dev->mutex); 3130 3125 if (copy) { 3131 3126 /* Special case: copy cmd back to user. */ ··· 3155 3140 return rc; 3156 3141 3157 3142 mutex_lock(&dev->mutex); 3158 - rc = do_cmdtest_ioctl(dev, &cmd, &copy, file); 3143 + if (!dev->attached) { 3144 + dev_dbg(dev->class_dev, "no driver attached\n"); 3145 + rc = -ENODEV; 3146 + } else { 3147 + rc = do_cmdtest_ioctl(dev, &cmd, &copy, file); 3148 + } 3159 3149 mutex_unlock(&dev->mutex); 3160 3150 if (copy) { 3161 3151 err = put_compat_cmd(compat_ptr(arg), &cmd); ··· 3220 3200 } 3221 3201 3222 3202 mutex_lock(&dev->mutex); 3223 - rc = do_insnlist_ioctl(dev, insns, insnlist32.n_insns, file); 3203 + if (!dev->attached) { 3204 + dev_dbg(dev->class_dev, "no driver attached\n"); 3205 + rc = -ENODEV; 3206 + } else { 3207 + rc = do_insnlist_ioctl(dev, insns, insnlist32.n_insns, file); 3208 + } 3224 3209 mutex_unlock(&dev->mutex); 3225 3210 kfree(insns); 3226 3211 return rc; ··· 3244 3219 return rc; 3245 3220 3246 3221 mutex_lock(&dev->mutex); 3247 - rc = do_insn_ioctl(dev, &insn, file); 3222 + if (!dev->attached) { 3223 + dev_dbg(dev->class_dev, "no driver attached\n"); 3224 + rc = -ENODEV; 3225 + } else { 3226 + rc = do_insn_ioctl(dev, &insn, file); 3227 + } 3248 3228 mutex_unlock(&dev->mutex); 3249 3229 return rc; 3250 3230 }