linux-kernel - Re: [PATCH v2] 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]

Date: Fri, 22 Dec 2023 22:41:48 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Wesley Cheng <quic_wcheng@...cinc.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        "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 v2] usb: dwc3: gadget: Handle EP0 request dequeuing
 properly

On Wed, Dec 06, 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.
> 
> Fixes: fcd2def66392 ("usb: dwc3: gadget: Refactor dwc3_gadget_ep_dequeue")
> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
> ---
> Changes from v1:
>  - Added fixes tag
> 
>  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;
>  		}
>  	}

Acked-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>

Thanks,
Thinh