Commit ab2a4bf8 authored by Alan Stern's avatar Alan Stern Committed by Greg Kroah-Hartman

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 #2    |
                           |                  |
                           |peer_hcd(kref=1)  |
                           |                  |
                         --(Link)-bandXX_mutex|
                         | |__________________|
                         |
    ___________________  |
   |<3>                | |
   |dwc3_otg_sm_work   | |
   |usb_put_hcd        | |
   |primary_hcd(kref=1)| |
   |___________________| |
    _________|_________  |
   |<4>                | |
   |New USB BUS #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: default avatarAlan Stern <stern@rowland.harvard.edu>
Reported-by: default avatarChung-Geol Kim <chunggeol.kim@samsung.com>
Tested-by: default avatarChung-Geol Kim <chunggeol.kim@samsung.com>
CC: <stable@vger.kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 7e8b3dfe
...@@ -2598,26 +2598,23 @@ EXPORT_SYMBOL_GPL(usb_create_hcd); ...@@ -2598,26 +2598,23 @@ EXPORT_SYMBOL_GPL(usb_create_hcd);
* Don't deallocate the bandwidth_mutex until the last shared usb_hcd is * Don't deallocate the bandwidth_mutex until the last shared usb_hcd is
* deallocated. * deallocated.
* *
* Make sure to only deallocate the bandwidth_mutex when the primary HCD is * Make sure to deallocate the bandwidth_mutex only when the last HCD is
* freed. When hcd_release() is called for either hcd in a peer set * freed. When hcd_release() is called for either hcd in a peer set,
* invalidate the peer's ->shared_hcd and ->primary_hcd pointers to * invalidate the peer's ->shared_hcd and ->primary_hcd pointers.
* block new peering attempts
*/ */
static void hcd_release(struct kref *kref) static void hcd_release(struct kref *kref)
{ {
struct usb_hcd *hcd = container_of (kref, struct usb_hcd, kref); struct usb_hcd *hcd = container_of (kref, struct usb_hcd, kref);
mutex_lock(&usb_port_peer_mutex); mutex_lock(&usb_port_peer_mutex);
if (usb_hcd_is_primary_hcd(hcd)) {
kfree(hcd->address0_mutex);
kfree(hcd->bandwidth_mutex);
}
if (hcd->shared_hcd) { if (hcd->shared_hcd) {
struct usb_hcd *peer = hcd->shared_hcd; struct usb_hcd *peer = hcd->shared_hcd;
peer->shared_hcd = NULL; peer->shared_hcd = NULL;
if (peer->primary_hcd == hcd) peer->primary_hcd = NULL;
peer->primary_hcd = NULL; } else {
kfree(hcd->address0_mutex);
kfree(hcd->bandwidth_mutex);
} }
mutex_unlock(&usb_port_peer_mutex); mutex_unlock(&usb_port_peer_mutex);
kfree(hcd); kfree(hcd);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment