All of lore.kernel.org
 help / color / mirror / Atom feed
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.

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.