The comedi device attribute functions such as `show_read_buffer_kb()` or `show_write_buffer_kb()` call `comedi_read_subdevice()` or `comedi_write_subdevice()` without acquiring the comedi device's mutex first, although the functions do acquire the mutex afterwards. Change them to acquire the mutex first. This is consistent with most of the comedi device file operation functions (apart from `comedi_read()` and `comedi_write()` which probably need looking at). Despite the use of the mutex, there are still race conditions as the `struct comedi_file_info *info` variable value set before acquiring the mutex could be stale after acquiring the mutex. This problem will be dealt with once reference counting has been implemented for the comedi devices. This patch also adds local variable `struct comedi_device *dev` to the functions to reduce the use of the `info` variable a little bit. Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx> --- drivers/staging/comedi/comedi_fops.c | 76 ++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index f379335..d06b13a 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -276,13 +276,16 @@ static ssize_t show_max_read_buffer_kb(struct device *csdev, struct device_attribute *attr, char *buf) { struct comedi_file_info *info = dev_get_drvdata(csdev); - struct comedi_subdevice *s = comedi_read_subdevice(info); + struct comedi_device *dev; + struct comedi_subdevice *s; unsigned int size = 0; - mutex_lock(&info->device->mutex); + dev = info->device; + mutex_lock(&dev->mutex); + s = comedi_read_subdevice(info); if (s && (s->subdev_flags & SDF_CMD_READ) && s->async) size = s->async->max_bufsize / 1024; - mutex_unlock(&info->device->mutex); + mutex_unlock(&dev->mutex); return snprintf(buf, PAGE_SIZE, "%i\n", size); } @@ -292,7 +295,8 @@ static ssize_t store_max_read_buffer_kb(struct device *csdev, const char *buf, size_t count) { struct comedi_file_info *info = dev_get_drvdata(csdev); - struct comedi_subdevice *s = comedi_read_subdevice(info); + struct comedi_device *dev; + struct comedi_subdevice *s; unsigned int size; int err; @@ -303,12 +307,14 @@ static ssize_t store_max_read_buffer_kb(struct device *csdev, return -EINVAL; size *= 1024; - mutex_lock(&info->device->mutex); + dev = info->device; + mutex_lock(&dev->mutex); + s = comedi_read_subdevice(info); if (s && (s->subdev_flags & SDF_CMD_READ) && s->async) s->async->max_bufsize = size; else err = -EINVAL; - mutex_unlock(&info->device->mutex); + mutex_unlock(&dev->mutex); return err ? err : count; } @@ -317,13 +323,16 @@ static ssize_t show_read_buffer_kb(struct device *csdev, struct device_attribute *attr, char *buf) { struct comedi_file_info *info = dev_get_drvdata(csdev); - struct comedi_subdevice *s = comedi_read_subdevice(info); + struct comedi_device *dev; + struct comedi_subdevice *s; unsigned int size = 0; - mutex_lock(&info->device->mutex); + dev = info->device; + mutex_lock(&dev->mutex); + s = comedi_read_subdevice(info); if (s && (s->subdev_flags & SDF_CMD_READ) && s->async) size = s->async->prealloc_bufsz / 1024; - mutex_unlock(&info->device->mutex); + mutex_unlock(&dev->mutex); return snprintf(buf, PAGE_SIZE, "%i\n", size); } @@ -333,7 +342,8 @@ static ssize_t store_read_buffer_kb(struct device *csdev, const char *buf, size_t count) { struct comedi_file_info *info = dev_get_drvdata(csdev); - struct comedi_subdevice *s = comedi_read_subdevice(info); + struct comedi_device *dev; + struct comedi_subdevice *s; unsigned int size; int err; @@ -344,12 +354,14 @@ static ssize_t store_read_buffer_kb(struct device *csdev, return -EINVAL; size *= 1024; - mutex_lock(&info->device->mutex); + dev = info->device; + mutex_lock(&dev->mutex); + s = comedi_read_subdevice(info); if (s && (s->subdev_flags & SDF_CMD_READ) && s->async) - err = resize_async_buffer(info->device, s, s->async, size); + err = resize_async_buffer(dev, s, s->async, size); else err = -EINVAL; - mutex_unlock(&info->device->mutex); + mutex_unlock(&dev->mutex); return err ? err : count; } @@ -359,13 +371,16 @@ static ssize_t show_max_write_buffer_kb(struct device *csdev, char *buf) { struct comedi_file_info *info = dev_get_drvdata(csdev); - struct comedi_subdevice *s = comedi_write_subdevice(info); + struct comedi_device *dev; + struct comedi_subdevice *s; unsigned int size = 0; - mutex_lock(&info->device->mutex); + dev = info->device; + mutex_lock(&dev->mutex); + s = comedi_write_subdevice(info); if (s && (s->subdev_flags & SDF_CMD_WRITE) && s->async) size = s->async->max_bufsize / 1024; - mutex_unlock(&info->device->mutex); + mutex_unlock(&dev->mutex); return snprintf(buf, PAGE_SIZE, "%i\n", size); } @@ -375,7 +390,8 @@ static ssize_t store_max_write_buffer_kb(struct device *csdev, const char *buf, size_t count) { struct comedi_file_info *info = dev_get_drvdata(csdev); - struct comedi_subdevice *s = comedi_write_subdevice(info); + struct comedi_device *dev; + struct comedi_subdevice *s; unsigned int size; int err; @@ -386,12 +402,14 @@ static ssize_t store_max_write_buffer_kb(struct device *csdev, return -EINVAL; size *= 1024; - mutex_lock(&info->device->mutex); + dev = info->device; + mutex_lock(&dev->mutex); + s = comedi_write_subdevice(info); if (s && (s->subdev_flags & SDF_CMD_WRITE) && s->async) s->async->max_bufsize = size; else err = -EINVAL; - mutex_unlock(&info->device->mutex); + mutex_unlock(&dev->mutex); return err ? err : count; } @@ -400,13 +418,16 @@ static ssize_t show_write_buffer_kb(struct device *csdev, struct device_attribute *attr, char *buf) { struct comedi_file_info *info = dev_get_drvdata(csdev); - struct comedi_subdevice *s = comedi_write_subdevice(info); + struct comedi_device *dev; + struct comedi_subdevice *s; unsigned int size = 0; - mutex_lock(&info->device->mutex); + dev = info->device; + mutex_lock(&dev->mutex); + s = comedi_write_subdevice(info); if (s && (s->subdev_flags & SDF_CMD_WRITE) && s->async) size = s->async->prealloc_bufsz / 1024; - mutex_unlock(&info->device->mutex); + mutex_unlock(&dev->mutex); return snprintf(buf, PAGE_SIZE, "%i\n", size); } @@ -416,7 +437,8 @@ static ssize_t store_write_buffer_kb(struct device *csdev, const char *buf, size_t count) { struct comedi_file_info *info = dev_get_drvdata(csdev); - struct comedi_subdevice *s = comedi_write_subdevice(info); + struct comedi_device *dev; + struct comedi_subdevice *s; unsigned int size; int err; @@ -427,12 +449,14 @@ static ssize_t store_write_buffer_kb(struct device *csdev, return -EINVAL; size *= 1024; - mutex_lock(&info->device->mutex); + dev = info->device; + mutex_lock(&dev->mutex); + s = comedi_write_subdevice(info); if (s && (s->subdev_flags & SDF_CMD_WRITE) && s->async) - err = resize_async_buffer(info->device, s, s->async, size); + err = resize_async_buffer(dev, s, s->async, size); else err = -EINVAL; - mutex_unlock(&info->device->mutex); + mutex_unlock(&dev->mutex); return err ? err : count; } -- 1.8.1.5 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel