From 4e1da516debbe6a573ffa0392e2809d180d0575c Mon Sep 17 00:00:00 2001 From: Ian Abbott Date: Thu, 23 Oct 2025 14:28:18 +0100 Subject: comedi: Add reference counting for Comedi command handling 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 --- include/linux/comedi/comedidev.h | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'include/linux/comedi') diff --git a/include/linux/comedi/comedidev.h b/include/linux/comedi/comedidev.h index 4cb0400ad616..35fdc41845ce 100644 --- a/include/linux/comedi/comedidev.h +++ b/include/linux/comedi/comedidev.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #define COMEDI_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c)) @@ -272,6 +273,8 @@ struct comedi_buf_map { * @events: Bit-vector of events that have occurred. * @cmd: Details of comedi command in progress. * @wait_head: Task wait queue for file reader or writer. + * @run_complete: "run complete" completion event. + * @run_active: "run active" reference counter. * @cb_mask: Bit-vector of events that should wake waiting tasks. * @inttrig: Software trigger function for command, or NULL. * @@ -357,6 +360,8 @@ struct comedi_async { unsigned int events; struct comedi_cmd cmd; wait_queue_head_t wait_head; + struct completion run_complete; + refcount_t run_active; unsigned int cb_mask; int (*inttrig)(struct comedi_device *dev, struct comedi_subdevice *s, unsigned int x); @@ -584,6 +589,8 @@ struct comedi_device *comedi_dev_get_from_minor(unsigned int minor); int comedi_dev_put(struct comedi_device *dev); bool comedi_is_subdevice_running(struct comedi_subdevice *s); +bool comedi_get_is_subdevice_running(struct comedi_subdevice *s); +void comedi_put_is_subdevice_running(struct comedi_subdevice *s); void *comedi_alloc_spriv(struct comedi_subdevice *s, size_t size); void comedi_set_spriv_auto_free(struct comedi_subdevice *s); -- cgit v1.2.3 From d1b3b9c70e11cb4f40b4e41a4dc1503b9a3c0109 Mon Sep 17 00:00:00 2001 From: Ian Abbott Date: Mon, 27 Oct 2025 15:25:02 +0000 Subject: comedi: kcomedilib: Add loop checking variants of open and close Add `comedi_open_from(path, from)` and `comedi_close_from(dev, from)` as variants of the existing `comedi_from(path)` and `comedi_close(dev)`. The additional `from` parameter is a minor device number that tells the function that the COMEDI device is being opened or closed from another COMEDI device if the value is in the range [0, `COMEDI_NUM_BOARD_MINORS`-1]. In that case the function will refuse to open the device if it would lead to a chain of devices opening each other. (It will also impose a limit on the number of simultaneous opens from one device to another because we need to count those.) The new functions are intended to be used by the "comedi_bond" driver, which is the only driver that uses the existing `comedi_open()` and `comedi_close()` functions. The new functions will be used to avoid some possible deadlock situations. Replace the existing, exported `comedi_open()` and `comedi_close()` functions with inline wrapper functions that call the newly exported `comedi_open_from()` and `comedi_close_from()` functions. Signed-off-by: Ian Abbott Link: https://patch.msgid.link/20251027153748.4569-2-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman --- drivers/comedi/kcomedilib/kcomedilib_main.c | 120 ++++++++++++++++++++++++++-- include/linux/comedi/comedilib.h | 34 +++++++- 2 files changed, 147 insertions(+), 7 deletions(-) (limited to 'include/linux/comedi') diff --git a/drivers/comedi/kcomedilib/kcomedilib_main.c b/drivers/comedi/kcomedilib/kcomedilib_main.c index 43fbe1a63b14..baa9eaaf97d4 100644 --- a/drivers/comedi/kcomedilib/kcomedilib_main.c +++ b/drivers/comedi/kcomedilib/kcomedilib_main.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -24,7 +25,104 @@ MODULE_AUTHOR("David Schleef "); MODULE_DESCRIPTION("Comedi kernel library"); MODULE_LICENSE("GPL"); -struct comedi_device *comedi_open(const char *filename) +static DEFINE_MUTEX(kcomedilib_to_from_lock); + +/* + * Row index is the "to" node, column index is the "from" node, element value + * is the number of links from the "from" node to the "to" node. + */ +static unsigned char + kcomedilib_to_from[COMEDI_NUM_BOARD_MINORS][COMEDI_NUM_BOARD_MINORS]; + +static bool kcomedilib_set_link_from_to(unsigned int from, unsigned int to) +{ + DECLARE_BITMAP(destinations[2], COMEDI_NUM_BOARD_MINORS); + unsigned int cur = 0; + bool okay = true; + + /* + * Allow "from" node to be out of range (no loop checking), + * but require "to" node to be in range. + */ + if (to >= COMEDI_NUM_BOARD_MINORS) + return false; + if (from >= COMEDI_NUM_BOARD_MINORS) + return true; + + /* + * Check that kcomedilib_to_from[to][from] can be made non-zero + * without creating a loop. + * + * Termination of the loop-testing code relies on the assumption that + * kcomedilib_to_from[][] does not contain any loops. + * + * Start with a set destinations set containing "from" as the only + * element and work backwards looking for loops. + */ + bitmap_zero(destinations[cur], COMEDI_NUM_BOARD_MINORS); + set_bit(from, destinations[cur]); + mutex_lock(&kcomedilib_to_from_lock); + do { + unsigned int next = 1 - cur; + unsigned int t = 0; + + if (test_bit(to, destinations[cur])) { + /* Loop detected. */ + okay = false; + break; + } + /* Create next set of destinations. */ + bitmap_zero(destinations[next], COMEDI_NUM_BOARD_MINORS); + while ((t = find_next_bit(destinations[cur], + COMEDI_NUM_BOARD_MINORS, + t)) < COMEDI_NUM_BOARD_MINORS) { + unsigned int f; + + for (f = 0; f < COMEDI_NUM_BOARD_MINORS; f++) { + if (kcomedilib_to_from[t][f]) + set_bit(f, destinations[next]); + } + t++; + } + cur = next; + } while (!bitmap_empty(destinations[cur], COMEDI_NUM_BOARD_MINORS)); + if (okay) { + /* Allow a maximum of 255 links from "from" to "to". */ + if (kcomedilib_to_from[to][from] < 255) + kcomedilib_to_from[to][from]++; + else + okay = false; + } + mutex_unlock(&kcomedilib_to_from_lock); + return okay; +} + +static void kcomedilib_clear_link_from_to(unsigned int from, unsigned int to) +{ + if (to < COMEDI_NUM_BOARD_MINORS && from < COMEDI_NUM_BOARD_MINORS) { + mutex_lock(&kcomedilib_to_from_lock); + if (kcomedilib_to_from[to][from]) + kcomedilib_to_from[to][from]--; + mutex_unlock(&kcomedilib_to_from_lock); + } +} + +/** + * comedi_open_from() - Open a COMEDI device from the kernel with loop checks + * @filename: Fake pathname of the form "/dev/comediN". + * @from: Device number it is being opened from (if in range). + * + * Converts @filename to a COMEDI device number and "opens" it if it exists + * and is attached to a low-level COMEDI driver. + * + * If @from is in range, refuse to open the device if doing so would form a + * loop of devices opening each other. There is also a limit of 255 on the + * number of concurrent opens from one device to another. + * + * Return: A pointer to the COMEDI device on success. + * Return %NULL on failure. + */ +struct comedi_device *comedi_open_from(const char *filename, int from) { struct comedi_device *dev, *retval = NULL; unsigned int minor; @@ -43,7 +141,7 @@ struct comedi_device *comedi_open(const char *filename) return NULL; down_read(&dev->attach_lock); - if (dev->attached) + if (dev->attached && kcomedilib_set_link_from_to(from, minor)) retval = dev; else retval = NULL; @@ -54,14 +152,26 @@ struct comedi_device *comedi_open(const char *filename) return retval; } -EXPORT_SYMBOL_GPL(comedi_open); +EXPORT_SYMBOL_GPL(comedi_open_from); -int comedi_close(struct comedi_device *dev) +/** + * comedi_close_from() - Close a COMEDI device from the kernel with loop checks + * @dev: COMEDI device. + * @from: Device number it was opened from (if in range). + * + * Closes a COMEDI device previously opened by comedi_open_from(). + * + * If @from is in range, it should be match the one used by comedi_open_from(). + * + * Returns: 0 + */ +int comedi_close_from(struct comedi_device *dev, int from) { + kcomedilib_clear_link_from_to(from, dev->minor); comedi_dev_put(dev); return 0; } -EXPORT_SYMBOL_GPL(comedi_close); +EXPORT_SYMBOL_GPL(comedi_close_from); static int comedi_do_insn(struct comedi_device *dev, struct comedi_insn *insn, diff --git a/include/linux/comedi/comedilib.h b/include/linux/comedi/comedilib.h index 0223c9cd9215..1f2b22b383cc 100644 --- a/include/linux/comedi/comedilib.h +++ b/include/linux/comedi/comedilib.h @@ -10,8 +10,38 @@ #ifndef _LINUX_COMEDILIB_H #define _LINUX_COMEDILIB_H -struct comedi_device *comedi_open(const char *path); -int comedi_close(struct comedi_device *dev); +struct comedi_device *comedi_open_from(const char *path, int from); + +/** + * comedi_open() - Open a COMEDI device from the kernel + * @filename: Fake pathname of the form "/dev/comediN". + * + * Converts @filename to a COMEDI device number and "opens" it if it exists + * and is attached to a low-level COMEDI driver. + * + * Return: A pointer to the COMEDI device on success. + * Return %NULL on failure. + */ +static inline struct comedi_device *comedi_open(const char *path) +{ + return comedi_open_from(path, -1); +} + +int comedi_close_from(struct comedi_device *dev, int from); + +/** + * comedi_close() - Close a COMEDI device from the kernel + * @dev: COMEDI device. + * + * Closes a COMEDI device previously opened by comedi_open(). + * + * Returns: 0 + */ +static inline int comedi_close(struct comedi_device *dev) +{ + return comedi_close_from(dev, -1); +} + int comedi_dio_get_config(struct comedi_device *dev, unsigned int subdev, unsigned int chan, unsigned int *io); int comedi_dio_config(struct comedi_device *dev, unsigned int subdev, -- cgit v1.2.3 From 578d62a2e51614ea117ccf05fce6aaa6257dfd60 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Sun, 14 Dec 2025 12:27:27 -0800 Subject: comedi: comedi_8254: correct kernel-doc warnings Correct typos in 3 struct member descriptions to eliminate all kernel-doc warnings in comedi_8254.h: Warning: include/linux/comedi/comedi_8254.h:112 struct member 'next_div' not described in 'comedi_8254' Warning: include/linux/comedi/comedi_8254.h:112 struct member 'clock_src' not described in 'comedi_8254' Warning: include/linux/comedi/comedi_8254.h:112 struct member 'gate_src' not described in 'comedi_8254' Signed-off-by: Randy Dunlap Reviewed-by: Ian Abbott Link: https://patch.msgid.link/20251214202727.2215461-1-rdunlap@infradead.org Signed-off-by: Greg Kroah-Hartman --- include/linux/comedi/comedi_8254.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'include/linux/comedi') diff --git a/include/linux/comedi/comedi_8254.h b/include/linux/comedi/comedi_8254.h index d527f04400df..91f5861061ec 100644 --- a/include/linux/comedi/comedi_8254.h +++ b/include/linux/comedi/comedi_8254.h @@ -83,11 +83,11 @@ typedef unsigned int comedi_8254_iocb_fn(struct comedi_8254 *i8254, int dir, * @divisor: divisor for single counter * @divisor1: divisor loaded into first cascaded counter * @divisor2: divisor loaded into second cascaded counter - * #next_div: next divisor for single counter + * @next_div: next divisor for single counter * @next_div1: next divisor to use for first cascaded counter * @next_div2: next divisor to use for second cascaded counter - * @clock_src; current clock source for each counter (driver specific) - * @gate_src; current gate source for each counter (driver specific) + * @clock_src: current clock source for each counter (driver specific) + * @gate_src: current gate source for each counter (driver specific) * @busy: flags used to indicate that a counter is "busy" * @insn_config: driver specific (*insn_config) callback */ -- cgit v1.2.3