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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 26 May 2016 21:41:03 -0700
From:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To:	Chung-Geol Kim <chunggeol.kim@...sung.com>
Cc:	"mathias.nyman@...ux.intel.com" <mathias.nyman@...ux.intel.com>,
	"stefan.koch10@...il.com" <stefan.koch10@...il.com>,
	"hkallweit1@...il.com" <hkallweit1@...il.com>,
	"sergei.shtylyov@...entembedded.com" 
	<sergei.shtylyov@...entembedded.com>,
	"dan.j.williams@...el.com" <dan.j.williams@...el.com>,
	"sarah.a.sharp@...ux.intel.com" <sarah.a.sharp@...ux.intel.com>,
	"stern@...land.harvard.edu" <stern@...land.harvard.edu>,
	"chris.bainbridge@...il.com" <chris.bainbridge@...il.com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: core: fix a double free in the usb driver

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