From c383e2d6dacf0b6fdd40fbaf044e235cac54a20f Mon Sep 17 00:00:00 2001 From: Ian Abbott Date: Thu, 27 Jun 2013 14:50:58 +0100 Subject: [PATCH] staging: comedi: use a mutex when accessing driver list Low-level comedi drivers registered with the comedi core by `comedi_driver_register()` are linked together into a simple linked list headed by the `comedi_drivers` variable and chained by the `next` member of `struct comedi_driver`. A driver is removed from the list by `comedi_driver_unregister()`. The driver list is iterated through by `comedi_device_attach()` when the `COMEDI_DEVCONFIG` ioctl is used to attach a "legacy" device to a driver, and is also iterated through by `comedi_read()` in "proc.c" when reading "/proc/comedi". There is currently no protection against items being added or removed from the list while it is being iterated. Add a mutex `comedi_drivers_list_lock` to be locked while adding or removing an item on the list, or when iterating through the list. `comedi_driver_unregister()` also checks for and detaches any devices using the driver. This is currently done before unlinking the driver from the list, but it makes more sense to unlink the driver from the list first to prevent `comedi_device_attach()` attempting to use it, so move the unlinking part to the start of the function. Also, in `comedi_device_attach()` hold on to the mutex until we've finished attempting to attach the device to avoid it interfering with the detachment in `comedi_driver_unregister()`. Signed-off-by: Ian Abbott Signed-off-by: Greg Kroah-Hartman --- drivers/staging/comedi/comedi_internal.h | 1 + drivers/staging/comedi/drivers.c | 38 +++++++++++++++--------- drivers/staging/comedi/proc.c | 2 ++ 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/drivers/staging/comedi/comedi_internal.h b/drivers/staging/comedi/comedi_internal.h index d5e03e558b3..fda1a7ba0e1 100644 --- a/drivers/staging/comedi/comedi_internal.h +++ b/drivers/staging/comedi/comedi_internal.h @@ -24,6 +24,7 @@ extern unsigned int comedi_default_buf_maxsize_kb; /* drivers.c */ extern struct comedi_driver *comedi_drivers; +extern struct mutex comedi_drivers_list_lock; int insn_inval(struct comedi_device *, struct comedi_subdevice *, struct comedi_insn *, unsigned int *); diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c index ba5d6d927a4..791a26bd5f6 100644 --- a/drivers/staging/comedi/drivers.c +++ b/drivers/staging/comedi/drivers.c @@ -38,6 +38,7 @@ #include "comedi_internal.h" struct comedi_driver *comedi_drivers; +DEFINE_MUTEX(comedi_drivers_list_lock); int comedi_set_hw_dev(struct comedi_device *dev, struct device *hw_dev) { @@ -453,6 +454,7 @@ int comedi_device_attach(struct comedi_device *dev, struct comedi_devconfig *it) if (dev->attached) return -EBUSY; + mutex_lock(&comedi_drivers_list_lock); for (driv = comedi_drivers; driv; driv = driv->next) { if (!try_module_get(driv->module)) continue; @@ -473,7 +475,8 @@ int comedi_device_attach(struct comedi_device *dev, struct comedi_devconfig *it) comedi_report_boards(driv); module_put(driv->module); } - return -EIO; + ret = -EIO; + goto out; } if (driv->attach == NULL) { /* driver does not support manual configuration */ @@ -481,7 +484,8 @@ int comedi_device_attach(struct comedi_device *dev, struct comedi_devconfig *it) "driver '%s' does not support attach using comedi_config\n", driv->driver_name); module_put(driv->module); - return -ENOSYS; + ret = -ENOSYS; + goto out; } /* initialize dev->driver here so * comedi_error() can be called from attach */ @@ -496,6 +500,8 @@ int comedi_device_attach(struct comedi_device *dev, struct comedi_devconfig *it) module_put(dev->driver->module); } /* On success, the driver module count has been incremented. */ +out: + mutex_unlock(&comedi_drivers_list_lock); return ret; } @@ -552,8 +558,10 @@ EXPORT_SYMBOL_GPL(comedi_auto_unconfig); int comedi_driver_register(struct comedi_driver *driver) { + mutex_lock(&comedi_drivers_list_lock); driver->next = comedi_drivers; comedi_drivers = driver; + mutex_unlock(&comedi_drivers_list_lock); return 0; } @@ -564,6 +572,20 @@ void comedi_driver_unregister(struct comedi_driver *driver) struct comedi_driver *prev; int i; + /* unlink the driver */ + mutex_lock(&comedi_drivers_list_lock); + if (comedi_drivers == driver) { + comedi_drivers = driver->next; + } else { + for (prev = comedi_drivers; prev->next; prev = prev->next) { + if (prev->next == driver) { + prev->next = driver->next; + break; + } + } + } + mutex_unlock(&comedi_drivers_list_lock); + /* check for devices using this driver */ for (i = 0; i < COMEDI_NUM_BOARD_MINORS; i++) { struct comedi_device *dev = comedi_dev_from_minor(i); @@ -581,17 +603,5 @@ void comedi_driver_unregister(struct comedi_driver *driver) } mutex_unlock(&dev->mutex); } - - if (comedi_drivers == driver) { - comedi_drivers = driver->next; - return; - } - - for (prev = comedi_drivers; prev->next; prev = prev->next) { - if (prev->next == driver) { - prev->next = driver->next; - return; - } - } } EXPORT_SYMBOL_GPL(comedi_driver_unregister); diff --git a/drivers/staging/comedi/proc.c b/drivers/staging/comedi/proc.c index 8ee94424bc8..ade00035d3b 100644 --- a/drivers/staging/comedi/proc.c +++ b/drivers/staging/comedi/proc.c @@ -55,6 +55,7 @@ static int comedi_read(struct seq_file *m, void *v) if (!devices_q) seq_puts(m, "no devices\n"); + mutex_lock(&comedi_drivers_list_lock); for (driv = comedi_drivers; driv; driv = driv->next) { seq_printf(m, "%s:\n", driv->driver_name); for (i = 0; i < driv->num_names; i++) @@ -65,6 +66,7 @@ static int comedi_read(struct seq_file *m, void *v) if (!driv->num_names) seq_printf(m, " %s\n", driv->driver_name); } + mutex_unlock(&comedi_drivers_list_lock); return 0; } -- 2.46.0