linux-kernel - Re: [PATCH] usb: dwc3: gadget: Handle EP0 request dequeuing properly

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: <5aa271d5-1d7b-ed6a-2066-ad86ed5edc8b@quicinc.com>
Date:   Tue, 5 Dec 2023 12:10:43 -0800
From:   Wesley Cheng <quic_wcheng@...cinc.com>
To:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>
CC:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: dwc3: gadget: Handle EP0 request dequeuing properly

Hi Thinh,

On 12/4/2023 5:46 PM, Thinh Nguyen wrote:
> On Tue, Nov 21, 2023, Wesley Cheng wrote:
>> Current EP0 dequeue path will share the same as other EPs.  However, there
>> are some special considerations that need to be made for EP0 transfers:
>>
>>    - EP0 transfers never transition into the started_list
>>    - EP0 only has one active request at a time
>>
>> In case there is a vendor specific control message for a function over USB
>> FFS, then there is no guarantee on the timeline which the DATA/STATUS stage
>> is responded to.  While this occurs, any attempt to end transfers on
>> non-control EPs will end up having the DWC3_EP_DELAY_STOP flag set, and
>> defer issuing of the end transfer command.  If the USB FFS application
>> decides to timeout the control transfer, or if USB FFS AIO path exits, the
>> USB FFS driver will issue a call to usb_ep_dequeue() for the ep0 request.
>>
>> In case of the AIO exit path, the AIO FS blocks until all pending USB
>> requests utilizing the AIO path is completed.  However, since the dequeue
>> of ep0 req does not happen properly, all non-control EPs with the
>> DWC3_EP_DELAY_STOP flag set will not be handled, and the AIO exit path will
>> be stuck waiting for the USB FFS data endpoints to receive a completion
>> callback.
>>
>> Fix is to utilize dwc3_ep0_reset_state() in the dequeue API to ensure EP0
>> is brought back to the SETUP state, and ensures that any deferred end
>> transfer commands are handled.  This also will end any active transfers
>> on EP0, compared to the previous implementation which directly called
>> giveback only.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
>> ---
>>   drivers/usb/dwc3/gadget.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 858fe4c299b7..88d8d589f014 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2103,7 +2103,17 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>>   
>>   	list_for_each_entry(r, &dep->pending_list, list) {
>>   		if (r == req) {
>> -			dwc3_gadget_giveback(dep, req, -ECONNRESET);
>> +			/*
>> +			 * Explicitly check for EP0/1 as dequeue for those
>> +			 * EPs need to be handled differently.  Control EP
>> +			 * only deals with one USB req, and giveback will
>> +			 * occur during dwc3_ep0_stall_and_restart().  EP0
>> +			 * requests are never added to started_list.
>> +			 */
>> +			if (dep->number > 1)
>> +				dwc3_gadget_giveback(dep, req, -ECONNRESET);
>> +			else
>> +				dwc3_ep0_reset_state(dwc);
>>   			goto out;
>>   		}
>>   	}
> 
> Should we add Fixes tag?
> 

Sure will add and resubmit.

Thanks
Wesley Cheng