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 <abbotti@mev.co.uk>
Link: https://patch.msgid.link/20251027153748.4569-2-abbotti@mev.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Ian Abbott and committed by

Greg Kroah-Hartman 5 months ago d1b3b9c7 51495254

+147 -7

2 changed files

expand all

drivers

comedi

kcomedilib

kcomedilib_main.c

include

linux

comedi

comedilib.h

+115 -5

drivers/comedi/kcomedilib/kcomedilib_main.c

··· 15 15 #include <linux/fcntl.h> 16 16 #include <linux/mm.h> 17 17 #include <linux/io.h> 18 + #include <linux/bitmap.h> 18 19 19 20 #include <linux/comedi.h> 20 21 #include <linux/comedi/comedidev.h> ··· 25 24 MODULE_DESCRIPTION("Comedi kernel library"); 26 25 MODULE_LICENSE("GPL"); 27 26 28 - struct comedi_device *comedi_open(const char *filename) 27 + static DEFINE_MUTEX(kcomedilib_to_from_lock); 28 + 29 + /* 30 + * Row index is the "to" node, column index is the "from" node, element value 31 + * is the number of links from the "from" node to the "to" node. 32 + */ 33 + static unsigned char 34 + kcomedilib_to_from[COMEDI_NUM_BOARD_MINORS][COMEDI_NUM_BOARD_MINORS]; 35 + 36 + static bool kcomedilib_set_link_from_to(unsigned int from, unsigned int to) 37 + { 38 + DECLARE_BITMAP(destinations[2], COMEDI_NUM_BOARD_MINORS); 39 + unsigned int cur = 0; 40 + bool okay = true; 41 + 42 + /* 43 + * Allow "from" node to be out of range (no loop checking), 44 + * but require "to" node to be in range. 45 + */ 46 + if (to >= COMEDI_NUM_BOARD_MINORS) 47 + return false; 48 + if (from >= COMEDI_NUM_BOARD_MINORS) 49 + return true; 50 + 51 + /* 52 + * Check that kcomedilib_to_from[to][from] can be made non-zero 53 + * without creating a loop. 54 + * 55 + * Termination of the loop-testing code relies on the assumption that 56 + * kcomedilib_to_from[][] does not contain any loops. 57 + * 58 + * Start with a set destinations set containing "from" as the only 59 + * element and work backwards looking for loops. 60 + */ 61 + bitmap_zero(destinations[cur], COMEDI_NUM_BOARD_MINORS); 62 + set_bit(from, destinations[cur]); 63 + mutex_lock(&kcomedilib_to_from_lock); 64 + do { 65 + unsigned int next = 1 - cur; 66 + unsigned int t = 0; 67 + 68 + if (test_bit(to, destinations[cur])) { 69 + /* Loop detected. */ 70 + okay = false; 71 + break; 72 + } 73 + /* Create next set of destinations. */ 74 + bitmap_zero(destinations[next], COMEDI_NUM_BOARD_MINORS); 75 + while ((t = find_next_bit(destinations[cur], 76 + COMEDI_NUM_BOARD_MINORS, 77 + t)) < COMEDI_NUM_BOARD_MINORS) { 78 + unsigned int f; 79 + 80 + for (f = 0; f < COMEDI_NUM_BOARD_MINORS; f++) { 81 + if (kcomedilib_to_from[t][f]) 82 + set_bit(f, destinations[next]); 83 + } 84 + t++; 85 + } 86 + cur = next; 87 + } while (!bitmap_empty(destinations[cur], COMEDI_NUM_BOARD_MINORS)); 88 + if (okay) { 89 + /* Allow a maximum of 255 links from "from" to "to". */ 90 + if (kcomedilib_to_from[to][from] < 255) 91 + kcomedilib_to_from[to][from]++; 92 + else 93 + okay = false; 94 + } 95 + mutex_unlock(&kcomedilib_to_from_lock); 96 + return okay; 97 + } 98 + 99 + static void kcomedilib_clear_link_from_to(unsigned int from, unsigned int to) 100 + { 101 + if (to < COMEDI_NUM_BOARD_MINORS && from < COMEDI_NUM_BOARD_MINORS) { 102 + mutex_lock(&kcomedilib_to_from_lock); 103 + if (kcomedilib_to_from[to][from]) 104 + kcomedilib_to_from[to][from]--; 105 + mutex_unlock(&kcomedilib_to_from_lock); 106 + } 107 + } 108 + 109 + /** 110 + * comedi_open_from() - Open a COMEDI device from the kernel with loop checks 111 + * @filename: Fake pathname of the form "/dev/comediN". 112 + * @from: Device number it is being opened from (if in range). 113 + * 114 + * Converts @filename to a COMEDI device number and "opens" it if it exists 115 + * and is attached to a low-level COMEDI driver. 116 + * 117 + * If @from is in range, refuse to open the device if doing so would form a 118 + * loop of devices opening each other. There is also a limit of 255 on the 119 + * number of concurrent opens from one device to another. 120 + * 121 + * Return: A pointer to the COMEDI device on success. 122 + * Return %NULL on failure. 123 + */ 124 + struct comedi_device *comedi_open_from(const char *filename, int from) 29 125 { 30 126 struct comedi_device *dev, *retval = NULL; 31 127 unsigned int minor; ··· 141 43 return NULL; 142 44 143 45 down_read(&dev->attach_lock); 144 - if (dev->attached) 46 + if (dev->attached && kcomedilib_set_link_from_to(from, minor)) 145 47 retval = dev; 146 48 else 147 49 retval = NULL; ··· 152 54 153 55 return retval; 154 56 } 155 - EXPORT_SYMBOL_GPL(comedi_open); 57 + EXPORT_SYMBOL_GPL(comedi_open_from); 156 58 157 - int comedi_close(struct comedi_device *dev) 59 + /** 60 + * comedi_close_from() - Close a COMEDI device from the kernel with loop checks 61 + * @dev: COMEDI device. 62 + * @from: Device number it was opened from (if in range). 63 + * 64 + * Closes a COMEDI device previously opened by comedi_open_from(). 65 + * 66 + * If @from is in range, it should be match the one used by comedi_open_from(). 67 + * 68 + * Returns: 0 69 + */ 70 + int comedi_close_from(struct comedi_device *dev, int from) 158 71 { 72 + kcomedilib_clear_link_from_to(from, dev->minor); 159 73 comedi_dev_put(dev); 160 74 return 0; 161 75 } 162 - EXPORT_SYMBOL_GPL(comedi_close); 76 + EXPORT_SYMBOL_GPL(comedi_close_from); 163 77 164 78 static int comedi_do_insn(struct comedi_device *dev, 165 79 struct comedi_insn *insn,

+32 -2

include/linux/comedi/comedilib.h

··· 10 10 #ifndef _LINUX_COMEDILIB_H 11 11 #define _LINUX_COMEDILIB_H 12 12 13 - struct comedi_device *comedi_open(const char *path); 14 - int comedi_close(struct comedi_device *dev); 13 + struct comedi_device *comedi_open_from(const char *path, int from); 14 + 15 + /** 16 + * comedi_open() - Open a COMEDI device from the kernel 17 + * @filename: Fake pathname of the form "/dev/comediN". 18 + * 19 + * Converts @filename to a COMEDI device number and "opens" it if it exists 20 + * and is attached to a low-level COMEDI driver. 21 + * 22 + * Return: A pointer to the COMEDI device on success. 23 + * Return %NULL on failure. 24 + */ 25 + static inline struct comedi_device *comedi_open(const char *path) 26 + { 27 + return comedi_open_from(path, -1); 28 + } 29 + 30 + int comedi_close_from(struct comedi_device *dev, int from); 31 + 32 + /** 33 + * comedi_close() - Close a COMEDI device from the kernel 34 + * @dev: COMEDI device. 35 + * 36 + * Closes a COMEDI device previously opened by comedi_open(). 37 + * 38 + * Returns: 0 39 + */ 40 + static inline int comedi_close(struct comedi_device *dev) 41 + { 42 + return comedi_close_from(dev, -1); 43 + } 44 + 15 45 int comedi_dio_get_config(struct comedi_device *dev, unsigned int subdev, 16 46 unsigned int chan, unsigned int *io); 17 47 int comedi_dio_config(struct comedi_device *dev, unsigned int subdev,

Configure Feed

Configure Feed