On Mon, 20 Mar 2017, Guenter Roeck wrote: > While running a bind/unbind stress test with the dwc3 usb driver on rk3399, > the following crash was observed. > > Unable to handle kernel NULL pointer dereference at virtual address 00000218 > pgd = ffffffc00165f000 > [00000218] *pgd=000000000174f003, *pud=000000000174f003, > *pmd=0000000001750003, *pte=00e8000001751713 ... > Code analysis shows that intf is set to NULL in usb_disable_device() prior > to setting actconfig to NULL. At the same time, usb_runtime_idle() does not > lock the usb device, and neither does any of the functions in the > traceback. This means that there is no protection against a race condition > where usb_disable_device() is removing dev->actconfig->interface[] pointers > while those are being accessed from autosuspend_check(). > > To solve the problem, synchronize and validate device state between > autosuspend_check() and usb_disconnect(). > > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx> > --- > v2: Do not disable autosuspend in usb_disconnect(). Instead, check the > usb device state in autosuspend_check(). > In usb_disconnect(), call pm_runtime_barrier() earlier, immediately > after changing the USB device state. > > drivers/usb/core/driver.c | 3 +++ > drivers/usb/core/hub.c | 7 +++++++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c > index 7ebdf2a4e8fe..eb87a259d55c 100644 > --- a/drivers/usb/core/driver.c > +++ b/drivers/usb/core/driver.c > @@ -1781,6 +1781,9 @@ static int autosuspend_check(struct usb_device *udev) > int w, i; > struct usb_interface *intf; > > + if (udev->state == USB_STATE_NOTATTACHED) > + return -ENODEV; > + > /* Fail if autosuspend is disabled, or any interfaces are in use, or > * any interface drivers require remote wakeup but it isn't available. > */ > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 57bb427c8878..fd80513aaeff 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -2084,6 +2084,13 @@ void usb_disconnect(struct usb_device **pdev) > * this quiesces everything except pending urbs. > */ > usb_set_device_state(udev, USB_STATE_NOTATTACHED); > + > + /* > + * Ensure that the pm runtime code knows that the USB device > + * is in the process of being disconnected. > + */ > + pm_runtime_barrier(&udev->dev); > + > dev_info(&udev->dev, "USB disconnect, device number %d\n", > udev->devnum); > Very minor nit: I would place the pm_runtime_barrier() after the dev_info(), so that we would know the device is going away if anything should go wrong during the barrier call. Aside from that, Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
