* [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.