linux-kernel - Re: [PATCH 5/5] usb: xhci: Skip only one TD on Ring Underrun/Overrun

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: <e72b2f2b-d327-49f6-bf16-d846e9283e00@linux.intel.com>
Date: Fri, 21 Feb 2025 15:17:19 +0200
From: Mathias Nyman <mathias.nyman@...ux.intel.com>
To: Michał Pecio <michal.pecio@...il.com>
Cc: Mathias Nyman <mathias.nyman@...el.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Niklas Neronin <niklas.neronin@...ux.intel.com>, linux-usb@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/5] usb: xhci: Skip only one TD on Ring Underrun/Overrun

On 21.2.2025 3.17, Michał Pecio wrote:
> On Tue, 11 Feb 2025 17:41:39 +0200, Mathias Nyman wrote:
>> On 10.2.2025 9.42, Michal Pecio wrote:
>>> +				if (ring_xrun_event) {
>>> +					/*
>>> +					 * If we are here, we are on xHCI 1.0 host with no idea how
>>> +					 * many TDs were missed and where the xrun occurred. Don't
>>> +					 * skip more TDs, they may have been queued after the xrun.
>>> +					 */
>>> +					xhci_dbg(xhci, "Skipped one TD for slot %u ep %u",
>>> +							slot_id, ep_index);
>>> +					break;
>>
>> This would be the same as return 0; right?
>>
>> Whole series looks good, I'll add it
> 
> I hope you haven't sent it out yet because I found two minor issues.
> 
> 
> Firstly,
> [PATCH 3/5] usb: xhci: Fix isochronous Ring Underrun/Overrun event handling
> 
> increases the number of xrun events that we handle but doesn't suppress
> the "Event TRB for slot %u ep %u with no TDs queued\n" warning, so the
> warning started to show up sometimes for no good reason. The fix is to
> add ring_xrun_event to the list of exception for this warning.
> 
> 
> Secondly,
> [PATCH 5/5] usb: xhci: Skip only one TD on Ring Underrun/Overrun
> 
> can be improved to clear the skip flag if skipped TD was the only one.
> This eliminates any confusion and risk of skipping bugs in the future.
> The change is a matter of moving that code to a different branch.
> 
> I also changed 'break' to 'return 0' because it gets hard to follow at
> this level of indentation.
> 
> 
> I'll send a v2 of those two patches. Sorry for any inconvenience.

Patches updated, they are now in my for-usb-next branch

Thanks
Mathias