On Wed, 7 Aug 2019, Oliver Neukum wrote: > Am Mittwoch, den 07.08.2019, 10:07 -0400 schrieb Alan Stern: > > On Wed, 7 Aug 2019, Oliver Neukum wrote: > > > technically yes. However in practical terms the straight revert I sent > > > out yesterday should fix it. > > > > I didn't see the revert, and it doesn't appear to have reached the > > mailing list archive. Can you post it again? > > As soon as our VPN server is back up again. The revert may not be necessay; a little fix should get rid of the locking violation. The key is to avoid calling the registration or deregistration routines while holding the rio500_mutex, and to recognize that the probe and disconnect routines are both protected by the device mutex. How does this patch look? Alan Stern #syz test: https://github.com/google/kasan.git 7f7867ff Index: usb-devel/drivers/usb/misc/rio500.c =================================================================== --- usb-devel.orig/drivers/usb/misc/rio500.c +++ usb-devel/drivers/usb/misc/rio500.c @@ -454,52 +454,54 @@ static int probe_rio(struct usb_interfac { struct usb_device *dev = interface_to_usbdev(intf); struct rio_usb_data *rio = &rio_instance; - int retval = 0; + int retval; + 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); 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; - - if (!(rio->obuf = kmalloc(OBUF_SIZE, GFP_KERNEL))) { + obuf = kmalloc(OBUF_SIZE, GFP_KERNEL); + if (!obuf) { 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; + goto err_obuf; } - dev_dbg(&intf->dev, "obuf address:%p\n", rio->obuf); + dev_dbg(&intf->dev, "obuf address: %p\n", obuf); - if (!(rio->ibuf = kmalloc(IBUF_SIZE, GFP_KERNEL))) { + ibuf = kmalloc(IBUF_SIZE, GFP_KERNEL); + if (!ibuf) { 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; + goto err_ibuf; } - dev_dbg(&intf->dev, "ibuf address:%p\n", rio->ibuf); + dev_dbg(&intf->dev, "ibuf address: %p\n", ibuf); + mutex_lock(&rio500_mutex); + rio->rio_dev = dev; + rio->ibuf = ibuf; + rio->obuf = obuf; usb_set_intfdata (intf, rio); rio->present = 1; -bail_out: mutex_unlock(&rio500_mutex); return retval; + + err_ibuf: + kfree(obuf); + err_obuf: + usb_deregister_dev(intf, &usb_rio_class); + err_register: + return -ENOMEM; } static void disconnect_rio(struct usb_interface *intf) @@ -507,10 +509,10 @@ static void disconnect_rio(struct usb_in 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 +526,8 @@ static void disconnect_rio(struct usb_in 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[] = {
