openkylin
/
linux
mirror of https://gitee.com/openkylin/linux.git
9
0
Fork 0
Code Issues Projects Releases Wiki Activity

USB: rio500: Fix lockdep violation 

The syzbot fuzzer found a lockdep violation in the rio500 driver:

	======================================================
	WARNING: possible circular locking dependency detected
	5.3.0-rc2+ #23 Not tainted
	------------------------------------------------------
	syz-executor.2/20386 is trying to acquire lock:
	00000000772249c6 (rio500_mutex){+.+.}, at: open_rio+0x16/0xc0
	drivers/usb/misc/rio500.c:64

	but task is already holding lock:
	00000000d3e8f4b9 (minor_rwsem){++++}, at: usb_open+0x23/0x270
	drivers/usb/core/file.c:39

	which lock already depends on the new lock.

The problem is that the driver's open_rio() routine is called while
the usbcore's minor_rwsem is locked for reading, and it acquires the
rio500_mutex; whereas conversely, probe_rio() and disconnect_rio()
first acquire the rio500_mutex and then call usb_register_dev() or
usb_deregister_dev(), which lock minor_rwsem for writing.

The correct ordering of acquisition should be: minor_rwsem first, then
rio500_mutex (since the locking in open_rio() cannot be changed).
Thus, the probe and disconnect routines should avoid holding
rio500_mutex while doing their registration and deregistration.

This patch adjusts the code in those two routines to do just that.  It
also relies on the fact that the probe and disconnect routines are
protected by the device mutex, so the initial test of rio->present
needs no extra locking.

Reported-by: syzbot+7bbcbe9c9ff0cd49592a@syzkaller.appspotmail.com
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Fixes: d710734b06 ("USB: rio500: simplify locking")
Acked-by: Oliver Neukum <oneukum@suse.com>
CC: <stable@vger.kernel.org>

Link: https://lore.kernel.org/r/Pine.LNX.4.44L0.1908081329240.1319-100000@iolanthe.rowland.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Alan Stern 2019-08-08 13:34:08 -04:00 committed by Greg Kroah-Hartman
parent 95e29e9bbe
commit 9472aff16c
1 changed files with 38 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
drivers/usb/misc/rio500.c
View File

@ -454,51 +454,55 @@ 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 = 0;
int retval = -ENOMEM;
char *ibuf, *obuf;
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);
return -EBUSY;
}
dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum);
obuf = kmalloc(OBUF_SIZE, GFP_KERNEL);
if (!obuf) {
dev_err(&dev->dev,
"probe_rio: Not enough memory for the output buffer\n");
goto err_obuf;
}
dev_dbg(&intf->dev, "obuf address: %p\n", obuf);
ibuf = kmalloc(IBUF_SIZE, GFP_KERNEL);
if (!ibuf) {
dev_err(&dev->dev,
"probe_rio: Not enough memory for the input buffer\n");
goto err_ibuf;
}
dev_dbg(&intf->dev, "ibuf address: %p\n", ibuf);
mutex_lock(&rio500_mutex);
rio->rio_dev = dev;
rio->ibuf = ibuf;
rio->obuf = obuf;
rio->present = 1;
mutex_unlock(&rio500_mutex);
retval = usb_register_dev(intf, &usb_rio_class);
if (retval) {
dev_err(&dev->dev,
"Not able to get a minor for this device.\n");
retval = -ENOMEM;
goto bail_out;
goto err_register;
}
rio->rio_dev = dev;
usb_set_intfdata(intf, rio);
return retval;
if (!(rio->obuf = kmalloc(OBUF_SIZE, GFP_KERNEL))) {
dev_err(&dev->dev,
"probe_rio: Not enough memory for the output buffer\n");
usb_deregister_dev(intf, &usb_rio_class);
retval = -ENOMEM;
goto bail_out;
}
dev_dbg(&intf->dev, "obuf address:%p\n", rio->obuf);
if (!(rio->ibuf = kmalloc(IBUF_SIZE, GFP_KERNEL))) {
dev_err(&dev->dev,
"probe_rio: Not enough memory for the input buffer\n");
usb_deregister_dev(intf, &usb_rio_class);
kfree(rio->obuf);
retval = -ENOMEM;
goto bail_out;
}
dev_dbg(&intf->dev, "ibuf address:%p\n", rio->ibuf);
usb_set_intfdata (intf, rio);
rio->present = 1;
bail_out:
err_register:
mutex_lock(&rio500_mutex);
rio->present = 0;
mutex_unlock(&rio500_mutex);
err_ibuf:
kfree(obuf);
err_obuf:
return retval;
}
@ -507,10 +511,10 @@ static void disconnect_rio(struct usb_interface *intf)
struct rio_usb_data *rio = usb_get_intfdata (intf);
usb_set_intfdata (intf, NULL);
mutex_lock(&rio500_mutex);
if (rio) {
usb_deregister_dev(intf, &usb_rio_class);
mutex_lock(&rio500_mutex);
if (rio->isopen) {
rio->isopen = 0;
/* better let it finish - the release will do whats needed */
@ -524,8 +528,8 @@ static void disconnect_rio(struct usb_interface *intf)
dev_info(&intf->dev, "USB Rio disconnected.\n");
rio->present = 0;
mutex_unlock(&rio500_mutex);
}
mutex_unlock(&rio500_mutex);
}
static const struct usb_device_id rio_table[] = {