All of lore.kernel.org
 help / color / mirror / Atom feed
* [1/3] rio500: refuse more than one device at a time
@ 2019-04-30 14:23 ` Oliver Neukum
  0 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2019-04-30 14:23 UTC (permalink / raw)
  To: gregKH, miquel, linux-usb; +Cc: Oliver Neukum

This driver is using a global variable. It cannot handle more than
one device at a time. The issue has been exisying since the dawn
of the driver.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Reported-by: syzbot+35f04d136fc975a70da4@syzkaller.appspotmail.com
---
 drivers/usb/misc/rio500.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c
index 13e4889bc34f..a4b6fbea975f 100644
--- a/drivers/usb/misc/rio500.c
+++ b/drivers/usb/misc/rio500.c
@@ -449,7 +449,12 @@ static int probe_rio(struct usb_interface *intf,
 	struct rio_usb_data *rio = &rio_instance;
 	int retval;
 
-	dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
+	if (rio->present) {
+		dev_info(&intf->dev, "Second USB Rio at address %d refused\n", dev->devnum);
+		return -EBUSY;
+	} else {
+		dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
+	}
 
 	retval = usb_register_dev(intf, &usb_rio_class);
 	if (retval) {

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 1/3] rio500: refuse more than one device at a time
@ 2019-04-30 14:23 ` Oliver Neukum
  0 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2019-04-30 14:23 UTC (permalink / raw)
  To: gregKH, miquel, linux-usb; +Cc: Oliver Neukum

This driver is using a global variable. It cannot handle more than
one device at a time. The issue has been exisying since the dawn
of the driver.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Reported-by: syzbot+35f04d136fc975a70da4@syzkaller.appspotmail.com
---
 drivers/usb/misc/rio500.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c
index 13e4889bc34f..a4b6fbea975f 100644
--- a/drivers/usb/misc/rio500.c
+++ b/drivers/usb/misc/rio500.c
@@ -449,7 +449,12 @@ static int probe_rio(struct usb_interface *intf,
 	struct rio_usb_data *rio = &rio_instance;
 	int retval;
 
-	dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
+	if (rio->present) {
+		dev_info(&intf->dev, "Second USB Rio at address %d refused\n", dev->devnum);
+		return -EBUSY;
+	} else {
+		dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
+	}
 
 	retval = usb_register_dev(intf, &usb_rio_class);
 	if (retval) {
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [2/3] rio500: fix memeory leak in close after disconnect
@ 2019-04-30 14:23 ` Oliver Neukum
  0 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2019-04-30 14:23 UTC (permalink / raw)
  To: gregKH, miquel, linux-usb; +Cc: Oliver Neukum

If a disconnected device is closed, rio_close() must free
the buffers.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/misc/rio500.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c
index a4b6fbea975f..20c3eb0af7ad 100644
--- a/drivers/usb/misc/rio500.c
+++ b/drivers/usb/misc/rio500.c
@@ -86,9 +86,22 @@ static int close_rio(struct inode *inode, struct file *file)
 {
 	struct rio_usb_data *rio = &rio_instance;
 
-	rio->isopen = 0;
+	/* against disconnect() */
+	mutex_lock(&rio500_mutex);
+	mutex_lock(&(rio->lock));
 
-	dev_info(&rio->rio_dev->dev, "Rio closed.\n");
+	rio->isopen = 0;
+	if (!rio->present) {
+		/* cleanup has been delayed */
+		kfree(rio->ibuf);
+		kfree(rio->obuf);
+		rio->ibuf = NULL;
+		rio->obuf = NULL;
+	} else {
+		dev_info(&rio->rio_dev->dev, "Rio closed.\n");
+	}
+	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 	return 0;
 }
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 2/3] rio500: fix memeory leak in close after disconnect
@ 2019-04-30 14:23 ` Oliver Neukum
  0 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2019-04-30 14:23 UTC (permalink / raw)
  To: gregKH, miquel, linux-usb; +Cc: Oliver Neukum

If a disconnected device is closed, rio_close() must free
the buffers.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/misc/rio500.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c
index a4b6fbea975f..20c3eb0af7ad 100644
--- a/drivers/usb/misc/rio500.c
+++ b/drivers/usb/misc/rio500.c
@@ -86,9 +86,22 @@ static int close_rio(struct inode *inode, struct file *file)
 {
 	struct rio_usb_data *rio = &rio_instance;
 
-	rio->isopen = 0;
+	/* against disconnect() */
+	mutex_lock(&rio500_mutex);
+	mutex_lock(&(rio->lock));
 
-	dev_info(&rio->rio_dev->dev, "Rio closed.\n");
+	rio->isopen = 0;
+	if (!rio->present) {
+		/* cleanup has been delayed */
+		kfree(rio->ibuf);
+		kfree(rio->obuf);
+		rio->ibuf = NULL;
+		rio->obuf = NULL;
+	} else {
+		dev_info(&rio->rio_dev->dev, "Rio closed.\n");
+	}
+	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 	return 0;
 }
 
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [3/3] rio500: simplify locking
@ 2019-04-30 14:23 ` Oliver Neukum
  0 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2019-04-30 14:23 UTC (permalink / raw)
  To: gregKH, miquel, linux-usb; +Cc: Oliver Neukum

Admitting that there can be only one device allows us
to drop any pretense about locking one device or
a table of devices.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/misc/rio500.c | 43 ++++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c
index 20c3eb0af7ad..5110eaeda8b4 100644
--- a/drivers/usb/misc/rio500.c
+++ b/drivers/usb/misc/rio500.c
@@ -51,7 +51,6 @@ struct rio_usb_data {
         char *obuf, *ibuf;              /* transfer buffers */
         char bulk_in_ep, bulk_out_ep;   /* Endpoint assignments */
         wait_queue_head_t wait_q;       /* for timeouts */
-	struct mutex lock;          /* general race avoidance */
 };
 
 static DEFINE_MUTEX(rio500_mutex);
@@ -63,10 +62,8 @@ static int open_rio(struct inode *inode, struct file *file)
 
 	/* against disconnect() */
 	mutex_lock(&rio500_mutex);
-	mutex_lock(&(rio->lock));
 
 	if (rio->isopen || !rio->present) {
-		mutex_unlock(&(rio->lock));
 		mutex_unlock(&rio500_mutex);
 		return -EBUSY;
 	}
@@ -74,7 +71,6 @@ static int open_rio(struct inode *inode, struct file *file)
 
 	init_waitqueue_head(&rio->wait_q);
 
-	mutex_unlock(&(rio->lock));
 
 	dev_info(&rio->rio_dev->dev, "Rio opened.\n");
 	mutex_unlock(&rio500_mutex);
@@ -88,7 +84,6 @@ static int close_rio(struct inode *inode, struct file *file)
 
 	/* against disconnect() */
 	mutex_lock(&rio500_mutex);
-	mutex_lock(&(rio->lock));
 
 	rio->isopen = 0;
 	if (!rio->present) {
@@ -100,7 +95,6 @@ static int close_rio(struct inode *inode, struct file *file)
 	} else {
 		dev_info(&rio->rio_dev->dev, "Rio closed.\n");
 	}
-	mutex_unlock(&(rio->lock));
 	mutex_unlock(&rio500_mutex);
 	return 0;
 }
@@ -115,7 +109,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg)
 	int retries;
 	int retval=0;
 
-	mutex_lock(&(rio->lock));
+	mutex_lock(&rio500_mutex);
         /* Sanity check to make sure rio is connected, powered, etc */
         if (rio->present == 0 || rio->rio_dev == NULL) {
 		retval = -ENODEV;
@@ -259,7 +253,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg)
 
 
 err_out:
-	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 	return retval;
 }
 
@@ -279,12 +273,12 @@ write_rio(struct file *file, const char __user *buffer,
 	int errn = 0;
 	int intr;
 
-	intr = mutex_lock_interruptible(&(rio->lock));
+	intr = mutex_lock_interruptible(&rio500_mutex);
 	if (intr)
 		return -EINTR;
         /* Sanity check to make sure rio is connected, powered, etc */
         if (rio->present == 0 || rio->rio_dev == NULL) {
-		mutex_unlock(&(rio->lock));
+		mutex_unlock(&rio500_mutex);
 		return -ENODEV;
 	}
 
@@ -307,7 +301,7 @@ write_rio(struct file *file, const char __user *buffer,
 				goto error;
 			}
 			if (signal_pending(current)) {
-				mutex_unlock(&(rio->lock));
+				mutex_unlock(&rio500_mutex);
 				return bytes_written ? bytes_written : -EINTR;
 			}
 
@@ -345,12 +339,12 @@ write_rio(struct file *file, const char __user *buffer,
 		buffer += copy_size;
 	} while (count > 0);
 
-	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 
 	return bytes_written ? bytes_written : -EIO;
 
 error:
-	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 	return errn;
 }
 
@@ -367,12 +361,12 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 	char *ibuf;
 	int intr;
 
-	intr = mutex_lock_interruptible(&(rio->lock));
+	intr = mutex_lock_interruptible(&rio500_mutex);
 	if (intr)
 		return -EINTR;
 	/* Sanity check to make sure rio is connected, powered, etc */
         if (rio->present == 0 || rio->rio_dev == NULL) {
-		mutex_unlock(&(rio->lock));
+		mutex_unlock(&rio500_mutex);
 		return -ENODEV;
 	}
 
@@ -383,11 +377,11 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 
 	while (count > 0) {
 		if (signal_pending(current)) {
-			mutex_unlock(&(rio->lock));
+			mutex_unlock(&rio500_mutex);
 			return read_count ? read_count : -EINTR;
 		}
 		if (!rio->rio_dev) {
-			mutex_unlock(&(rio->lock));
+			mutex_unlock(&rio500_mutex);
 			return -ENODEV;
 		}
 		this_read = (count >= IBUF_SIZE) ? IBUF_SIZE : count;
@@ -405,7 +399,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 			count = this_read = partial;
 		} else if (result == -ETIMEDOUT || result == 15) {	/* FIXME: 15 ??? */
 			if (!maxretry--) {
-				mutex_unlock(&(rio->lock));
+				mutex_unlock(&rio500_mutex);
 				dev_err(&rio->rio_dev->dev,
 					"read_rio: maxretry timeout\n");
 				return -ETIME;
@@ -415,19 +409,19 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 			finish_wait(&rio->wait_q, &wait);
 			continue;
 		} else if (result != -EREMOTEIO) {
-			mutex_unlock(&(rio->lock));
+			mutex_unlock(&rio500_mutex);
 			dev_err(&rio->rio_dev->dev,
 				"Read Whoops - result:%d partial:%u this_read:%u\n",
 				result, partial, this_read);
 			return -EIO;
 		} else {
-			mutex_unlock(&(rio->lock));
+			mutex_unlock(&rio500_mutex);
 			return (0);
 		}
 
 		if (this_read) {
 			if (copy_to_user(buffer, ibuf, this_read)) {
-				mutex_unlock(&(rio->lock));
+				mutex_unlock(&rio500_mutex);
 				return -EFAULT;
 			}
 			count -= this_read;
@@ -435,7 +429,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 			buffer += this_read;
 		}
 	}
-	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 	return read_count;
 }
 
@@ -495,8 +489,6 @@ static int probe_rio(struct usb_interface *intf,
 	}
 	dev_dbg(&intf->dev, "ibuf address:%p\n", rio->ibuf);
 
-	mutex_init(&(rio->lock));
-
 	usb_set_intfdata (intf, rio);
 	rio->present = 1;
 
@@ -512,12 +504,10 @@ static void disconnect_rio(struct usb_interface *intf)
 	if (rio) {
 		usb_deregister_dev(intf, &usb_rio_class);
 
-		mutex_lock(&(rio->lock));
 		if (rio->isopen) {
 			rio->isopen = 0;
 			/* better let it finish - the release will do whats needed */
 			rio->rio_dev = NULL;
-			mutex_unlock(&(rio->lock));
 			mutex_unlock(&rio500_mutex);
 			return;
 		}
@@ -529,7 +519,6 @@ static void disconnect_rio(struct usb_interface *intf)
 		dev_info(&intf->dev, "USB Rio disconnected.\n");
 
 		rio->present = 0;
-		mutex_unlock(&(rio->lock));
 	}
 	mutex_unlock(&rio500_mutex);
 }

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 3/3] rio500: simplify locking
@ 2019-04-30 14:23 ` Oliver Neukum
  0 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2019-04-30 14:23 UTC (permalink / raw)
  To: gregKH, miquel, linux-usb; +Cc: Oliver Neukum

Admitting that there can be only one device allows us
to drop any pretense about locking one device or
a table of devices.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/misc/rio500.c | 43 ++++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c
index 20c3eb0af7ad..5110eaeda8b4 100644
--- a/drivers/usb/misc/rio500.c
+++ b/drivers/usb/misc/rio500.c
@@ -51,7 +51,6 @@ struct rio_usb_data {
         char *obuf, *ibuf;              /* transfer buffers */
         char bulk_in_ep, bulk_out_ep;   /* Endpoint assignments */
         wait_queue_head_t wait_q;       /* for timeouts */
-	struct mutex lock;          /* general race avoidance */
 };
 
 static DEFINE_MUTEX(rio500_mutex);
@@ -63,10 +62,8 @@ static int open_rio(struct inode *inode, struct file *file)
 
 	/* against disconnect() */
 	mutex_lock(&rio500_mutex);
-	mutex_lock(&(rio->lock));
 
 	if (rio->isopen || !rio->present) {
-		mutex_unlock(&(rio->lock));
 		mutex_unlock(&rio500_mutex);
 		return -EBUSY;
 	}
@@ -74,7 +71,6 @@ static int open_rio(struct inode *inode, struct file *file)
 
 	init_waitqueue_head(&rio->wait_q);
 
-	mutex_unlock(&(rio->lock));
 
 	dev_info(&rio->rio_dev->dev, "Rio opened.\n");
 	mutex_unlock(&rio500_mutex);
@@ -88,7 +84,6 @@ static int close_rio(struct inode *inode, struct file *file)
 
 	/* against disconnect() */
 	mutex_lock(&rio500_mutex);
-	mutex_lock(&(rio->lock));
 
 	rio->isopen = 0;
 	if (!rio->present) {
@@ -100,7 +95,6 @@ static int close_rio(struct inode *inode, struct file *file)
 	} else {
 		dev_info(&rio->rio_dev->dev, "Rio closed.\n");
 	}
-	mutex_unlock(&(rio->lock));
 	mutex_unlock(&rio500_mutex);
 	return 0;
 }
@@ -115,7 +109,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg)
 	int retries;
 	int retval=0;
 
-	mutex_lock(&(rio->lock));
+	mutex_lock(&rio500_mutex);
         /* Sanity check to make sure rio is connected, powered, etc */
         if (rio->present == 0 || rio->rio_dev == NULL) {
 		retval = -ENODEV;
@@ -259,7 +253,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg)
 
 
 err_out:
-	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 	return retval;
 }
 
@@ -279,12 +273,12 @@ write_rio(struct file *file, const char __user *buffer,
 	int errn = 0;
 	int intr;
 
-	intr = mutex_lock_interruptible(&(rio->lock));
+	intr = mutex_lock_interruptible(&rio500_mutex);
 	if (intr)
 		return -EINTR;
         /* Sanity check to make sure rio is connected, powered, etc */
         if (rio->present == 0 || rio->rio_dev == NULL) {
-		mutex_unlock(&(rio->lock));
+		mutex_unlock(&rio500_mutex);
 		return -ENODEV;
 	}
 
@@ -307,7 +301,7 @@ write_rio(struct file *file, const char __user *buffer,
 				goto error;
 			}
 			if (signal_pending(current)) {
-				mutex_unlock(&(rio->lock));
+				mutex_unlock(&rio500_mutex);
 				return bytes_written ? bytes_written : -EINTR;
 			}
 
@@ -345,12 +339,12 @@ write_rio(struct file *file, const char __user *buffer,
 		buffer += copy_size;
 	} while (count > 0);
 
-	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 
 	return bytes_written ? bytes_written : -EIO;
 
 error:
-	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 	return errn;
 }
 
@@ -367,12 +361,12 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 	char *ibuf;
 	int intr;
 
-	intr = mutex_lock_interruptible(&(rio->lock));
+	intr = mutex_lock_interruptible(&rio500_mutex);
 	if (intr)
 		return -EINTR;
 	/* Sanity check to make sure rio is connected, powered, etc */
         if (rio->present == 0 || rio->rio_dev == NULL) {
-		mutex_unlock(&(rio->lock));
+		mutex_unlock(&rio500_mutex);
 		return -ENODEV;
 	}
 
@@ -383,11 +377,11 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 
 	while (count > 0) {
 		if (signal_pending(current)) {
-			mutex_unlock(&(rio->lock));
+			mutex_unlock(&rio500_mutex);
 			return read_count ? read_count : -EINTR;
 		}
 		if (!rio->rio_dev) {
-			mutex_unlock(&(rio->lock));
+			mutex_unlock(&rio500_mutex);
 			return -ENODEV;
 		}
 		this_read = (count >= IBUF_SIZE) ? IBUF_SIZE : count;
@@ -405,7 +399,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 			count = this_read = partial;
 		} else if (result == -ETIMEDOUT || result == 15) {	/* FIXME: 15 ??? */
 			if (!maxretry--) {
-				mutex_unlock(&(rio->lock));
+				mutex_unlock(&rio500_mutex);
 				dev_err(&rio->rio_dev->dev,
 					"read_rio: maxretry timeout\n");
 				return -ETIME;
@@ -415,19 +409,19 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 			finish_wait(&rio->wait_q, &wait);
 			continue;
 		} else if (result != -EREMOTEIO) {
-			mutex_unlock(&(rio->lock));
+			mutex_unlock(&rio500_mutex);
 			dev_err(&rio->rio_dev->dev,
 				"Read Whoops - result:%d partial:%u this_read:%u\n",
 				result, partial, this_read);
 			return -EIO;
 		} else {
-			mutex_unlock(&(rio->lock));
+			mutex_unlock(&rio500_mutex);
 			return (0);
 		}
 
 		if (this_read) {
 			if (copy_to_user(buffer, ibuf, this_read)) {
-				mutex_unlock(&(rio->lock));
+				mutex_unlock(&rio500_mutex);
 				return -EFAULT;
 			}
 			count -= this_read;
@@ -435,7 +429,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos)
 			buffer += this_read;
 		}
 	}
-	mutex_unlock(&(rio->lock));
+	mutex_unlock(&rio500_mutex);
 	return read_count;
 }
 
@@ -495,8 +489,6 @@ static int probe_rio(struct usb_interface *intf,
 	}
 	dev_dbg(&intf->dev, "ibuf address:%p\n", rio->ibuf);
 
-	mutex_init(&(rio->lock));
-
 	usb_set_intfdata (intf, rio);
 	rio->present = 1;
 
@@ -512,12 +504,10 @@ static void disconnect_rio(struct usb_interface *intf)
 	if (rio) {
 		usb_deregister_dev(intf, &usb_rio_class);
 
-		mutex_lock(&(rio->lock));
 		if (rio->isopen) {
 			rio->isopen = 0;
 			/* better let it finish - the release will do whats needed */
 			rio->rio_dev = NULL;
-			mutex_unlock(&(rio->lock));
 			mutex_unlock(&rio500_mutex);
 			return;
 		}
@@ -529,7 +519,6 @@ static void disconnect_rio(struct usb_interface *intf)
 		dev_info(&intf->dev, "USB Rio disconnected.\n");
 
 		rio->present = 0;
-		mutex_unlock(&(rio->lock));
 	}
 	mutex_unlock(&rio500_mutex);
 }
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [1/3] rio500: refuse more than one device at a time
@ 2019-04-30 14:47 ` Alan Stern
  0 siblings, 0 replies; 8+ messages in thread
From: Alan Stern @ 2019-04-30 14:47 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: gregKH, miquel, linux-usb

On Tue, 30 Apr 2019, Oliver Neukum wrote:

> This driver is using a global variable. It cannot handle more than
> one device at a time. The issue has been exisying since the dawn

s/exisying/existing/

> of the driver.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Reported-by: syzbot+35f04d136fc975a70da4@syzkaller.appspotmail.com
> ---
>  drivers/usb/misc/rio500.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c
> index 13e4889bc34f..a4b6fbea975f 100644
> --- a/drivers/usb/misc/rio500.c
> +++ b/drivers/usb/misc/rio500.c
> @@ -449,7 +449,12 @@ static int probe_rio(struct usb_interface *intf,
>  	struct rio_usb_data *rio = &rio_instance;
>  	int retval;
>  
> -	dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
> +	if (rio->present) {
> +		dev_info(&intf->dev, "Second USB Rio at address %d refused\n", dev->devnum);
> +		return -EBUSY;
> +	} else {
> +		dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
> +	}

This will race if more than one Rio is probed at the same time.  You 
should hold the rio500_mutex throughout this routine.

Alan Stern

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] rio500: refuse more than one device at a time
@ 2019-04-30 14:47 ` Alan Stern
  0 siblings, 0 replies; 8+ messages in thread
From: Alan Stern @ 2019-04-30 14:47 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: gregKH, miquel, linux-usb

On Tue, 30 Apr 2019, Oliver Neukum wrote:

> This driver is using a global variable. It cannot handle more than
> one device at a time. The issue has been exisying since the dawn

s/exisying/existing/

> of the driver.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Reported-by: syzbot+35f04d136fc975a70da4@syzkaller.appspotmail.com
> ---
>  drivers/usb/misc/rio500.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c
> index 13e4889bc34f..a4b6fbea975f 100644
> --- a/drivers/usb/misc/rio500.c
> +++ b/drivers/usb/misc/rio500.c
> @@ -449,7 +449,12 @@ static int probe_rio(struct usb_interface *intf,
>  	struct rio_usb_data *rio = &rio_instance;
>  	int retval;
>  
> -	dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
> +	if (rio->present) {
> +		dev_info(&intf->dev, "Second USB Rio at address %d refused\n", dev->devnum);
> +		return -EBUSY;
> +	} else {
> +		dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
> +	}

This will race if more than one Rio is probed at the same time.  You 
should hold the rio500_mutex throughout this routine.

Alan Stern


^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-04-30 14:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-30 14:23 [2/3] rio500: fix memeory leak in close after disconnect Oliver Neukum
2019-04-30 14:23 ` [PATCH 2/3] " Oliver Neukum
  -- strict thread matches above, loose matches on Subject: below --
2019-04-30 14:47 [1/3] rio500: refuse more than one device at a time Alan Stern
2019-04-30 14:47 ` [PATCH 1/3] " Alan Stern
2019-04-30 14:23 [3/3] rio500: simplify locking Oliver Neukum
2019-04-30 14:23 ` [PATCH 3/3] " Oliver Neukum
2019-04-30 14:23 [1/3] rio500: refuse more than one device at a time Oliver Neukum
2019-04-30 14:23 ` [PATCH 1/3] " Oliver Neukum

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.