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-next>] [day] [month] [year] [list]
Message-id: <1611404982.38912.1465274362339.JavaMail.weblogic@ep1ml103c>
Date:	Tue, 07 Jun 2016 04:39:22 +0000
From:	Chung-Geol Kim <chunggeol.kim@...sung.com>
To:	Alan Stern <stern@...land.harvard.edu>
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: 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.

Thank you for your kind explanation. It has been modified as shown below.
=================================================
     At *Insert USB(3.0) Storage
     sequence <1> --> <5>
=================================================
   ___________________
  |<1>                |
  |New USB BUS #1     |
  |usb_create_hcd     |
  |primary_hcd(kref=1)|
  |                   |
  |bandXX_mutex(alloc)|<--
  |___________________|  |
   _________|_________   |
  |<2>                |  |
  |dwc3_otg_sm_work   |  |
  |usb_get_hcd        |  |
  |primary_hcd(kref=2)|  |
  |___________________|  |
                         |  __________________
                         | |<3>               |
                         | |New USB BUS #2    |
                         | |usb_create_hcd    |
                         | |shared_hcd(kref=1)|
                         | |                  |
                         --(Link)-bandXX_mutex|
                           |__________________|
                            ________|_________ 
                           |<4>               |
                           |dwc3_otg_sm_work  |
                           |usb_get_hcd       |
                           |shared_hcd(kref=2)|
                           |__________________|
                            _______|__________ 
                           |<5>               |
                           |      SCSI        |
                           |usb_get_hcd       |
                           |shared_hcd(kref=3)|
                           |__________________|
                                    |
                                (uevent)
-----------------------------------|------------
                                 VOLD
=================================================


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

=================================================

>
>> ==============================================
>>     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>).
>


=================================================
     At *remove USB(3.0) Storage 
     sequence <1> --> <5> ((Problem Case))
=================================================
                                  VOLD
------------------------------------|------------
                                 (uevent)
                            ________|_________
                           |<1>               |
                           |dwc3_otg_sm_work  |
                           |usb_put_hcd       |
                           |shared_hcd(kref=2)|
                           |__________________|
                            ________|_________ 
                           |<2>               |
                           |New USB BUS #2    |
                           |                  |
                           |shared_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       |
                           |shared_hcd(kref=0)|
                           |*hcd_release      |
                           |bandXX_mutex(free*)|<- double free
                           |__________________|

=================================================

>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