From: "Michał Pecio" <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Niklas Neronin <niklas.neronin@linux.intel.com>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/5] usb: xhci: Don't skip on Stopped - Length Invalid
Date: Wed, 26 Feb 2025 23:38:15 +0100 [thread overview]
Message-ID: <20250226233815.46d2f053@foxbook> (raw)
In-Reply-To: <20250226080255.770ca055@foxbook>
On Wed, 26 Feb 2025 08:02:55 +0100, Michal Pecio wrote:
> After d56b0b2ab142, TDs are immediately skipped when handling those
> Stopped events. This poses a potential problem in case of Stopped -
> Length Invalid, which occurs either on completed TDs (likely already
> given back) or Link and No-Op TRBs. Such event won't be recognized
> as matching any TD (unless it's the rare Link TRB inside a TD) and
> will result in skipping all pending TDs, giving them back possibly
> before they are done, risking isoc data loss and maybe UAF by HW.
Actually, Stopped and Stopped - Short Packet may be unsafe too.
As far as I understand, one of those (depending on SPC capability)
can occur on the second TRB of a TD whose first TRB completed with
Short Packet. Then the TD is already given back and removed from
td_list, so no match will be found with this Stopped event.
I suspect this is the reason why the driver has a policy to silently
ignore Stopped events which don't match the pending TD, and not only
Stopped - Length Invalid. Not sure why Stopped - Short Packet isn't
also ignored and yet apparently doesn't cause problems.
next prev parent reply other threads:[~2025-02-26 22:38 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-26 7:02 [PATCH v3 0/5] xHCI: Isochronous error handling fixes and improvements Michal Pecio
2025-02-26 7:02 ` [PATCH v3 1/5] usb: xhci: Don't skip on Stopped - Length Invalid Michal Pecio
2025-02-26 22:38 ` Michał Pecio [this message]
2025-02-26 7:03 ` [PATCH v3 2/5] usb: xhci: Complete 'error mid TD' transfers when handling Missed Service Michal Pecio
2025-02-26 7:04 ` [PATCH v3 3/5] usb: xhci: Fix isochronous Ring Underrun/Overrun event handling Michal Pecio
2025-02-26 7:05 ` [PATCH v3 4/5] usb: xhci: Expedite skipping missed isoch TDs on modern HCs Michal Pecio
2025-02-26 7:05 ` [PATCH v3 5/5] usb: xhci: Skip only one TD on Ring Underrun/Overrun Michal Pecio
2025-02-26 12:41 ` [PATCH v3 0/5] xHCI: Isochronous error handling fixes and improvements Mathias Nyman
2025-02-26 22:05 ` Michał Pecio
2025-02-27 12:10 ` Mathias Nyman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250226233815.46d2f053@foxbook \
--to=michal.pecio@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
--cc=niklas.neronin@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).