linux-kernel - Re: [PATCH v4 0/3] Avoid having pending end transfers on soft disconnect

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: <2b514830-f179-cf13-3f16-3fafce033286@quicinc.com>
Date:   Thu, 13 Apr 2023 12:58:53 -0700
From:   Wesley Cheng <quic_wcheng@...cinc.com>
To:     <gregkh@...uxfoundation.org>, <Thinh.Nguyen@...opsys.com>
CC:     <linux-kernel@...r.kernel.org>, <linux-usb@...r.kernel.org>
Subject: Re: [PATCH v4 0/3] Avoid having pending end transfers on soft
 disconnect

Sorry please disregard this submission, tools used added improper stable 
tag.

Thanks
Wesley Cheng

On 4/13/2023 12:53 PM, Wesley Cheng wrote:
> In case there is a host which takes time to complete a SETUP transaction,
> during the soft disconnect sequence multiple DWC3 EPs will have their
> DWC3_EP_DELAY_STOP flag set w/o issuing the end transfer command.  Once the
> controller halt sequence occurs, the soft disconnect is successful, and
> the subsequent soft connect will attempt to flush the pending end transfers.
> 
> Soft disconnect sequence:
>    dwc3_gadget_ep_disable   name=ep8in flags=0x3009  direction=1
>    dwc3_gadget_ep_disable   name=ep4in flags=1  direction=1
>    dwc3_gadget_ep_disable   name=ep3out flags=1  direction=0
>    usb_gadget_disconnect   deactivated=0  connected=0  ret=0
> 
> Soft connect bug:
>    BUG: spinlock already unlocked on CPU
>    spin_bug+0x0
>    dwc3_remove_requests+0x278
>    dwc3_ep0_out_start+0xb0
>    __dwc3_gadget_start+0x25c
> 
> The bug occurs due to the flush of the pending end transfers, as the gadget
> start routine is not held with a spinlock.  However, if the DWC3_EP_DELAY_STOP
> is set, it will call the giveback API, which attempts to unlock the dwc->lock.
> Ideally, the DWC3 gadget should not have pending end transfers on a soft
> connect, so fix this by:
> 
>   1. Re-locating the SETUP phase check after stop active transfers, since
>   that is where the DWC3_EP_DELAY_STOP is potentially set.  This also allows
>   for handling of a host that may be unresponsive by using the completion
>   timeout to trigger the stall and restart for EP0.
> 
>   2. Do not call gadget stop until the poll for controller halt is
>   completed.  DEVTEN is cleared as part of gadget stop, so the intention to
>   allow ep0 events to continue while waiting for controller halt is not
>   happening.
> 
> Changes in v4:
> - Added stable to patches that address the actual issue
> - Re-ordered patch series to include the refactor change at the end to remove
>    any dependencies to the actual fix.
> 
> Changes in v3:
> - Removed fixes tag in the refactor change
> 
> Wesley Cheng (3):
>    usb: dwc3: gadget: Stall and restart EP0 if host is unresponsive
>    usb: dwc3: gadget: Execute gadget stop after halting the controller
>    usb: dwc3: gadget: Refactor EP0 forced stall/restart into a separate
>      API
> 
>   drivers/usb/dwc3/gadget.c | 101 ++++++++++++++++++++++----------------
>   1 file changed, 58 insertions(+), 43 deletions(-)
>