Re: [PROBLEM] usb hub malformed packets causes null pointer dereference

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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,�(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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux