linux-kernel - Re: [PATCH v1 1/2] usb: xhci: Skip xhci_reset in xhci

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250523230633.u46zpptaoob5jcdk@synopsys.com>
Date: Fri, 23 May 2025 23:06:36 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: "mathias.nyman@...el.com" <mathias.nyman@...el.com>,
        Roy Luo
	<royluo@...gle.com>
CC: "quic_ugoswami@...cinc.com" <quic_ugoswami@...cinc.com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "michal.pecio@...il.com" <michal.pecio@...il.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v1 1/2] usb: xhci: Skip xhci_reset in xhci_resume if xhci
 is being removed

Hi Mathias, Roy,

On Thu, May 22, 2025, Roy Luo wrote:
> xhci_reset() currently returns -ENODEV if XHCI_STATE_REMOVING is
> set, without completing the xhci handshake, unless the reset completes
> exceptionally quickly. This behavior causes a regression on Synopsys
> DWC3 USB controllers with dual-role capabilities.
> 
> Specifically, when a DWC3 controller exits host mode and removes xhci
> while a reset is still in progress, and then attempts to configure its
> hardware for device mode, the ongoing, incomplete reset leads to
> critical register access issues. All register reads return zero, not
> just within the xHCI register space (which might be expected during a
> reset), but across the entire DWC3 IP block.
> 
> This patch addresses the issue by preventing xhci_reset() from being
> called in xhci_resume() and bailing out early in the reinit flow when
> XHCI_STATE_REMOVING is set.
> 
> Cc: stable@...r.kernel.org
> Fixes: 6ccb83d6c497 ("usb: xhci: Implement xhci_handshake_check_state() helper")
> Suggested-by: Mathias Nyman <mathias.nyman@...el.com>
> Signed-off-by: Roy Luo <royluo@...gle.com>
> ---
>  drivers/usb/host/xhci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 90eb491267b5..244b12eafd95 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1084,7 +1084,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
>  		xhci_dbg(xhci, "Stop HCD\n");
>  		xhci_halt(xhci);
>  		xhci_zero_64b_regs(xhci);
> -		retval = xhci_reset(xhci, XHCI_RESET_LONG_USEC);
> +		if (xhci->xhc_state & XHCI_STATE_REMOVING)
> +			retval = -ENODEV;
> +		else
> +			retval = xhci_reset(xhci, XHCI_RESET_LONG_USEC);

How can this prevent the xhc_state from changing while in reset? There's
no locking in xhci-plat.

I would suggest to simply revert the commit 6ccb83d6c497 that causes
regression first. We can investigate and look into a solution to the
specific Qcom issue afterward.

Note that this commit may impact role-switching flow for all DRD dwc3
(and perhaps others), which may also impact other Qcom DRD platforms.

Thanks,
Thinh

>  		spin_unlock_irq(&xhci->lock);
>  		if (retval)
>  			return retval;
> -- 
> 2.49.0.1204.g71687c7c1d-goog
>