linux-kernel - Re: [PATCH v2 5/5] xhci: Correct handling of one-TRB isoc TD on Etron xHCI host

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: <20241028105451.0e2e92a7@foxbook>
Date: Mon, 28 Oct 2024 10:54:51 +0100
From: Michał Pecio <michal.pecio@...il.com>
To: ki.chiang65@...il.com
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
 linux-usb@...r.kernel.org, mathias.nyman@...el.com
Subject: Re: [PATCH v2 5/5] xhci: Correct handling of one-TRB isoc TD on
 Etron xHCI host

Hi,

That's a bug I'm familiar with.

> Unplugging a USB3.0 webcam while streaming results in errors
> like this

Not only unplugging but also any random error due to EMI or bad cable.

> If an error is detected while processing an one-TRB isoc TD,
> the Etron xHC generates two transfer events for the TRB that
> the error was detected on. The first event is "USB Transcation
> Error", and the second event is "Success".

IIRC, it wasn't just Transaction Errors but any sort of error, like
Babble or Bandwidth Overrun. But not sure about Missed Service, etc.

And IIRC I confirmed that it was *not* the case on Short Packet.

Also, I'm 99% sure the problem is not limited to one-TRB TDs, but
it occurs every time there is an error on the last TRB of any TD.

> As a solution, we can set the flag after the first error event
> and don't print the error message after the second event if the
> flag is set.

Yes, but I think it would be better to use error_mid_td instead of
last_td_was_short, so that the TD is only freed on the final event,
not on the first one.

The spec is clear that we should only free TRBs when the xHC is done
with them. Maybe it wouldn't be a problem in this case, and it surely
wouldn't be worse than what happens with Etron today, but IMO it could
be a real (even if rare) problem in other cases when this flag is used,
so I would rather remove the flag and handle short packets as per spec.

Regards,
Michal