Skip to content

Commit ab2a4bf

Browse files
AlanSterngregkh
authored andcommitted
USB: don't free bandwidth_mutex too early
The USB core contains a bug that can show up when a USB-3 host controller is removed. If the primary (USB-2) hcd structure is released before the shared (USB-3) hcd, the core will try to do a double-free of the common bandwidth_mutex. The problem was described in graphical form by Chung-Geol Kim, who first reported it: ================================================= At *remove USB(3.0) Storage sequence <1> --> <5> ((Problem Case)) ================================================= VOLD ------------------------------------|------------ (uevent) ________|_________ |<1> | |dwc3_otg_sm_work | |usb_put_hcd | |peer_hcd(kref=2)| |__________________| ________|_________ |<2> | |New USB BUS rib#2 | | | |peer_hcd(kref=1) | | | --(Link)-bandXX_mutex| | |__________________| | ___________________ | |<3> | | |dwc3_otg_sm_work | | |usb_put_hcd | | |primary_hcd(kref=1)| | |___________________| | _________|_________ | |<4> | | |New USB BUS rib#1 | | |hcd_release | | |primary_hcd(kref=0)| | | | | |bandXX_mutex(free) |<- |___________________| (( VOLD )) ______|___________ |<5> | | SCSI | |usb_put_hcd | |peer_hcd(kref=0) | |*hcd_release | |bandXX_mutex(free*)|<- double free |__________________| ================================================= This happens because hcd_release() frees the bandwidth_mutex whenever it sees a primary hcd being released (which is not a very good idea in any case), but in the course of releasing the primary hcd, it changes the pointers in the shared hcd in such a way that the shared hcd will appear to be primary when it gets released. This patch fixes the problem by changing hcd_release() so that it deallocates the bandwidth_mutex only when the _last_ hcd structure referencing it is released. The patch also removes an unnecessary test, so that when an hcd is released, both the shared_hcd and primary_hcd pointers in the hcd's peer will be cleared. Signed-off-by: Alan Stern <[email protected]> Reported-by: Chung-Geol Kim <[email protected]> Tested-by: Chung-Geol Kim <[email protected]> CC: <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 7e8b3df commit ab2a4bf

File tree

1 file changed

+7
-10
lines changed

1 file changed

+7
-10
lines changed

drivers/usb/core/hcd.c

+7-10
Original file line numberDiff line numberDiff line change
@@ -2598,26 +2598,23 @@ EXPORT_SYMBOL_GPL(usb_create_hcd);
25982598
* Don't deallocate the bandwidth_mutex until the last shared usb_hcd is
25992599
* deallocated.
26002600
*
2601-
* Make sure to only deallocate the bandwidth_mutex when the primary HCD is
2602-
* freed. When hcd_release() is called for either hcd in a peer set
2603-
* invalidate the peer's ->shared_hcd and ->primary_hcd pointers to
2604-
* block new peering attempts
2601+
* Make sure to deallocate the bandwidth_mutex only when the last HCD is
2602+
* freed. When hcd_release() is called for either hcd in a peer set,
2603+
* invalidate the peer's ->shared_hcd and ->primary_hcd pointers.
26052604
*/
26062605
static void hcd_release(struct kref *kref)
26072606
{
26082607
struct usb_hcd *hcd = container_of (kref, struct usb_hcd, kref);
26092608

26102609
mutex_lock(&usb_port_peer_mutex);
2611-
if (usb_hcd_is_primary_hcd(hcd)) {
2612-
kfree(hcd->address0_mutex);
2613-
kfree(hcd->bandwidth_mutex);
2614-
}
26152610
if (hcd->shared_hcd) {
26162611
struct usb_hcd *peer = hcd->shared_hcd;
26172612

26182613
peer->shared_hcd = NULL;
2619-
if (peer->primary_hcd == hcd)
2620-
peer->primary_hcd = NULL;
2614+
peer->primary_hcd = NULL;
2615+
} else {
2616+
kfree(hcd->address0_mutex);
2617+
kfree(hcd->bandwidth_mutex);
26212618
}
26222619
mutex_unlock(&usb_port_peer_mutex);
26232620
kfree(hcd);

0 commit comments

Comments
 (0)