public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Michael Bommarito <michael.bommarito@gmail.com>
To: Mika Westerberg <westeri@kernel.org>, linux-usb@vger.kernel.org
Cc: Andreas Noever <andreas.noever@gmail.com>,
	Yehezkel Bernat <YehezkelShB@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Michael Jamet <michael.jamet@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: [PATCH v3 0/4] thunderbolt: harden XDomain property parser
Date: Sun,  3 May 2026 10:15:04 -0400	[thread overview]
Message-ID: <cover.1777817011.git.michael.bommarito@gmail.com> (raw)
In-Reply-To: <20260415123221.225149-1-michael.bommarito@gmail.com>

Caught a few more cases, so this addresses both your cosmetic asks
on v2 and the more material changes noted in each patch below.

Three independent memory-safety defects in drivers/thunderbolt/property.c
are reachable when an untrusted Thunderbolt/USB4 XDomain peer responds
to a PROPERTIES_REQUEST during host-to-host discovery.  The peer
supplies up to TB_XDP_PROPERTIES_MAX_LENGTH (500) dwords of attacker-
controlled property block which the local host passes to
tb_property_parse_dir() as part of the control-plane exchange that
runs before any tunnels are set up.

Patches 1-3 are one bug per patch: u32 overflow in
tb_property_entry_valid(), short-dir_len OOB+underflow in
__tb_property_parse_dir(), and unbounded recursion in the same.
Patch 4 is three KUnit regression cases exercising all three.

All three defects are OOB-read or DoS at worst.  No controlled OOB
write is reachable through the parser; parse_dwdata()'s destination
is a freshly kcalloc'd buffer sized by entry->length.

Operators who do not need XDomain host-to-host discovery can disable
the path entirely with thunderbolt.xdomain=0 on the kernel command
line.

Reproduced on v7.0-rc7 + CONFIG_KASAN=y + CONFIG_USB4_KUNIT_TEST=y
via the KUnit suite in patch 4.  Pre-fix on a v7.0-rc7 + patch 4
kernel: u32_wrap fails with a KASAN use-after-free trace in
__tb_property_parse_dir() (the parser reads ~16 GiB past the
block); recursion fails with KASAN + an Oops on RIP=0 as the
parser exhausts its guard page.  dir_len_underflow returns NULL
on pre-fix because the downstream content_len = dir_len - 4
underflow makes the entry walk bail at tb_property_entry_valid();
the UUID kmemdup over-read is silent here because KASAN-Generic's
slab redzones do not flag a 4-byte over-read into the
kmalloc-chunk tail.  Treat dir_len_underflow as the post-fix
invariant pin; u32_wrap and recursion are the active pre-fix
detectors.

Post-fix (all four patches): all three pass cleanly with KASAN
active.

Changes since v2
----------------

Material:

  - Patch 2/4: move "dir_len < 4" reject before the UUID kmemdup
    in the non-root parse path.  v2 placed it after, so a crafted
    entry with dir_offset near end of block and dir_len in 0..3
    OOB-read up to 4 dwords past the block before the reject ran
    (dir_offset=497, dir_len=3, block_len=500 reads
    block[497..501]).  Both that OOB and the original
    content_len = dir_len - 4 underflow now hit the same gate.

  - Patch 4/4: tighten dir_len_underflow's buffer (7 dwords,
    kmalloc-32) and reposition the entry (e->value=4) to focus the
    UUID kmemdup on the chunk tail.  KASAN-Generic does not flag
    the 4-byte over-read into the tail, so the test remains a
    post-fix invariant pin (documented above); v2's wider buffer
    obscured even the post-fix-pin shape.

  - Patches 1/4, 2/4, 3/4: fix Fixes: SHA.  v2 used e69b6c02b4c3
    ("net: Add support for networking over Thunderbolt cable"),
    the wrong commit.  Correct is cdae7c07e3e3 ("thunderbolt: Add
    support for XDomain properties").

Cosmetic (per v2 review):

  - Lowercase 0xffffff00 in 1/4 and 4/4 commit messages, and 4/4
    code + comments.
  - Patch 4/4: hoist the on-wire entry layout into a single shared
    struct tb_test_property_entry instead of re-declaring an
    anonymous struct in each of the three tests.
  - Patch 4/4: use TB_PROPERTY_TYPE_DATA / TB_PROPERTY_TYPE_DIRECTORY
    constants from <linux/thunderbolt.h> instead of bare 0x64 / 0x44.
  - Patch 4/4: convert all multi-line block comments to put the
    opening "/*" on its own line per the thunderbolt subsystem's
    coding style.


Michael Bommarito (4):
  thunderbolt: property: reject u32 wrap in tb_property_entry_valid()
  thunderbolt: property: reject dir_len < 4 to prevent size_t underflow
  thunderbolt: property: cap recursion depth in
    __tb_property_parse_dir()
  thunderbolt: test: add KUnit regression tests for XDomain property
    parser

 drivers/thunderbolt/property.c |  32 +++++---
 drivers/thunderbolt/test.c     | 132 +++++++++++++++++++++++++++++++++
 2 files changed, 155 insertions(+), 9 deletions(-)

-- 
2.53.0


  parent reply	other threads:[~2026-05-03 14:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-15  3:23 [PATCH 0/2] thunderbolt: harden XDomain property parser Michael Bommarito
2026-04-15  3:23 ` [PATCH 1/2] thunderbolt: property: harden XDomain property parser against crafted peer Michael Bommarito
2026-04-15  4:52   ` Mika Westerberg
2026-04-15 11:41     ` Michael Bommarito
2026-04-15  3:23 ` [PATCH 2/2] thunderbolt: test: add KUnit regression tests for XDomain property parser Michael Bommarito
2026-04-15 12:32 ` [PATCH v2 0/4] thunderbolt: harden " Michael Bommarito
2026-04-15 12:32   ` [PATCH v2 1/4] thunderbolt: property: reject u32 wrap in tb_property_entry_valid() Michael Bommarito
2026-04-27  5:35     ` Mika Westerberg
2026-05-02 17:55       ` Michael Bommarito
2026-04-15 12:32   ` [PATCH v2 2/4] thunderbolt: property: reject dir_len < 4 to prevent size_t underflow Michael Bommarito
2026-04-15 12:32   ` [PATCH v2 3/4] thunderbolt: property: cap recursion depth in __tb_property_parse_dir() Michael Bommarito
2026-04-15 12:32   ` [PATCH v2 4/4] thunderbolt: test: add KUnit regression tests for XDomain property parser Michael Bommarito
2026-04-27  5:40     ` Mika Westerberg
2026-05-03 14:15   ` Michael Bommarito [this message]
2026-05-03 14:15     ` [PATCH v3 1/4] thunderbolt: property: reject u32 wrap in tb_property_entry_valid() Michael Bommarito
2026-05-03 14:15     ` [PATCH v3 2/4] thunderbolt: property: reject dir_len < 4 to prevent size_t underflow Michael Bommarito
2026-05-03 14:15     ` [PATCH v3 3/4] thunderbolt: property: cap recursion depth in __tb_property_parse_dir() Michael Bommarito
2026-05-03 14:15     ` [PATCH v3 4/4] thunderbolt: test: add KUnit regression tests for XDomain property parser Michael Bommarito

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=cover.1777817011.git.michael.bommarito@gmail.com \
    --to=michael.bommarito@gmail.com \
    --cc=YehezkelShB@gmail.com \
    --cc=andreas.noever@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=michael.jamet@intel.com \
    --cc=stable@vger.kernel.org \
    --cc=westeri@kernel.org \
    /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