linux-kernel - Re: [PATCH] usb: xhci: Handle quirky SuperSpeed isoc error reporting by Etron HCs

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: <41847336-9111-4aaa-b3dc-f3c18bb03508@linux.intel.com>
Date: Fri, 28 Feb 2025 18:13:50 +0200
From: Mathias Nyman <mathias.nyman@...ux.intel.com>
To: Michał Pecio <michal.pecio@...il.com>,
 Kuangyi Chiang <ki.chiang65@...il.com>
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
 linux-usb@...r.kernel.org, mathias.nyman@...el.com, stable@...r.kernel.org
Subject: Re: [PATCH] usb: xhci: Handle quirky SuperSpeed isoc error reporting
 by Etron HCs

On 12.2.2025 10.12, Michał Pecio wrote:
> On Wed, 12 Feb 2025 13:59:49 +0800, Kuangyi Chiang wrote:
>>> +       if (xhci->quirks & XHCI_ETRON_HOST && td->urb->dev->speed == USB_SPEED_SUPER) {
>>> +               td->error_mid_td |= error_event;
>>> +               etron_quirk |= error_event;
>>
>> This would be the same as etron_quirk = error_event; right?
> 
> Yeah, same thing I guess.
> 
>> I tested this with Etron EJ168 and EJ188 under Linux-6.13.1. It works.
> 
> Well, I found one case where it doesn't work optimally. There is a
> separate patch to skip "Missed Service Error" TDs immediately when the
> error is reported and also to treat MSE as another 'error mid TD', so
> with this Etron patch we would end up expecting spurious success after
> an MSE on the last TRB.
> 
> Well, AFAIS, no such event is generated by Etron in this case so we are
> back to waiting till next event and then giving back the missed TD.
> 
> 
> Maybe I will seriously look into decoupling giveback and dequeue ptr
> tracking, not only for those spurious Etron events but everywhere.
> 
> Mathias is right that HW has no sensible reason to touch DMA buffers
> after an error, I will look if the spec is very explicit about it.
> If so, we could give back TDs after the first event and merely keep
> enough information to recognize and silently ignore further events.

This issue was left hanging, I'll clean up my proposal and send it as
a proper RFT PATCH.

Thanks
Mathias