kernel/linux.git/drivers/comedi/comedi_fops.c, branch v6.19.11

comedi: Fix getting range information for subdevices 16 to 255

2026-01-16T15:42:15+00:00

The `COMEDI_RANGEINFO` ioctl does not work properly for subdevice indices above 15. Currently, the only in-tree COMEDI drivers that support more than 16 subdevices are the "8255" driver and the "comedi_bond" driver. Making the ioctl work for subdevice indices up to 255 is achievable. It needs minor changes to the handling of the `COMEDI_RANGEINFO` and `COMEDI_CHANINFO` ioctls that should be mostly harmless to user-space, apart from making them less broken. Details follow... The `COMEDI_RANGEINFO` ioctl command gets the list of supported ranges (usually with units of volts or milliamps) for a COMEDI subdevice or channel. (Only some subdevices have per-channel range tables, indicated by the `SDF_RANGETYPE` flag in the subdevice information.) It uses a `range_type` value and a user-space pointer, both supplied by user-space, but the `range_type` value should match what was obtained using the `COMEDI_CHANINFO` ioctl (if the subdevice has per-channel range tables) or `COMEDI_SUBDINFO` ioctl (if the subdevice uses a single range table for all channels). Bits 15 to 0 of the `range_type` value contain the length of the range table, which is the only part that user-space should care about (so it can use a suitably sized buffer to fetch the range table). Bits 23 to 16 store the channel index, which is assumed to be no more than 255 if the subdevice has per-channel range tables, and is set to 0 if the subdevice has a single range table. For `range_type` values produced by the `COMEDI_SUBDINFO` ioctl, bits 31 to 24 contain the subdevice index, which is assumed to be no more than 255. But for `range_type` values produced by the `COMEDI_CHANINFO` ioctl, bits 27 to 24 contain the subdevice index, which is assumed to be no more than 15, and bits 31 to 28 contain the COMEDI device's minor device number for some unknown reason lost in the mists of time. The `COMEDI_RANGEINFO` ioctl extract the length from bits 15 to 0 of the user-supplied `range_type` value, extracts the channel index from bits 23 to 16 (only used if the subdevice has per-channel range tables), extracts the subdevice index from bits 27 to 24, and ignores bits 31 to 28. So for subdevice indices 16 to 255, the `COMEDI_SUBDINFO` or `COMEDI_CHANINFO` ioctl will report a `range_type` value that doesn't work with the `COMEDI_RANGEINFO` ioctl. It will either get the range table for the subdevice index modulo 16, or will fail with `-EINVAL`. To fix this, always use bits 31 to 24 of the `range_type` value to hold the subdevice index (assumed to be no more than 255). This affects the `COMEDI_CHANINFO` and `COMEDI_RANGEINFO` ioctls. There should not be anything in user-space that depends on the old, broken usage, although it may now see different values in bits 31 to 28 of the `range_type` values reported by the `COMEDI_CHANINFO` ioctl for subdevices that have per-channel subdevices. User-space should not be trying to decode bits 31 to 16 of the `range_type` values anyway. Fixes: ed9eccbe8970 ("Staging: add comedi core") Cc: stable@vger.kernel.org #5.17+ Signed-off-by: Ian Abbott Link: https://patch.msgid.link/20251203162438.176841-1-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman

comedi: Use reference count for asynchronous command functions

2025-11-26T13:19:41+00:00

comedi: Add reference counting for Comedi command handling

2025-11-26T13:19:41+00:00

For interrupts from badly behaved hardware (as emulated by Syzbot), it is possible for the Comedi core functions that manage the progress of asynchronous data acquisition to be called from driver ISRs while no asynchronous command has been set up, which can cause problems such as invalid pointer dereferencing or dividing by zero. To help protect against that, introduce new functions to maintain a reference counter for asynchronous commands that are being set up. `comedi_get_is_subdevice_running(s)` will check if a command has been set up on a subdevice and is still marked as running, and if so will increment the reference counter and return `true`, otherwise it will return `false` without modifying the reference counter. `comedi_put_is_subdevice_running(s)` will decrement the reference counter and set a completion event when decremented to 0. Change the `do_cmd_ioctl()` function (responsible for setting up the asynchronous command) to reinitialize the completion event and set the reference counter to 1 before it marks the subdevice as running. Change the `do_become_nonbusy()` function (responsible for destroying a completed command) to call `comedi_put_is_subdevice_running(s)` and wait for the completion event after marking the subdevice as not running. Because the subdevice normally gets marked as not running before the call to `do_become_nonbusy()` (and may also be called when the Comedi device is being detached from the low-level driver), add a new flag `COMEDI_SRF_BUSY` to the set of subdevice run-flags that indicates that an asynchronous command was set up and will need to be destroyed. This flag is set by `do_cmd_ioctl()` and cleared and checked by `do_become_nonbusy()`. Subsequent patches will change the Comedi core functions that are called from low-level drivers for asynchrous command handling to make use of the `comedi_get_is_subdevice_running()` and `comedi_put_is_subdevice_running()` functions, and will modify the ISRs of some of these low-level drivers if they dereference the subdevice's `async` pointer directly. Signed-off-by: Ian Abbott Link: https://patch.msgid.link/20251023133001.8439-2-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman

comedi: check device's attached status in compat ioctls

2025-11-26T12:57:29+00:00

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: 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 Reviewed-by: Ian Abbott Signed-off-by: Nikita Zhandarovich Link: https://patch.msgid.link/20251023132234.395794-1-n.zhandarovich@fintech.ru Signed-off-by: Greg Kroah-Hartman

comedi: Replace kcalloc + copy_from_user with memdup_array_user

2025-10-22T05:54:41+00:00

Replace kcalloc() followed by copy_from_user() with memdup_array_user() to improve and simplify comedi_unlocked_ioctl(). No functional changes intended. Signed-off-by: Thorsten Blum Reviewed-by: Ian Abbott Link: https://patch.msgid.link/20250917131349.117642-2-thorsten.blum@linux.dev Signed-off-by: Greg Kroah-Hartman

comedi: Fix use of uninitialized memory in do_insn_ioctl() and do_insnlist_ioctl()

2025-08-19T10:56:47+00:00

syzbot reports a KMSAN kernel-infoleak in `do_insn_ioctl()`. A kernel buffer is allocated to hold `insn->n` samples (each of which is an `unsigned int`). For some instruction types, `insn->n` samples are copied back to user-space, unless an error code is being returned. The problem is that not all the instruction handlers that need to return data to userspace fill in the whole `insn->n` samples, so that there is an information leak. There is a similar syzbot report for `do_insnlist_ioctl()`, although it does not have a reproducer for it at the time of writing. One culprit is `insn_rw_emulate_bits()` which is used as the handler for `INSN_READ` or `INSN_WRITE` instructions for subdevices that do not have a specific handler for that instruction, but do have an `INSN_BITS` handler. For `INSN_READ` it only fills in at most 1 sample, so if `insn->n` is greater than 1, the remaining `insn->n - 1` samples copied to userspace will be uninitialized kernel data. Another culprit is `vm80xx_ai_insn_read()` in the "vm80xx" driver. It never returns an error, even if it fails to fill the buffer. Fix it in `do_insn_ioctl()` and `do_insnlist_ioctl()` by making sure that uninitialized parts of the allocated buffer are zeroed before handling each instruction. Thanks to Arnaud Lecomte for their fix to `do_insn_ioctl()`. That fix replaced the call to `kmalloc_array()` with `kcalloc()`, but it is not always necessary to clear the whole buffer. Fixes: ed9eccbe8970 ("Staging: add comedi core") Reported-by: syzbot+a5e45f768aab5892da5d@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=a5e45f768aab5892da5d Reported-by: syzbot+fb4362a104d45ab09cf9@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=fb4362a104d45ab09cf9 Cc: stable # 5.13+ Cc: Arnaud Lecomte Signed-off-by: Ian Abbott Link: https://lore.kernel.org/r/20250725125324.80276-1-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman

Merge tag 'char-misc-6.17-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc

2025-07-29T16:52:01+00:00

Pull char / misc / IIO / other driver updates from Greg KH: "Here is the big set of char/misc/iio and other smaller driver subsystems for 6.17-rc1. It's a big set this time around, with the huge majority being in the iio subsystem with new drivers and dts files being added there. Highlights include: - IIO driver updates, additions, and changes making more code const and cleaning up some init logic - bus_type constant conversion changes - misc device test functions added - rust miscdevice minor fixup - unused function removals for some drivers - mei driver updates - mhi driver updates - interconnect driver updates - Android binder updates and test infrastructure added - small cdx driver updates - small comedi fixes - small nvmem driver updates - small pps driver updates - some acrn virt driver fixes for printk messages - other small driver updates All of these have been in linux-next with no reported issues" * tag 'char-misc-6.17-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc: (292 commits) binder: Use seq_buf in binder_alloc kunit tests binder: Add copyright notice to new kunit files misc: ti_fpc202: Switch to of_fwnode_handle() bus: moxtet: Use dev_fwnode() pc104: move PC104 option to drivers/Kconfig drivers: virt: acrn: Don't use %pK through printk comedi: fix race between polling and detaching interconnect: qcom: Add Milos interconnect provider driver dt-bindings: interconnect: document the RPMh Network-On-Chip Interconnect in Qualcomm Milos SoC mei: more prints with client prefix mei: bus: use cldev in prints bus: mhi: host: pci_generic: Add Telit FN990B40 modem support bus: mhi: host: Detect events pointing to unexpected TREs bus: mhi: host: pci_generic: Add Foxconn T99W696 modem bus: mhi: host: Use str_true_false() helper bus: mhi: host: pci_generic: Add support for EM929x and set MRU to 32768 for better performance. bus: mhi: host: Fix endianness of BHI vector table bus: mhi: host: pci_generic: Disable runtime PM for QDU100 bus: mhi: host: pci_generic: Fix the modem name of Foxconn T99W640 dt-bindings: interconnect: qcom,msm8998-bwmon: Allow 'nonposted-mmio' ...

comedi: fix race between polling and detaching

2025-07-22T16:47:30+00:00

syzbot reports a use-after-free in comedi in the below link, which is due to comedi gladly removing the allocated async area even though poll requests are still active on the wait_queue_head inside of it. This can cause a use-after-free when the poll entries are later triggered or removed, as the memory for the wait_queue_head has been freed. We need to check there are no tasks queued on any of the subdevices' wait queues before allowing the device to be detached by the `COMEDI_DEVCONFIG` ioctl. Tasks will read-lock `dev->attach_lock` before adding themselves to the subdevice wait queue, so fix the problem in the `COMEDI_DEVCONFIG` ioctl handler by write-locking `dev->attach_lock` before checking that all of the subdevices are safe to be deleted. This includes testing for any sleepers on the subdevices' wait queues. It remains locked until the device has been detached. This requires the `comedi_device_detach()` function to be refactored slightly, moving the bulk of it into new function `comedi_device_detach_locked()`. Note that the refactor of `comedi_device_detach()` results in `comedi_device_cancel_all()` now being called while `dev->attach_lock` is write-locked, which wasn't the case previously, but that does not matter. Thanks to Jens Axboe for diagnosing the problem and co-developing this patch. Cc: stable Fixes: 2f3fdcd7ce93 ("staging: comedi: add rw_semaphore to protect against device detachment") Link: https://lore.kernel.org/all/687bd5fe.a70a0220.693ce.0091.GAE@google.com/ Reported-by: syzbot+01523a0ae5600aef5895@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=01523a0ae5600aef5895 Co-developed-by: Jens Axboe Signed-off-by: Jens Axboe Signed-off-by: Ian Abbott Tested-by: Jens Axboe Link: https://lore.kernel.org/r/20250722155316.27432-1-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman

comedi: Fix initialization of data for instructions that write to subdevice

2025-07-16T13:02:44+00:00

Some Comedi subdevice instruction handlers are known to access instruction data elements beyond the first `insn->n` elements in some cases. The `do_insn_ioctl()` and `do_insnlist_ioctl()` functions allocate at least `MIN_SAMPLES` (16) data elements to deal with this, but they do not initialize all of that. For Comedi instruction codes that write to the subdevice, the first `insn->n` data elements are copied from user-space, but the remaining elements are left uninitialized. That could be a problem if the subdevice instruction handler reads the uninitialized data. Ensure that the first `MIN_SAMPLES` elements are initialized before calling these instruction handlers, filling the uncopied elements with 0. For `do_insnlist_ioctl()`, the same data buffer elements are used for handling a list of instructions, so ensure the first `MIN_SAMPLES` elements are initialized for each instruction that writes to the subdevice. Fixes: ed9eccbe8970 ("Staging: add comedi core") Cc: stable@vger.kernel.org # 5.13+ Signed-off-by: Ian Abbott Link: https://lore.kernel.org/r/20250707161439.88385-1-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman

comedi: Fail COMEDI_INSNLIST ioctl if n_insns is too large

2025-07-16T12:58:54+00:00

The handling of the `COMEDI_INSNLIST` ioctl allocates a kernel buffer to hold the array of `struct comedi_insn`, getting the length from the `n_insns` member of the `struct comedi_insnlist` supplied by the user. The allocation will fail with a WARNING and a stack dump if it is too large. Avoid that by failing with an `-EINVAL` error if the supplied `n_insns` value is unreasonable. Define the limit on the `n_insns` value in the `MAX_INSNS` macro. Set this to the same value as `MAX_SAMPLES` (65536), which is the maximum allowed sum of the values of the member `n` in the array of `struct comedi_insn`, and sensible comedi instructions will have an `n` of at least 1. Reported-by: syzbot+d6995b62e5ac7d79557a@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=d6995b62e5ac7d79557a Fixes: ed9eccbe8970 ("Staging: add comedi core") Tested-by: Ian Abbott Cc: stable@vger.kernel.org # 5.13+ Signed-off-by: Ian Abbott Link: https://lore.kernel.org/r/20250704120405.83028-1-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman