CVE-2015-8816: Other (NVD-CWE-Other)
value: The hub_activate function in drivers/usb/core/hub.c in the Linux kernel before 4.3.5 does not properly maintain a hub-interface data structure, which allows physically proximate attackers to cause a denial of service (invalid memory access and system crash) or possibly have unspecified other impact by unplugging a USB hub device.
CWE description: NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
CVSS v3 base score: 6.8
Published: 2016-04-27T17:59Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: drivers/usb/core/hub.c
Commit date: 2015-12-18 09:30:34 -0800
Commit message: USB: fix invalid memory access in hub_activate()

Commit 8520f38099cc ("USB: change hub initialization sleeps to
delayed_work") changed the hub_activate() routine to make part of it
run in a workqueue.  However, the commit failed to take a reference to
the usb_hub structure or to lock the hub interface while doing so.  As
a result, if a hub is plugged in and quickly unplugged before the work
routine can run, the routine will try to access memory that has been
deallocated.  Or, if the hub is unplugged while the routine is
running, the memory may be deallocated while it is in active use.

This patch fixes the problem by taking a reference to the usb_hub at
the start of hub_activate() and releasing it at the end (when the work
is finished), and by locking the hub interface while the work routine
is running.  It also adds a check at the start of the routine to see
if the hub has already been disconnected, in which nothing should be
done.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Alexandru Cornea <alexandru.cornea@intel.com>
Tested-by: Alexandru Cornea <alexandru.cornea@intel.com>
Fixes: 8520f38099cc ("USB: change hub initialization sleeps to delayed_work")
CC: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Diff: diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a5cc032ef77a8d..ddbf32d599cba4 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1033,14 +1033,24 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 	int status;
 	bool need_debounce_delay = false;
 	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
 	 * it uses to determine the downstream port number. So hub driver
 	 * should send a set hub depth request to superspeed hub after
@@ -1234,10 +1244,11 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 		if (type == HUB_INIT2) {
 			INIT_DELAYED_WORK(&hub->init_work, hub_init_func3);
 			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);
 		}
 	}
@@ -1255,10 +1266,15 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 	kick_hub_wq(hub);
 
 	/* Allow autosuspend if it was suppressed */
 	if (type <= HUB_INIT3)
 		usb_autopm_put_interface_async(to_usb_interface(hub->intfdev));
+
+	if (type == HUB_INIT2 || type == HUB_INIT3)
+		device_unlock(hub->intfdev);
+
+	kref_put(&hub->kref, hub_release);
 }
 
 /* Implement the continuations for the delays above */
 static void hub_init_func2(struct work_struct *ws)
 {

Vulnerable code: 	if (type == HUB_INIT2)
		goto init2;
	if (type == HUB_INIT3)
Fixed code: 	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;
	}
	kref_get(&hub->kref);
			device_unlock(hub->intfdev);

	if (type == HUB_INIT2 || type == HUB_INIT3)
		device_unlock(hub->intfdev);

	kref_put(&hub->kref, hub_release);