[PATCH] usb: core: fix a double free in the usb driver

[PATCH] usb: core: fix a double free in the usb driver

[Chung-Geol Kim]

There is a double free problem in the usb driver.
This is caused by delayed deregister for scsi device.
<*> at Insert USB Storage
    -  USB bus #1 register
     usb_create_hcd (primary-kref==1)
     * primary-bandwidth_mutex(alloc))
usb_get_hcd    (primary-kref==2)
    -  USB bus #2 register
usb_create_hcd (second-kref==1)
     * second-bandwidth_mutex==primary-bandwidth_mutex
     usb_get_hcd    (second-kref==2)
    -  scsi_device_get
usb_get_hcd    (second-kref==3)

<*> at remove USB Storage (Normal)
    -  scsi_device_put
usb_put_hcd    (second-kref==2)
    -  USB bus #2 deregister
     usb_release_dev(second-kref==1)
usb_release_dev(second-kref==0)  -> hcd_release()
    -  USB bus #1 deregister
     usb_release_dev(primary-kref==1)
     usb_release_dev(primary-kref==0) -> hcd_release()
*(primary-bandwidth_mutex free)

at remove USB Storage 
    -  USB bus #2 deregister
     usb_release_dev(second-kref==2)
usb_release_dev(second-kref==1)
    -  USB bus #1 deregister
     usb_release_dev(primary-kref==1)
     usb_release_dev(primary-kref==0) -> hcd_release()
     *(primary-bandwidth_mutex free)
    -  scsi_device_put
     usb_put_hcd    (second-kref==0)  -> hcd_release(*)
     * at this, second->primary==0 therefore try to
free the primary-bandwidth_mutex.(already freed)

    To fix this problem kfree(hcd->bandwidth_mutex);
    should be executed at only (hcd->primary_hcd==hcd).

Signed-off-by: Chunggeol Kim 
---
drivers/usb/core/hcd.c |    2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 34b837a..60077f3 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2608,7 +2608,7 @@ static void hcd_release(struct kref *kref)
struct usb_hcd *hcd = container_of (kref, struct usb_hcd, kref);

mutex_lock(&usb_port_peer_mutex);
- if (usb_hcd_is_primary_hcd(hcd)) {
+ if (hcd == hcd->primary_hcd) {
kfree(hcd->address0_mutex);
kfree(hcd->bandwidth_mutex);
}
-- 
1.7.9.5

Re: [PATCH] usb: core: fix a double free in the usb driver

[gregkh]

On Fri, May 27, 2016 at 01:38:17AM +0000, Chung-Geol Kim wrote:
> There is a double free problem in the usb driver.

Which driver?

> This is caused by delayed deregister for scsi device.
> <*> at Insert USB Storage
>     -  USB bus #1 register
>      usb_create_hcd (primary-kref==1)
>      * primary-bandwidth_mutex(alloc))
> usb_get_hcd    (primary-kref==2)
>     -  USB bus #2 register
> usb_create_hcd (second-kref==1)
>      * second-bandwidth_mutex==primary-bandwidth_mutex
>      usb_get_hcd    (second-kref==2)
>     -  scsi_device_get
> usb_get_hcd    (second-kref==3)
> 
> <*> at remove USB Storage (Normal)
>     -  scsi_device_put
> usb_put_hcd    (second-kref==2)
>     -  USB bus #2 deregister
>      usb_release_dev(second-kref==1)
> usb_release_dev(second-kref==0)  -> hcd_release()
>     -  USB bus #1 deregister
>      usb_release_dev(primary-kref==1)
>      usb_release_dev(primary-kref==0) -> hcd_release()
> *(primary-bandwidth_mutex free)
> 
> at remove USB Storage 
>     -  USB bus #2 deregister
>      usb_release_dev(second-kref==2)
> usb_release_dev(second-kref==1)
>     -  USB bus #1 deregister
>      usb_release_dev(primary-kref==1)
>      usb_release_dev(primary-kref==0) -> hcd_release()
>      *(primary-bandwidth_mutex free)
>     -  scsi_device_put
>      usb_put_hcd    (second-kref==0)  -> hcd_release(*)
>      * at this, second->primary==0 therefore try to
> free the primary-bandwidth_mutex.(already freed)

The formatting for this is all confused, can you fix it up?

> 
>     To fix this problem kfree(hcd->bandwidth_mutex);
>     should be executed at only (hcd->primary_hcd==hcd).
> 
> Signed-off-by: Chunggeol Kim 

We need an email address at the end of this line, look at how the
commits in the kernel git history look like for examples.

> ---
> drivers/usb/core/hcd.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 34b837a..60077f3 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2608,7 +2608,7 @@ static void hcd_release(struct kref *kref)
> struct usb_hcd *hcd = container_of (kref, struct usb_hcd, kref);
> 
> mutex_lock(&usb_port_peer_mutex);
> - if (usb_hcd_is_primary_hcd(hcd)) {
> + if (hcd == hcd->primary_hcd) {

That doesn't make sense, usb_hcd_is_primary_hcd() is the same as this
check, what are you changing here?

> kfree(hcd->address0_mutex);
> kfree(hcd->bandwidth_mutex);
> }

Your patch itself is also corrupted, and can't be applied, can you also
resolve this and resend?

thanks,

greg k-h