On Tue, 10 Nov 2015, Cornea, Alexandru wrote:
> Hello,
>
> We observed a kernel panic due to a null pointer dereference in the USB
> stack while sending malformed USB hub packets.
This is an interesting bug. Not least because it has nothing to do
with the malformed packets -- it was triggered when the USB device was
disconnected while the initialization procedure was still running.
> Testing was done on a Galileo board, using kernel version 4.1.8 (image
> built with Yocto project).
>
> Output of /proc/version: Linux version 4.1.8-yocto-standard (REDACTED)
> (gcc version 5.2.0 (GCC) ) #1 PREEMPT Fri Oct 30 15:05:46 EET 2015
>
> Please see full call trace at the end of the mail and reproduction steps.
>
> Running in debug mode, part of the call trace is displayed, along with:
> <6,�<6,� (null): activate --> -19
> which links to drivers/usb/core/hub.c:1239 , function hub_activate (init3).
> This shows that hub->intfdev is null.
>
> 1240 init3:
> 1241 hub->quiescing = 0;
> 1242
> 1243 status = usb_submit_urb(hub->urb, GFP_NOIO);
> 1244 if (status < 0)
> 1245 dev_err(hub->intfdev, "activate --> %d\n", status);
>
> If you need additional info, please let us know.
Please try the patch below and let us know if it fixes the problem.
Alan Stern
Index: usb-4.3/drivers/usb/core/hub.c
===================================================================
--- usb-4.3.orig/drivers/usb/core/hub.c
+++ usb-4.3/drivers/usb/core/hub.c
@@ -1031,10 +1031,20 @@ static void hub_activate(struct usb_hub
unsigned delay;
/* Continue a partial initialization */
- if (type == HUB_INIT2)
- goto init2;
- if (type == HUB_INIT3)
+ if (type == HUB_INIT2 || type == HUB_INIT3) {
+ device_lock(hub->intfdev);
+
+ /* Was the hub disconnected while we were waiting? */
+ if (hub->disconnected) {
+ device_unlock(hub->intfdev);
+ kref_put(&hub->kref, hub_release);
+ return;
+ }
+ if (type == HUB_INIT2)
+ goto init2;
goto init3;
+ }
+ kref_get(&hub->kref);
/* The superspeed hub except for root hub has to use Hub Depth
* value as an offset into the route string to locate the bits
@@ -1232,6 +1242,7 @@ static void hub_activate(struct usb_hub
queue_delayed_work(system_power_efficient_wq,
&hub->init_work,
msecs_to_jiffies(delay));
+ device_unlock(hub->intfdev);
return; /* Continues at init3: below */
} else {
msleep(delay);
@@ -1253,6 +1264,11 @@ static void hub_activate(struct usb_hub
/* Allow autosuspend if it was suppressed */
if (type <= HUB_INIT3)
usb_autopm_put_interface_async(to_usb_interface(hub->intfdev));
+
+ if (type == HUB_INIT3)
+ device_unlock(hub->intfdev);
+
+ kref_put(&hub->kref, hub_release);
}
/* Implement the continuations for the delays above */
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
