linux-kernel - Re: [RFT PATCH] xhci: Handle spurious events on Etron host isoc enpoints

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: <20250306095019.2e354de3@foxbook>
Date: Thu, 6 Mar 2025 09:50:19 +0100
From: Michał Pecio <michal.pecio@...il.com>
To: Mathias Nyman <mathias.nyman@...ux.intel.com>
Cc: ki.chiang65@...il.com, gregkh@...uxfoundation.org,
 linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFT PATCH] xhci: Handle spurious events on Etron host isoc
 enpoints

On Mon, 3 Mar 2025 17:08:39 +0200, Mathias Nyman wrote:
> > The hack could almost be removed now, but if there really are HCs
> > which report Success on the first event, this won't work for them:  
> 
> This looks better, and I agree that the hack/quirk is annoying, but
> in fear of regression I don't want to touch that in this patch yet.

For the record, I didn't mean removing support for HCs reporting
Success with nonzero residual, the problem may be real and the commit
which introduced this code describes plausible symptoms.

But handle_tx_event() part of this workaround could be done without
changing trb_comp_code, like process_xxx_td() are. You are replacing
practically all of this code already, so it's an opportunity.

And one more thing:

> -				ep_ring->last_td_was_short = false;
...
> +				ep_ring->old_trb_comp_code = trb_comp_code;

This is a behavior change, due to the aforementioned hack. You are
replacing comp_code 13 with 13 and the mechanism stays "armed". It
will continue silently ignoring arbitrary events because there is
no validation if they really came from the "old" TD's TRB.

It's a pet peeve of mine, because I have already seen cases when the
old mechanism "swallows" illegal events which should be reported, and
now this problem may only get worse.

You can preserve behavior by clearing old_trb_comp_code to 0 or -1
or otherwise marking this entry as "inactive".

Regards,
Michal