linux-kernel - Re: [PATCH 1/2] Revert "usb: dwc3: gadget: Fix incorrect UDC state after manual deconfiguration"

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: <109547e4-fc2a-42b6-9f16-6e0979b9a49b@quicinc.com>
Date: Fri, 28 Feb 2025 18:33:38 -0800
From: Wesley Cheng <quic_wcheng@...cinc.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: [PATCH 1/2] Revert "usb: dwc3: gadget: Fix incorrect UDC state
 after manual deconfiguration"

Hi Thinh,

On 2/28/2025 6:01 PM, Thinh Nguyen wrote:
> Hi,
> 
> On Fri, Feb 28, 2025, Wesley Cheng wrote:
>> This reverts commit 1ff24d40b3c3c673d833c546f898133b80dffc39.
>>
>> The following change creates a potential use after freed scenario within
>> the UDC core level.  When transitioning from peripheral to host mode, the
>> DWC3 DRD will remove the USB UDC using usb_del_gadget().  As part of the
>> UDC removal routine, the UDC core will issue a pullup disable call back to
>> the DWC3 gadget driver, which will now call usb_gadget_set_state(), and
>> queues gadget->work.  If gadget->work is unable to complete before the USB
>> gadget is freed, then we will run into the use after freed situation.
>>
>> Below is the callstack in question:
>>
>> dwc3_gadget_exit()
>>  --> usb_del_gadget()
>>  --> gadget_unbind_driver()
>>   --> device_del()
>>    --> gadget_unbind_driver()
>>     --> usb_gadget_disconnect_locked()
>>      --> pullup(0)
>>       --> dwc3_gadget_soft_disconnect()
>>        --> usb_gadget_set_state(UNATTACHED)
>>         --> queue_work(gadget->work) --> async
>>   --> device_unregister()
>>
>> Unfortunately, the flush_work() call within usb_del_gadget() doesn't work
>> in this situation, as it happens before the device_del() and soft
>> disconnect sequence.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
>> ---
>>  drivers/usb/dwc3/gadget.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 5156fafbda6c..921f72995dbf 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2781,8 +2781,6 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc)
>>  	__dwc3_gadget_stop(dwc);
>>  	spin_unlock_irqrestore(&dwc->lock, flags);
>>  
>> -	usb_gadget_set_state(dwc->gadget, USB_STATE_NOTATTACHED);
>> -
>>  	return ret;
>>  }
>>  
> 
> Have you tried with this fix?
> https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=399a45e5237ca14037120b1b895bd38a3b4492ea
> 

Thanks for pointing that out.  Looks like its working fine for me.  Bit odd
that we're still potentially utilizing the usb gadget after calling
device_del(), but I guess that should be fine.

Thanks
Wesley Cheng