[PATCH 27/37] staging: comedi: get mutex before subdevice in attribute functions

Ian Abbott abbotti at mev.co.uk
Thu Apr 4 13:59:08 UTC 2013


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 at mev.co.uk>
---
 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




More information about the devel mailing list