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]
Message-ID: <Pine.LNX.4.44L0.1606031021440.1920-100000@iolanthe.rowland.org>
Date:	Fri, 3 Jun 2016 10:28:00 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Chung-Geol Kim <chunggeol.kim@...sung.com>
cc:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"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>,
	"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: Re: Re: [PATCH] usb: core: fix a double free in the usb driver

On Fri, 3 Jun 2016, Chung-Geol Kim wrote:

> Yes, you are right, The presentational errors in order to obtain an understanding of the process.
> Therefore, I will be happy to explain again the diagrammatic representation as shown below.
> If using usb 3.0 storage(OTG), you can see as below.
> 
> ==============================================
>     At *Insert USB(3.0) Storage
>     sequence <1> --> <5>
> ==============================================
>                                 VOLD       
> =================================|============
>                              (uevent)
>                            ______|___________ 
>                           |<5>               |
>                           |      SCSI        |
>                           |usb_get_hcd       |
>                           |shared_hcd(kref=3)|
>                           |__________________|
>    ___________________     ________|_________ 
>   |<2>                |   |<4>               |
>   |dwc3_otg_sm_work   |   |dwc3_otg_sm_work  |
>   |usb_get_hcd        |   |usb_get_hcd       |
>   |primary_hcd(kref=2)|   |shared_hcd(kref=2)|
>   |___________________|   |__________________|
>    _________|_________     ________|_________ 
>   |<1>                |   |<3>               |
>   |New USB BUS #1     |   |New USB BUS #2    |
>   |usb_create_hcd     |   |usb_create_hcd    |
>   |primary_hcd(kref=1)|   |shared_hcd(kref=1)|
>   |                   |   |                  |
>   |bandXX_mutex(alloc)|<-(Link)-bandXX_mutex |
>   |___________________|   |__________________|
> 

When people present diagrams like this, the universal convention is to 
show earlier times at the top and later times at the bottom.  That way 
the order in which you read the diagram is the same as the order in 
which the events are supposed to occur.

Also, the convention is to put events next to each other if they happen 
at the same time.  In your diagram, <1> and <3> are next to each other 
but they don't happen at the same time.

> ==============================================
>     At *remove USB(3.0) Storage 
>     sequence <1> --> <5> ((Normal Case))
> ==============================================
>                                 VOLD       
> =================================|============
>                              (uevent)
>                            ______|___________ 
>                           |<1>               |
>                           |      SCSI        |
>                           |usb_put_hcd       |
>                           |shared_hcd(kref=2)|
>                           |__________________|
>    ___________________     ________|_________ 
>   |<4>                |   |<2>               |
>   |dwc3_otg_sm_work   |   |dwc3_otg_sm_work  |
>   |usb_put_hcd        |   |usb_put_hcd       |
>   |primary_hcd(kref=1)|   |shared_hcd(kref=1)|
>   |___________________|   |__________________|
>    _________|_________     ________|_________ 
>   |<5>                |   |<3>               |
>   |New USB BUS #1     |   |New USB BUS #2    |
>   |hcd_release        |   |hcd_release       |
>   |primary_hcd(kref=0)|   |shared_hcd(kref=0)|
>   |                   |   |                  |
>   |bandXX_mutex(free) | -X-cut off)-bandXX_mutex|
>   |___________________|   |__________________|
> 
> ----------------------------------------------

> >The real bug here is that the shared_hcd is released after the 
> >primary_hcd.  That's what you need to fix.
> 
> NO, It 's only depend on vold(scsi) release time.
> If the vold later released and is being released first hcd,
> Double free happened at <5> as below.
> 
> ==============================================
>     At *remove USB(3.0) Storage 
>     sequence <1> --> <5> ((Problem Case))
> ==============================================
>                                 VOLD       
> =================================|============
>                              (uevent)
>                            ______|___________ 
>                           |<5>               |
>                           |      SCSI        |
>                           |usb_put_hcd       |
>                           |shared_hcd(kref=0)|
>                           |*hcd_release      |
>                           |bandXX_mutex(free*)|<- double free
>                           |__________________|
>    ___________________     ________|_________ 
>   |<3>                |   |<1>               |
>   |dwc3_otg_sm_work   |   |dwc3_otg_sm_work  |
>   |usb_put_hcd        |   |usb_put_hcd       |
>   |primary_hcd(kref=1)|   |shared_hcd(kref=2)|
>   |___________________|   |__________________|
>    _________|_________     ________|_________ 
>   |<4>                |   |<2>               |
>   |New USB BUS #1     |   |New USB BUS #2    |
>   |hcd_release        |   |                  |
>   |primary_hcd(kref=0)|   |shared_hcd(kref=1)|
>   |                   |   |                  |
>   |bandXX_mutex(free) |<-(Link)-bandXX_mutex |
>   |___________________|   |__________________|
> 
> ----------------------------------------------

That's the same as what I said.  In your diagram, vold releases the 
shared_hcd (in <5>) after dwc3_otg_sm_work releases the primary_hcd 
(in <4>).

If you change the code so that the shared_hcd takes a reference to the 
primary_hcd and drops that reference when the shared_hcd is released, 
this will never occur.

Alan Stern

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ