Re: [PATCH v3 3/4] thunderbolt: property: cap recursion depth in __tb_property_parse_dir()

Re: [PATCH v3 3/4] thunderbolt: property: cap recursion depth in __tb_property_parse_dir()

[Michael Bommarito]

On Mon, May 4, 2026 at 5:01 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> I would leave this on a single line (yes, slightly longer than 80 characters).

Sounds good.  I'll give it another ~day for anyone else (like Mika) to
weigh in, then send a v4 with your updates on 1/2/3

Re: [PATCH v3 4/4] thunderbolt: test: add KUnit regression tests for XDomain property parser

[Mika Westerberg]

Hi,

On Sun, May 03, 2026 at 10:15:08AM -0400, Michael Bommarito wrote:
> +/*
> + * Reproducers for three memory-safety defects in
> + * drivers/thunderbolt/property.c reached from a crafted XDomain
> + * PROPERTIES_RESPONSE payload.  Without the fix these trip KASAN or
> + * smash the kernel stack; with the fix each returns NULL cleanly.
> + *
> + * The on-wire entry layout mirrors struct tb_property_entry in
> + * property.c (private to that translation unit).
> + */
> +struct tb_test_property_entry {
> +	u32 key_hi, key_lo;
> +	u16 length;
> +	u8 reserved;
> +	u8 type;
> +	u32 value;
> +};

If possible, can you make the below just be u32 as we have root_directory
already? That way we don't need to duplicate this and I think with the AI
should not be a big deal.

[The other option is to move the structure into tb.h as that's internal but
I prefer the u32 array in tests].

> +static void tb_test_property_parse_u32_wrap(struct kunit *test)
> +{
> +	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);

This one is fine to be dynamic and just fill in below what is needed as
u32.

Otherwise this is good and I like the fact that you added the tests so I
can just first apply the test patch and see that it breaks left and right
and then apply fixes and expect it now passes :)

[PATCH v4 0/4] thunderbolt: harden XDomain property parser

[Michael Bommarito]

Style cleanups only on top of v3.  Andy's three nits on 1/4, 2/4,
3/4 are applied; Mika's request to drop the duplicated on-wire
entry struct in 4/4 is applied.  No behavioural change to any
patch; the bug analysis and the gating in patches 1-3 are
unchanged.

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 v3
----------------

Cosmetic (per v3 review):

  - Patch 1/4 (Andy Shevchenko): drop the redundant (u32) cast on
    entry->length in check_add_overflow().  __builtin_add_overflow
    handles mixed width via implicit promotion; the cast was noise.

  - Patch 2/4 (Andy Shevchenko): insert a blank line between the
    !dir error return and the new INIT_LIST_HEAD(&dir->properties).

  - Patch 3/4 (Andy Shevchenko): keep the four-argument
    tb_property_parse(block, block_len, &entries[i], depth) on a
    single line (>80 col) instead of wrapping the trailing argument.

  - Patch 4/4 (Mika Westerberg): drop the private
    struct tb_test_property_entry overlay.  Populate the crafted
    blocks by writing u32 dwords directly, matching the existing
    root_directory[] style used elsewhere in this file.  Each test's
    kunit_kzalloc is right-sized to the dwords needed to actually
    exercise the bug (0x102 for u32_wrap, 10 for recursion, 7 for
    dir_len_underflow); the 500-dword allocation v3 used has been
    dropped.

    u32_wrap retains length=0x100 / value=0xffffff00 from v3 so
    the entry's length field clears the "entry->length > block_len"
    gate (block_len = 0x102 dwords) and the wrap check is what
    actually fires.  recursion uses length=8 (was 16 in v3) so the
    smaller block can hold both the parent UUID kmemdup and the
    single child entry that drives the recursion.  All three
    pre-fix scenarios are still observable: u32_wrap page-faults
    on the KASAN shadow lookup for the wild OOB offset, recursion
    trips a KASAN out-of-bounds report in __unwind_start as the
    per-task kernel stack is consumed, dir_len_underflow trips
    KASAN slab-out-of-bounds in kmemdup_noprof.  Post-fix all
    three pass.

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: use TB_PROPERTY_TYPE_DATA / TB_PROPERTY_TYPE_DIRECTORY
    constants from <linux/thunderbolt.h> instead of bare 0x64 / 0x44.
    (v4 reverts to bare hex in the u32 dword that packs (type <<
    24), since the type byte is now part of a packed dword literal;
    each dword carries a `/* type=... */` comment.)
  - 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     | 126 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 149 insertions(+), 9 deletions(-)

--
2.53.0

[PATCH v4 1/4] thunderbolt: property: reject u32 wrap in tb_property_entry_valid()

[Michael Bommarito]

entry->value is u32 and entry->length is u16; the sum is performed in
u32 and wraps.  A malicious XDomain peer can pick
value = 0xffffff00, length = 0x100 so the sum 0x100000000 wraps to 0
and passes the > block_len check.  tb_property_parse() then passes
entry->value to parse_dwdata() as a dword offset into the property
block, reading attacker-directed memory far past the allocation.

For TEXT-typed entries with the "deviceid" or "vendorid" keys this
lands in xd->device_name / xd->vendor_name and is readable back via
the per-XDomain device_name / vendor_name sysfs attributes; the leak
is NUL-bounded (kstrdup() stops at the first zero byte) and
untargeted (the attacker picks a delta, not an absolute address).
DATA-typed entries are parsed into property->value.data but not
generically surfaced to userspace.

Use check_add_overflow() so a wrapped sum is rejected.

Fixes: cdae7c07e3e3 ("thunderbolt: Add support for XDomain properties")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-6
Assisted-by: Codex:gpt-5-4
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/thunderbolt/property.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index 50cbfc92fe65..29cd60c11ac4 100644
--- a/drivers/thunderbolt/property.c
+++ b/drivers/thunderbolt/property.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/err.h>
+#include <linux/overflow.h>
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/uuid.h>
@@ -52,13 +53,16 @@ static inline void format_dwdata(void *dst, const void *src, size_t dwords)
 static bool tb_property_entry_valid(const struct tb_property_entry *entry,
 				  size_t block_len)
 {
+	u32 end;
+
 	switch (entry->type) {
 	case TB_PROPERTY_TYPE_DIRECTORY:
 	case TB_PROPERTY_TYPE_DATA:
 	case TB_PROPERTY_TYPE_TEXT:
 		if (entry->length > block_len)
 			return false;
-		if (entry->value + entry->length > block_len)
+		if (check_add_overflow(entry->value, entry->length, &end) ||
+		    end > block_len)
 			return false;
 		break;
 
-- 
2.53.0

[PATCH v4 2/4] thunderbolt: property: reject dir_len < 4 to prevent size_t underflow

[Michael Bommarito]

On the non-root path, __tb_property_parse_dir() takes dir_len from
entry->length (u16 widened to size_t).  Two distinct OOB conditions
follow when entry->length < 4:

1. The non-root path begins with kmemdup(&block[dir_offset],
   sizeof(*dir->uuid), ...) which always reads 4 dwords from
   dir_offset.  tb_property_entry_valid() only enforces
   dir_offset + entry->length <= block_len, so a crafted entry
   with dir_offset close to the end of the property block and
   entry->length in 0..3 passes that gate but lets the UUID copy
   run off the block (e.g. dir_offset = 497, dir_len = 3 in a
   500-dword block reads block[497..501]).

2. After the kmemdup, content_len = dir_len - 4 underflows size_t
   to ~SIZE_MAX, nentries becomes SIZE_MAX / 4, and the entry
   walk runs OOB on each iteration until an entry fails
   validation or the kernel oopses on an unmapped page.

Reject dir_len < 4 on the non-root path *before* the UUID kmemdup,
which closes both holes.

Also move INIT_LIST_HEAD(&dir->properties) up to immediately after
the dir allocation so the new error-return path (and the existing
uuid-alloc failure path) calling tb_property_free_dir() sees a
walkable list rather than the zero-initialized NULL next/prev that
list_for_each_entry_safe() would oops on.

Fixes: cdae7c07e3e3 ("thunderbolt: Add support for XDomain properties")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-6
Assisted-by: Codex:gpt-5-4
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/thunderbolt/property.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index 29cd60c11ac4..74c92f9801ff 100644
--- a/drivers/thunderbolt/property.c
+++ b/drivers/thunderbolt/property.c
@@ -174,10 +174,16 @@ static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
 	if (!dir)
 		return NULL;
 
+	INIT_LIST_HEAD(&dir->properties);
+
 	if (is_root) {
 		content_offset = dir_offset + 2;
 		content_len = dir_len;
 	} else {
+		if (dir_len < 4) {
+			tb_property_free_dir(dir);
+			return NULL;
+		}
 		dir->uuid = kmemdup(&block[dir_offset], sizeof(*dir->uuid),
 				    GFP_KERNEL);
 		if (!dir->uuid) {
@@ -191,8 +197,6 @@ static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
 	entries = (const struct tb_property_entry *)&block[content_offset];
 	nentries = content_len / (sizeof(*entries) / 4);
 
-	INIT_LIST_HEAD(&dir->properties);
-
 	for (i = 0; i < nentries; i++) {
 		struct tb_property *property;
 
-- 
2.53.0

[PATCH v4 3/4] thunderbolt: property: cap recursion depth in __tb_property_parse_dir()

[Michael Bommarito]

A DIRECTORY entry's value field is used as the dir_offset for a
recursive call into __tb_property_parse_dir() with no depth counter.
A crafted peer that chains DIRECTORY entries into a back-reference
loop drives the parser until the kernel stack is exhausted and the
guard page fires.  Any untrusted XDomain peer (cable, dock, in-line
inspector, adjacent host) that reaches the PROPERTIES_REQUEST
control-plane exchange can trigger this without authentication.

Thread a depth counter through tb_property_parse() and
__tb_property_parse_dir(), and reject blocks that exceed
TB_PROPERTY_MAX_DEPTH = 8.  That is comfortably larger than any
observed legitimate XDomain layout.

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

Fixes: cdae7c07e3e3 ("thunderbolt: Add support for XDomain properties")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-6
Assisted-by: Codex:gpt-5-4
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/thunderbolt/property.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index 74c92f9801ff..da2c59a17db5 100644
--- a/drivers/thunderbolt/property.c
+++ b/drivers/thunderbolt/property.c
@@ -35,10 +35,11 @@ struct tb_property_dir_entry {
 };
 
 #define TB_PROPERTY_ROOTDIR_MAGIC	0x55584401
+#define TB_PROPERTY_MAX_DEPTH		8
 
 static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
 	size_t block_len, unsigned int dir_offset, size_t dir_len,
-	bool is_root);
+	bool is_root, unsigned int depth);
 
 static inline void parse_dwdata(void *dst, const void *src, size_t dwords)
 {
@@ -97,7 +98,8 @@ tb_property_alloc(const char *key, enum tb_property_type type)
 }
 
 static struct tb_property *tb_property_parse(const u32 *block, size_t block_len,
-					const struct tb_property_entry *entry)
+					const struct tb_property_entry *entry,
+					unsigned int depth)
 {
 	char key[TB_PROPERTY_KEY_SIZE + 1];
 	struct tb_property *property;
@@ -118,7 +120,7 @@ static struct tb_property *tb_property_parse(const u32 *block, size_t block_len,
 	switch (property->type) {
 	case TB_PROPERTY_TYPE_DIRECTORY:
 		dir = __tb_property_parse_dir(block, block_len, entry->value,
-					      entry->length, false);
+					      entry->length, false, depth + 1);
 		if (!dir) {
 			kfree(property);
 			return NULL;
@@ -163,13 +165,17 @@ static struct tb_property *tb_property_parse(const u32 *block, size_t block_len,
 }
 
 static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
-	size_t block_len, unsigned int dir_offset, size_t dir_len, bool is_root)
+	size_t block_len, unsigned int dir_offset, size_t dir_len, bool is_root,
+	unsigned int depth)
 {
 	const struct tb_property_entry *entries;
 	size_t i, content_len, nentries;
 	unsigned int content_offset;
 	struct tb_property_dir *dir;
 
+	if (depth > TB_PROPERTY_MAX_DEPTH)
+		return NULL;
+
 	dir = kzalloc_obj(*dir);
 	if (!dir)
 		return NULL;
@@ -200,7 +206,7 @@ static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
 	for (i = 0; i < nentries; i++) {
 		struct tb_property *property;
 
-		property = tb_property_parse(block, block_len, &entries[i]);
+		property = tb_property_parse(block, block_len, &entries[i], depth);
 		if (!property) {
 			tb_property_free_dir(dir);
 			return NULL;
@@ -239,7 +245,7 @@ struct tb_property_dir *tb_property_parse_dir(const u32 *block,
 		return NULL;
 
 	return __tb_property_parse_dir(block, block_len, 0, rootdir->length,
-				       true);
+				       true, 0);
 }
 
 /**
-- 
2.53.0

[PATCH v4 4/4] thunderbolt: test: add KUnit regression tests for XDomain property parser

[Michael Bommarito]

Add three KUnit cases that exercise the defects fixed by the sibling
commits in this series by feeding crafted XDomain property blocks to
tb_property_parse_dir():

  tb_test_property_parse_u32_wrap - entry->value = 0xffffff00 and
    entry->length = 0x100 so their u32 sum 0x100000000 wraps to 0
    under the block_len guard; without the fix the subsequent
    parse_dwdata() reads attacker-directed OOB memory.

  tb_test_property_parse_recursion - two DIRECTORY entries pointing
    at each other, driving __tb_property_parse_dir() recursion;
    without the fix the kernel stack is exhausted.

  tb_test_property_parse_dir_len_underflow - a DIRECTORY entry with
    length < 4 placed near the end of the block so the non-root UUID
    kmemdup of 4 dwords from dir_offset reads OOB before the later
    content_len = dir_len - 4 underflow path is reached.

Each test asserts tb_property_parse_dir() returns NULL on the
crafted input.  With CONFIG_KASAN=y, running these on the pre-fix
kernel produces an oops inside __tb_property_parse_dir or its
callees: u32_wrap takes a page fault on the KASAN shadow lookup for
the wild ~16 GiB OOB offset; recursion trips a KASAN out-of-bounds
report in __unwind_start as the per-task kernel stack is consumed;
dir_len_underflow trips a KASAN slab-out-of-bounds report in
kmemdup_noprof reading 16 bytes past the 28-byte block.  Post-fix
they pass cleanly.

The crafted blocks are populated by writing u32 dwords directly,
matching the existing root_directory[] style used elsewhere in
this file rather than imposing a private struct overlay.

Run with:
  ./tools/testing/kunit/kunit.py run --arch=x86_64 \
    --kconfig_add CONFIG_PCI=y --kconfig_add CONFIG_NVMEM=y \
    --kconfig_add CONFIG_USB4=y --kconfig_add CONFIG_USB4_KUNIT_TEST=y \
    --kconfig_add CONFIG_KASAN=y 'thunderbolt.tb_test_property_parse_*'

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/thunderbolt/test.c | 126 +++++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 1f4318249c22..f41fabf15456 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -2852,7 +2852,133 @@ static void tb_test_property_copy(struct kunit *test)
 	tb_property_free_dir(src);
 }
 
+/*
+ * Reproducers for three memory-safety defects in
+ * drivers/thunderbolt/property.c reached from a crafted XDomain
+ * PROPERTIES_RESPONSE payload.  Without the fix these trip KASAN or
+ * smash the kernel stack; with the fix each returns NULL cleanly.
+ *
+ * The on-wire entry layout matches struct tb_property_entry in
+ * property.c (private to that translation unit): u32 key_hi, u32
+ * key_lo, then a packed u32 = (type << 24) | (reserved << 16) |
+ * length, then u32 value.  Each entry is 4 dwords.
+ */
+
+static void tb_test_property_parse_u32_wrap(struct kunit *test)
+{
+	/*
+	 * 0x102 dwords: enough for the entry's length field (0x100) to
+	 * pass the "entry->length > block_len" gate so the wrap check
+	 * is actually exercised.  parse_dwdata's downstream OOB read
+	 * lands ~16 GiB past the allocation regardless.
+	 */
+	u32 *block = kunit_kzalloc(test, 0x102 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+
+	KUNIT_ASSERT_NOT_NULL(test, block);
+
+	block[0] = 0x55584401;	/* "UXD" v1 magic */
+	block[1] = 0x00000004;	/* Root directory length: one entry */
+
+	/*
+	 * DATA entry whose value 0xffffff00 + length 0x100 wrap to 0
+	 * in u32, passing the sum <= block_len guard even though the
+	 * real offset is far past the allocation.
+	 */
+	block[2] = 0x61616161;	/* key_hi */
+	block[3] = 0x61616161;	/* key_lo */
+	block[4] = 0x64000100;	/* type=DATA, reserved=0, length=0x100 */
+	block[5] = 0xffffff00;	/* value */
+
+	dir = tb_property_parse_dir(block, 0x102);
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
+static void tb_test_property_parse_recursion(struct kunit *test)
+{
+	/*
+	 * 10 dwords: rootdir header (2) + parent DIRECTORY entry (4) +
+	 * the child entry that lives at dir_offset(2) + UUID(4) = 6,
+	 * occupying block[6..9].  Each recursive level re-reads the
+	 * same block[6..9] as its first child entry, which is itself
+	 * a DIRECTORY pointing at offset 2.
+	 */
+	u32 *block = kunit_kzalloc(test, 10 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+
+	KUNIT_ASSERT_NOT_NULL(test, block);
+
+	block[0] = 0x55584401;	/* "UXD" v1 magic */
+	block[1] = 0x00000004;	/* Root directory length: one entry */
+
+	/*
+	 * DIRECTORY entry pointing at dir_offset = 2 with length = 8.
+	 * Non-root parse derives content_offset = 6, content_len = 4,
+	 * nentries = 1.  block[6..9] is read both as the parent's UUID
+	 * (kmemdup'd into dir->uuid) and as the single child entry --
+	 * which is itself a DIRECTORY pointing at offset 2, so the
+	 * recursion never terminates and the kernel stack is exhausted.
+	 */
+	block[2] = 0x61616161;	/* key_hi */
+	block[3] = 0x61616161;	/* key_lo */
+	block[4] = 0x44000008;	/* type=DIRECTORY, reserved=0, length=8 */
+	block[5] = 0x00000002;	/* value = dir_offset */
+
+	block[6] = 0x62626262;	/* doubles as UUID dword 0 / child key_hi */
+	block[7] = 0x62626262;	/* doubles as UUID dword 1 / child key_lo */
+	block[8] = 0x44000008;	/* type=DIRECTORY, reserved=0, length=8 */
+	block[9] = 0x00000002;	/* value = dir_offset (back at parent) */
+
+	dir = tb_property_parse_dir(block, 10);
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
+static void tb_test_property_parse_dir_len_underflow(struct kunit *test)
+{
+	/*
+	 * Allocate exactly 7 dwords (28 bytes) so the kmalloc-32 chunk
+	 * leaves a 4-byte slab redzone tail that KASAN-Generic can flag.
+	 * With block_len = 7, dir_offset = 4, dir_len = 3, the non-root
+	 * UUID kmemdup reads 16 bytes from byte 16, so bytes 28..31 fall
+	 * in the redzone and trip a KASAN slab-out-of-bounds report on
+	 * the pre-fix kernel.  Sizing the buffer at a power of two (32,
+	 * 64, ...) puts the over-read into the slab cache tail where
+	 * KASAN's generic shadow does not flag it, and the test reduces
+	 * to the downstream content_len = dir_len - 4 underflow path
+	 * which also returns NULL.
+	 */
+	u32 *block = kunit_kzalloc(test, 7 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+
+	KUNIT_ASSERT_NOT_NULL(test, block);
+
+	block[0] = 0x55584401;	/* "UXD" v1 magic */
+	block[1] = 0x00000004;	/* Root directory length: one entry */
+
+	/*
+	 * DIRECTORY entry with length = 3 pointing at dir_offset = 4.
+	 * tb_property_entry_valid() permits value(4) + length(3) <=
+	 * block_len(7).  Non-root parse begins with a kmemdup of 4
+	 * dwords from dir_offset for the UUID; that read runs past the
+	 * 28-byte allocation before the dir_len < 4 reject would fire.
+	 */
+	block[2] = 0x61616161;	/* key_hi */
+	block[3] = 0x61616161;	/* key_lo */
+	block[4] = 0x44000003;	/* type=DIRECTORY, reserved=0, length=3 */
+	block[5] = 0x00000004;	/* value = dir_offset */
+	/* block[6] is the start of the four UUID dwords; block[7..] is OOB. */
+
+	dir = tb_property_parse_dir(block, 7);
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
 static struct kunit_case tb_test_cases[] = {
+	KUNIT_CASE(tb_test_property_parse_u32_wrap),
+	KUNIT_CASE(tb_test_property_parse_recursion),
+	KUNIT_CASE(tb_test_property_parse_dir_len_underflow),
 	KUNIT_CASE(tb_test_path_basic),
 	KUNIT_CASE(tb_test_path_not_connected_walk),
 	KUNIT_CASE(tb_test_path_single_hop_walk),
-- 
2.53.0

Re: [PATCH v4 0/4] thunderbolt: harden XDomain property parser

[Mika Westerberg]

Hi Michael,

On Sun, May 10, 2026 at 07:16:55PM -0400, Michael Bommarito wrote:
> Style cleanups only on top of v3.  Andy's three nits on 1/4, 2/4,
> 3/4 are applied; Mika's request to drop the duplicated on-wire
> entry struct in 4/4 is applied.  No behavioural change to any
> patch; the bug analysis and the gating in patches 1-3 are
> unchanged.
> 
> 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.

Applied 1-3 to thunderbolt.git/fixes and the last one to
thunderbolt.git/next. Thanks a lot!