Skip to content

Commit e50293e

Browse files
AlanSterngregkh
authored andcommitted
USB: fix invalid memory access in hub_activate()
Commit 8520f38 ("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 <[email protected]> Reported-by: Alexandru Cornea <[email protected]> Tested-by: Alexandru Cornea <[email protected]> Fixes: 8520f38 ("USB: change hub initialization sleeps to delayed_work") CC: <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent abdc9a3 commit e50293e

File tree

1 file changed

+19
-3
lines changed

1 file changed

+19
-3
lines changed

drivers/usb/core/hub.c

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,10 +1035,20 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
10351035
unsigned delay;
10361036

10371037
/* Continue a partial initialization */
1038-
if (type == HUB_INIT2)
1039-
goto init2;
1040-
if (type == HUB_INIT3)
1038+
if (type == HUB_INIT2 || type == HUB_INIT3) {
1039+
device_lock(hub->intfdev);
1040+
1041+
/* Was the hub disconnected while we were waiting? */
1042+
if (hub->disconnected) {
1043+
device_unlock(hub->intfdev);
1044+
kref_put(&hub->kref, hub_release);
1045+
return;
1046+
}
1047+
if (type == HUB_INIT2)
1048+
goto init2;
10411049
goto init3;
1050+
}
1051+
kref_get(&hub->kref);
10421052

10431053
/* The superspeed hub except for root hub has to use Hub Depth
10441054
* value as an offset into the route string to locate the bits
@@ -1236,6 +1246,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
12361246
queue_delayed_work(system_power_efficient_wq,
12371247
&hub->init_work,
12381248
msecs_to_jiffies(delay));
1249+
device_unlock(hub->intfdev);
12391250
return; /* Continues at init3: below */
12401251
} else {
12411252
msleep(delay);
@@ -1257,6 +1268,11 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
12571268
/* Allow autosuspend if it was suppressed */
12581269
if (type <= HUB_INIT3)
12591270
usb_autopm_put_interface_async(to_usb_interface(hub->intfdev));
1271+
1272+
if (type == HUB_INIT2 || type == HUB_INIT3)
1273+
device_unlock(hub->intfdev);
1274+
1275+
kref_put(&hub->kref, hub_release);
12601276
}
12611277

12621278
/* Implement the continuations for the delays above */

0 commit comments

Comments
 (0)