linux-kernel - Re: [PATCH v2 1/2] xhci: Add a quirk for full reset on removal

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+zupgwSVRNyf40JiDi6ugSLHX_rXkyS2=pwc9_VHsSXj4AV5g@mail.gmail.com>
Date: Fri, 16 May 2025 16:11:20 -0700
From: Roy Luo <royluo@...gle.com>
To: Michał Pecio <michal.pecio@...il.com>
Cc: Thinh Nguyen <Thinh.Nguyen@...opsys.com>, 
	"mathias.nyman@...el.com" <mathias.nyman@...el.com>, 
	"quic_ugoswami@...cinc.com" <quic_ugoswami@...cinc.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>, 
	"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] xhci: Add a quirk for full reset on removal

> There's no state 0. Checking against that is odd. Couldn't we just use
> xhci_handshake() equivalent instead?

Ok, I will change it in the next version.

On Thu, May 15, 2025 at 11:33 PM Michał Pecio <michal.pecio@...il.com> wrote:
>
> On Thu, 15 May 2025 23:42:50 +0000, Thinh Nguyen wrote:
> > In any case, this is basically a revert of this change:
> > 6ccb83d6c497 ("usb: xhci: Implement xhci_handshake_check_state()
> > helper")
> >
> > Can't we just revert or fix the above patch that causes a regression?
>
> Also note that 6ccb83d6c497 claimed to fix actual problems, so
> disabling it on selected hardware could bring the old bug back:
>
> > In some situations where xhci removal happens parallel to
> > xhci_handshake, we encounter a scenario where the xhci_handshake
> > can't succeed, and it polls until timeout.
> >
> > If xhci_handshake runs until timeout it can on some platforms result
> > in a long wait which might lead to a watchdog timeout.

On top of this, xhci_handshake_check_state(XHCI_STATE_REMOVING)
is also used elsewhere like xhci_abort_cmd_ring(), so a simple revert is
off the table. Commit 6ccb83d6c497 did not specify which platform and
in what circumstance would xhci handshake timeout, adding a quirk for
DWC3 seems to be the better option here.

>
> But on the other hand, xhci_handshake() has long timeouts because
> the handshakes themselves can take a surprisingly long time (and
> sometimes still succeed), so any reliance on handshake completing
> before timeout is frankly a bug in itself.

This patch simply honors the contract between the software and
hardware, allowing the handshake to complete. It doesn't assume the
handshake will finish on time. If it times out, then it times out and
returns a failure.

Thanks,
Roy