* [PATCHv2 1/4] rio500: refuse more than one device at a time
@ 2019-05-07 7:38 Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 2/4] rio500: fix memeory leak in close after disconnect Oliver Neukum
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Oliver Neukum @ 2019-05-07 7:38 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 existing since the dawn
of the driver.
V2: Fixed locking in probe()
Fixed Documentation
Signed-off-by: Oliver Neukum <oneukum@suse.com>
Reported-by: syzbot+35f04d136fc975a70da4@syzkaller.appspotmail.com
---
drivers/usb/misc/rio500.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c
index 13e4889bc34f..fa921ae60ffa 100644
--- a/drivers/usb/misc/rio500.c
+++ b/drivers/usb/misc/rio500.c
@@ -447,15 +447,23 @@ static int probe_rio(struct usb_interface *intf,
{
struct usb_device *dev = interface_to_usbdev(intf);
struct rio_usb_data *rio = &rio_instance;
- int retval;
+ int retval = 0;
- dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
+ mutex_lock(&rio500_mutex);
+ if (rio->present) {
+ dev_info(&intf->dev, "Second USB Rio at address %d refused\n", dev->devnum);
+ retval = -EBUSY;
+ goto bail_out;
+ } else {
+ dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
+ }
retval = usb_register_dev(intf, &usb_rio_class);
if (retval) {
dev_err(&dev->dev,
"Not able to get a minor for this device.\n");
- return -ENOMEM;
+ retval = -ENOMEM;
+ goto bail_out;
}
rio->rio_dev = dev;
@@ -464,7 +472,8 @@ static int probe_rio(struct usb_interface *intf,
dev_err(&dev->dev,
"probe_rio: Not enough memory for the output buffer\n");
usb_deregister_dev(intf, &usb_rio_class);
- return -ENOMEM;
+ retval = -ENOMEM;
+ goto bail_out;
}
dev_dbg(&intf->dev, "obuf address:%p\n", rio->obuf);
@@ -473,7 +482,8 @@ static int probe_rio(struct usb_interface *intf,
"probe_rio: Not enough memory for the input buffer\n");
usb_deregister_dev(intf, &usb_rio_class);
kfree(rio->obuf);
- return -ENOMEM;
+ retval = -ENOMEM;
+ goto bail_out;
}
dev_dbg(&intf->dev, "ibuf address:%p\n", rio->ibuf);
@@ -481,8 +491,10 @@ static int probe_rio(struct usb_interface *intf,
usb_set_intfdata (intf, rio);
rio->present = 1;
+bail_out:
+ mutex_unlock(&rio500_mutex);
- return 0;
+ return retval;
}
static void disconnect_rio(struct usb_interface *intf)
--
2.16.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCHv2 2/4] rio500: fix memeory leak in close after disconnect
2019-05-07 7:38 [PATCHv2 1/4] rio500: refuse more than one device at a time Oliver Neukum
@ 2019-05-07 7:38 ` Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 3/4] rio500: simplify locking Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 4/4] USB: rio500: update Documentation Oliver Neukum
2 siblings, 0 replies; 5+ messages in thread
From: Oliver Neukum @ 2019-05-07 7:38 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 fa921ae60ffa..a1832c195b5f 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] 5+ messages in thread
* [PATCHv2 3/4] rio500: simplify locking
2019-05-07 7:38 [PATCHv2 1/4] rio500: refuse more than one device at a time Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 2/4] rio500: fix memeory leak in close after disconnect Oliver Neukum
@ 2019-05-07 7:38 ` Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 4/4] USB: rio500: update Documentation Oliver Neukum
2 siblings, 0 replies; 5+ messages in thread
From: Oliver Neukum @ 2019-05-07 7:38 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 a1832c195b5f..a0574f54738f 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;
}
@@ -500,8 +494,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;
bail_out:
@@ -519,12 +511,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;
}
@@ -536,7 +526,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] 5+ messages in thread
* [PATCHv2 4/4] USB: rio500: update Documentation
2019-05-07 7:38 [PATCHv2 1/4] rio500: refuse more than one device at a time Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 2/4] rio500: fix memeory leak in close after disconnect Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 3/4] rio500: simplify locking Oliver Neukum
@ 2019-05-07 7:38 ` Oliver Neukum
2019-05-07 8:51 ` Sergei Shtylyov
2 siblings, 1 reply; 5+ messages in thread
From: Oliver Neukum @ 2019-05-07 7:38 UTC (permalink / raw)
To: gregKH, miquel, linux-usb; +Cc: Oliver Neukum
Added the newly added limit and updated the text a bit
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
Documentation/usb/rio.txt | 54 +++++++++++++----------------------------------
1 file changed, 15 insertions(+), 39 deletions(-)
diff --git a/Documentation/usb/rio.txt b/Documentation/usb/rio.txt
index ca9adcf56355..63adb030e0e9 100644
--- a/Documentation/usb/rio.txt
+++ b/Documentation/usb/rio.txt
@@ -76,54 +76,25 @@ Additional Information and userspace tools
Requirements
============
-A host with a USB port. Ideally, either a UHCI (Intel) or OHCI
-(Compaq and others) hardware port should work.
+A host with a USB port running a Linux kernel with RIO 500 support enabled.
-A Linux development kernel (2.3.x) with USB support enabled or a
-backported version to linux-2.2.x. See http://www.linux-usb.org for
-more information on accomplishing this.
+The driver is a module called rio500, which should be automatically loaded
+as you plug in your device.
+If that fails you can manually load it with
-A Linux kernel with RIO 500 support enabled.
+ modprobe rio500
-'lspci' which is only needed to determine the type of USB hardware
-available in your machine.
-
-Configuration
-
-Using `lspci -v`, determine the type of USB hardware available.
-
- If you see something like::
-
- USB Controller: ......
- Flags: .....
- I/O ports at ....
-
- Then you have a UHCI based controller.
-
- If you see something like::
-
- USB Controller: .....
- Flags: ....
- Memory at .....
-
- Then you have a OHCI based controller.
-
-Using `make menuconfig` or your preferred method for configuring the
-kernel, select 'Support for USB', 'OHCI/UHCI' depending on your
-hardware (determined from the steps above), 'USB Diamond Rio500 support', and
-'Preliminary USB device filesystem'. Compile and install the modules
-(you may need to execute `depmod -a` to update the module
-dependencies).
-
-Add a device for the USB rio500::
+Udev should automatically create a device node as soon as plug in your device.
+If that fails, you can manually add a device for the USB rio500::
mknod /dev/usb/rio500 c 180 64
-Set appropriate permissions for /dev/usb/rio500 (don't forget about
+In that case,
+set appropriate permissions for /dev/usb/rio500 (don't forget about
group and world permissions). Both read and write permissions are
required for proper operation.
-Load the appropriate modules (if compiled as modules):
+Load the appropriate modules (if ddcompiled as modules):
OHCI::
@@ -140,6 +111,11 @@ Load the appropriate modules (if compiled as modules):
That's it. The Rio500 Utils at: http://rio500.sourceforge.net should
be able to access the rio500.
+Limits
+======
+
+You can use only a single rio500 device at a time with your computer.
+
Bugs
====
--
2.16.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCHv2 4/4] USB: rio500: update Documentation
2019-05-07 7:38 ` [PATCHv2 4/4] USB: rio500: update Documentation Oliver Neukum
@ 2019-05-07 8:51 ` Sergei Shtylyov
0 siblings, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2019-05-07 8:51 UTC (permalink / raw)
To: Oliver Neukum, gregKH, miquel, linux-usb
On 07.05.2019 10:38, Oliver Neukum wrote:
> Added the newly added limit and updated the text a bit
>
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
> Documentation/usb/rio.txt | 54 +++++++++++++----------------------------------
> 1 file changed, 15 insertions(+), 39 deletions(-)
>
> diff --git a/Documentation/usb/rio.txt b/Documentation/usb/rio.txt
> index ca9adcf56355..63adb030e0e9 100644
> --- a/Documentation/usb/rio.txt
> +++ b/Documentation/usb/rio.txt
> @@ -76,54 +76,25 @@ Additional Information and userspace tools
[...]
> -Add a device for the USB rio500::
> +Udev should automatically create a device node as soon as plug in your device.
> +If that fails, you can manually add a device for the USB rio500::
>
> mknod /dev/usb/rio500 c 180 64
>
> -Set appropriate permissions for /dev/usb/rio500 (don't forget about
> +In that case,
Strange formatting...
> +set appropriate permissions for /dev/usb/rio500 (don't forget about
> group and world permissions). Both read and write permissions are
> required for proper operation.
>
> -Load the appropriate modules (if compiled as modules):
> +Load the appropriate modules (if ddcompiled as modules):
ddcompiled? :-)
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-07 8:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-07 7:38 [PATCHv2 1/4] rio500: refuse more than one device at a time Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 2/4] rio500: fix memeory leak in close after disconnect Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 3/4] rio500: simplify locking Oliver Neukum
2019-05-07 7:38 ` [PATCHv2 4/4] USB: rio500: update Documentation Oliver Neukum
2019-05-07 8:51 ` Sergei Shtylyov
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.