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

[gregkh@xxxxxxxxxxxxxxxxxxx]

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