[PATCH 0/2] thunderbolt: harden XDomain property parser

[PATCH 0/2] thunderbolt: harden XDomain property parser

[Michael Bommarito]

Hi all,

Three independent memory-safety defects in drivers/thunderbolt/
property.c, each reproduced on v7.0-rc7 with CONFIG_KASAN=y
under qemu + KUnit.  Pre-fix, crafted XDomain property blocks
oops the kernel inside __tb_property_parse_dir with CR2 in the
KASAN shadow region.  Post-fix, the three KUnit cases in patch
2/2 pass cleanly.  

The defects are reachable when any untrusted Thunderbolt/USB4
XDomain peer responds to a PROPERTIES_REQUEST during host-to-host
discovery.  The peer delivers up to TB_XDP_PROPERTIES_MAX_LENGTH
(500) dwords of property block which the local host parses before
any PCIe tunnel is authorized.  The specific parser sites here
have been sitting here since the original 2017 XDomain support;
the file has had three prior NULL-check fixes in 2019
(106204b56f60, e4dfdd5804cc, 6183d5a51866), none of which touch
the bounds / arithmetic / recursion sites addressed here.

Worst case situation is either small OOB reads or DoS for
someone with physical access.

Defects
-------

A. u32 overflow in tb_property_entry_valid() (property.c:61).
   entry->value (u32) + entry->length (u16->u32) is performed in
   u32 and wraps.  With block_len = 500 an attacker picks
   value = 0xFFFFFF00, length = 0x100; the u32 sum 0x100000000
   wraps to 0, passing the > block_len check.  The subsequent
   parse_dwdata() at :132/:143 reads entry->length*4 bytes from
   block + entry->value (pointer arithmetic promoted to size_t,
   ~16 GiB past the allocation) into a freshly kcalloc'd
   destination.

   Exfil path: TB_PROPERTY_TYPE_TEXT on the "deviceid" or
   "vendorid" keys has its value.text copied into xd->device_name
   / xd->vendor_name via kstrdup() (xdomain.c:1157-1162), which
   are read back via the per-XDomain device_name and vendor_name
   sysfs attribute show() functions (xdomain.c:1730, 1763).
   kstrdup stops at the first NUL byte, so the usable leak is a
   NUL-bounded prefix of attacker-directed kernel memory.  This
   is not an RCE or arbitrary-write primitive; it is an
   OOB-read / info-leak class, untargeted (the attacker does not
   know block's KASLR/slab placement) and bounded by per-read NUL
   termination.  Fix: check_add_overflow() on value + length.

B. Unbounded recursion in __tb_property_parse_dir().
   DIRECTORY entries are parsed recursively with no depth counter;
   a peer that crafts a back-reference chain drives the parser
   until the 16 KiB kernel stack is exhausted and the guard page
   fires (pre-authentication remote DoS).  Fix: bound recursion
   to TB_PROPERTY_MAX_DEPTH = 8.

C. size_t underflow on dir_len - 4 (property.c:184).
   dir_len arrives as size_t sourced from entry->length (u16) on
   the non-root path; length < 4 underflows to ~SIZE_MAX,
   nentries = SIZE_MAX/4, loop walks entries past the block.
   OOB read + potential kernel oops.  Fix: reject dir_len < 4 on
   the non-root path.

Additional hardening: move INIT_LIST_HEAD(&dir->properties) to
immediately after dir allocation so every error-return path that
calls tb_property_free_dir() sees a walkable empty list rather
than the zero-initialized NULL next/prev that would oops
list_for_each_entry_safe().  This also closes a pre-existing
latent bug in the dir->uuid kmemdup-failure path at
property.c:180.

No controlled OOB-write is reachable through the parser;
parse_dwdata's destination is a freshly kcalloc'd buffer sized by
entry->length.

Attacker model
--------------

Malicious Thunderbolt/USB4 XDomain peer (cable, dock, in-line
inspector, adjacent host).  Discovery fires during link training;
PCIe tunnel authorization (the thunderbolt/.../authorized sysfs
gate) does not guard the control-plane PROPERTIES_REQUEST /
RESPONSE path.  Host IOMMU does not mitigate because the data
arrives as a control-plane payload the driver willingly copies
into its own buffer before parsing.  No user interaction beyond
the link-up event.

Reproduction
------------

Patch 2/2 adds three KUnit regression tests
(tb_test_property_parse_u32_wrap / _recursion /
_dir_len_underflow) to drivers/thunderbolt/test.c.  Tested
end-to-end on v7.0-rc7 + CONFIG_USB4_KUNIT_TEST=y + CONFIG_KASAN=y
under tools/testing/kunit/kunit.py run --arch=x86_64:

Pre-fix, each test oopses inside __tb_property_parse_dir:

  Oops: SMP KASAN PTI
  RIP: __tb_property_parse_dir+0x3ce
  CR2: 0xffffed108045eb80     # KASAN shadow region
  Code: ... 48 c1 ea 03 <0f b6 0c 2a>    # KASAN shadow byte load
  R8:  0x100                  # crafted entry->length
  [FAILED] tb_test_property_parse_u32_wrap
  [FAILED] tb_test_property_parse_recursion
  [FAILED] tb_test_property_parse_dir_len_underflow

Post-fix:

  [PASSED] tb_test_property_parse_u32_wrap
  [PASSED] tb_test_property_parse_recursion
  [PASSED] tb_test_property_parse_dir_len_underflow

Full run command:

  ./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_*'

Series
------

  [PATCH 1/2] thunderbolt: property: harden XDomain property
    parser against crafted peer
  [PATCH 2/2] thunderbolt: test: add KUnit regression tests for
    XDomain property parser

Patches apply to v7.0-rc7 (commit a8ee600e7aff).

Thanks,
Michael

Michael Bommarito (2):
  thunderbolt: property: harden XDomain property parser against crafted
    peer
  thunderbolt: test: add KUnit regression tests for XDomain property
    parser

 drivers/thunderbolt/property.c |  67 ++++++++++++++---
 drivers/thunderbolt/test.c     | 127 +++++++++++++++++++++++++++++++++
 2 files changed, 185 insertions(+), 9 deletions(-)

--
2.53.0

[PATCH 1/2] thunderbolt: property: harden XDomain property parser against crafted peer

[Michael Bommarito]

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()
before PCIe tunnel authorization.

Bug A - u32 overflow in tb_property_entry_valid() (property.c:61).

    if (entry->value + entry->length > block_len)
            return false;

entry->value is u32, entry->length is u16.  The sum is computed in
u32 and wraps.  With block_len = 500 an attacker picks
value = 0xFFFFFF00, length = 0x100; the u32 sum 0x100000000 wraps
to 0, passing the > block_len check.  tb_property_parse() then
runs

    parse_dwdata(property->value.data, block + entry->value,
                 entry->length);

where block is const u32 * and entry->value is promoted to size_t
for the pointer arithmetic.  The read source is block +
(u64)value * 4, tens of GiB past the property-block allocation.
Up to entry->length * 4 bytes are read from there into a freshly
kcalloc'd property->value.data or property->value.text buffer.

Exfiltration path for TB_PROPERTY_TYPE_TEXT on the "deviceid" or
"vendorid" keys: populate_properties() (xdomain.c:1157,1162) runs
kstrdup(p->value.text, ...) into xd->device_name / xd->vendor_name,
which are read back via the per-XDomain device_name and vendor_name
sysfs attributes (xdomain.c:1730, 1763).  kstrdup stops at the
first NUL byte in the OOB region, so the usable leak is the prefix
up to the first zero byte at an attacker-chosen offset past the
property block.  The attacker does not know block's KASLR/slab
placement, so the read is untargeted in absolute terms - they pick
a delta, not an address.  There is no generic "properties" sysfs
blob; DATA-typed properties are parsed into property->value.data
but never generically surfaced to userspace, so only the TEXT path
with the two named keys is exfil-reachable.  NUL-bounded,
untargeted, but still an attacker-directed OOB read.

Replace the u32 addition with check_add_overflow() so a wrapped
sum is rejected.

Bug B - unbounded recursion in __tb_property_parse_dir().

A DIRECTORY entry's value field is used as dir_offset for a
recursive call with no depth counter.  A peer that crafts a back-
reference chain drives the parser until the 16 KiB kernel stack
is exhausted and the guard page fires - pre-authentication remote
DoS.  Bound the recursion to TB_PROPERTY_MAX_DEPTH = 8,
comfortably larger than any legitimate XDomain property layout.

Bug C - size_t underflow on dir_len - 4 (property.c:184).

    content_offset = dir_offset + 4;
    content_len = dir_len - 4; /* Length includes UUID */

dir_len arrives as a size_t sourced from entry->length (u16) on
the non-root path.  If entry->length < 4, the subtraction
underflows size_t to ~SIZE_MAX, nentries becomes SIZE_MAX / 4,
and the loop walks entries past the property block on each
iteration, reading OOB until either an entry fails validation or
the kernel oopses on an unmapped page.  Reject dir_len < 4
explicitly on the non-root path.

Additional hardening: move INIT_LIST_HEAD(&dir->properties) to
immediately after dir allocation so every error-return path that
calls tb_property_free_dir() (including the new dir_len path and
the pre-existing dir->uuid alloc-failure path at property.c:180)
sees a walkable empty list rather than the zero-initialized NULL
next/prev that would oops list_for_each_entry_safe().

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

Attacker model: malicious Thunderbolt/USB4 XDomain peer (cable,
dock, in-line inspector, adjacent host).  Discovery runs as soon
as the link is trained; PCIe tunnel authorization does not gate
the control-plane PROPERTIES_REQUEST/RESPONSE path, and the host
IOMMU does not mitigate because the data arrives as a control-
plane payload the driver willingly copies into its own buffer
before parsing.

Reproduced on v7.0-rc7 + CONFIG_KASAN=y + CONFIG_USB4_KUNIT_TEST=y
via the companion KUnit suite in the sibling patch.  Pre-fix, each
of the three cases oopses inside __tb_property_parse_dir (Bug A
hits a KASAN shadow-memory fault, Bug B trips the stack guard,
Bug C OOB-reads past the property block).  Post-fix, all three
tests return NULL cleanly and pass.

The parser sites fixed here have not been touched since the
initial 2017 XDomain landing, per git log -p.  property.c has had
three prior fixes by Kangjie Lu in 2019 (106204b56f60,
e4dfdd5804cc, 6183d5a51866) for NULL-check omissions on
kzalloc/kmemdup returns, and a 2025 documentation cleanup by Alan
Borzeszkowski (d015642ad36d); none of those touch the bounds /
arithmetic / recursion sites this patch addresses.  Verified via
lei queries against lore.kernel.org/linux-usb/ for
dfn:drivers/thunderbolt/property.c,
dfhh:tb_property_parse_dir, dfhh:tb_property_entry_valid (0 hits
beyond the doc cleanup); Patchwork linux-usb "thunderbolt
property" query (0 in-flight patches); and Mika Westerberg's
westeri/thunderbolt.git next/fixes/master branches (no pending
bounds work on this file).

Fixes: e69b6c02b4c3 ("thunderbolt: Add functions for parsing and creating XDomain property blocks")
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 | 67 +++++++++++++++++++++++++++++-----
 1 file changed, 58 insertions(+), 9 deletions(-)

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index 50cbfc92fe65..57ec742ed210 100644
--- a/drivers/thunderbolt/property.c
+++ b/drivers/thunderbolt/property.c
@@ -8,11 +8,21 @@
  */
 
 #include <linux/err.h>
+#include <linux/overflow.h>
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/uuid.h>
 #include <linux/thunderbolt.h>
 
+/*
+ * Bounds recursion depth when parsing a malicious XDomain property
+ * block whose DIRECTORY entries are crafted to self-refer.  The
+ * XDomain spec gives no hard limit; 8 is comfortably larger than any
+ * legitimate property layout observed in practice and leaves the
+ * kernel stack headroom.
+ */
+#define TB_PROPERTY_MAX_DEPTH	8
+
 struct tb_property_entry {
 	u32 key_hi;
 	u32 key_lo;
@@ -37,7 +47,7 @@ struct tb_property_dir_entry {
 
 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)
 {
@@ -52,13 +62,23 @@ 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)
+		/*
+		 * entry->value is u32 and entry->length is u16; the sum is
+		 * performed in u32 and wraps for crafted inputs.  Use an
+		 * overflow-aware check so a wrapped sum is rejected instead
+		 * of appearing to satisfy the bound.
+		 */
+		if (check_add_overflow(entry->value, (u32)entry->length, &end))
+			return false;
+		if (end > block_len)
 			return false;
 		break;
 
@@ -93,7 +113,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;
@@ -114,7 +135,8 @@ 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;
@@ -159,16 +181,33 @@ 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;
 
+	/*
+	 * A malicious XDomain peer can craft DIRECTORY entries whose
+	 * offsets point back at their own container, making the recursion
+	 * unbounded without this gate.
+	 */
+	if (depth > TB_PROPERTY_MAX_DEPTH)
+		return NULL;
+
 	dir = kzalloc_obj(*dir);
 	if (!dir)
 		return NULL;
+	/*
+	 * Initialize the list head immediately so every error-return path
+	 * that calls tb_property_free_dir() (the new dir_len reject and
+	 * the existing uuid-alloc failure path) sees a walkable empty
+	 * list rather than the zero-initialized NULL next/prev that
+	 * would oops list_for_each_entry_safe().
+	 */
+	INIT_LIST_HEAD(&dir->properties);
 
 	if (is_root) {
 		content_offset = dir_offset + 2;
@@ -181,18 +220,28 @@ static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
 			return NULL;
 		}
 		content_offset = dir_offset + 4;
+		/*
+		 * dir_len arrives here as the u16 entry->length widened to
+		 * size_t; values below 4 underflow size_t on the subtraction
+		 * below and produce a gigantic content_len, driving the
+		 * nentries loop off the block with OOB reads on each
+		 * iteration.
+		 */
+		if (dir_len < 4) {
+			tb_property_free_dir(dir);
+			return NULL;
+		}
 		content_len = dir_len - 4; /* Length includes UUID */
 	}
 
 	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;
 
-		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;
@@ -231,7 +280,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 2/2] thunderbolt: test: add KUnit regression tests for XDomain property parser

[Michael Bommarito]

Add three KUnit cases that exercise the defects fixed by the parent
commit 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 so non-root content_len = dir_len - 4 wraps size_t;
    without the fix nentries is huge and the entry walk runs OOB.

Each test asserts tb_property_parse_dir() returns NULL on the
crafted input.  With CONFIG_KASAN=y, running these on the pre-fix
kernel reproduces an oops inside __tb_property_parse_dir (KASAN
shadow-memory fault for the u32_wrap case, stack-guard trip for
recursion, OOB read past block for dir_len underflow).  Post-fix
they pass cleanly.

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-6
Assisted-by: Codex:gpt-5-4
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/thunderbolt/test.c | 127 +++++++++++++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)

diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 1f4318249c22..22f4107fcb8d 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -2852,7 +2852,134 @@ 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.
+ */
+static void tb_test_property_parse_u32_wrap(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+	struct {
+		u32 key_hi, key_lo;
+		u16 length;
+		u8 reserved;
+		u8 type;
+		u32 value;
+	} *e;
+
+	/* Root header: magic + length=6 (single entry body of 4 dwords +
+	 * 2 slack, keeps walk within block[]). */
+	block[0] = 0x55584401;
+	block[1] = 6;
+
+	/* Crafted DATA entry at block[2..5]: value = 0xFFFFFF00 and
+	 * length = 0x100 are u32/u16 such that the u32 sum 0x100000000
+	 * wraps to 0, passing the sum <= block_len guard even though
+	 * the real offset is block + 0xFFFFFF00 * 4 (~16 GiB past the
+	 * block).  The subsequent parse_dwdata() at property.c:132
+	 * copies entry->length*4 = 1024 bytes from that wild address
+	 * into a fresh kcalloc buffer.
+	 */
+	e = (void *)&block[2];
+	e->key_hi = 0x61616161;
+	e->key_lo = 0x61616161;
+	e->length = 0x100;
+	e->type   = 0x64;		/* TB_PROPERTY_TYPE_DATA */
+	e->value  = 0xFFFFFF00;
+
+	dir = tb_property_parse_dir(block, 500);
+	/* With the fix this returns NULL; without it, KASAN splats in
+	 * be32_to_cpu_array() / memcpy reading block + value*4 out of
+	 * bounds.  Assert on the safe outcome: a NULL dir. */
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
+static void tb_test_property_parse_recursion(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+	struct entry {
+		u32 key_hi, key_lo;
+		u16 length;
+		u8 reserved;
+		u8 type;
+		u32 value;
+	} *e, *child_e;
+
+	block[0] = 0x55584401;
+	block[1] = 4;		/* rootdir length = one entry */
+
+	/* DIRECTORY entry pointing at dir_offset=2 with length=16.
+	 * When parsed as non-root: content_offset = 6, content_len = 12,
+	 * nentries = 3.  The child's first entry at block[6] is also
+	 * DIRECTORY pointing at 2, so the recursion oscillates between
+	 * two dir_offsets until the kernel stack is exhausted.
+	 */
+	e = (void *)&block[2];
+	e->key_hi = 0x61616161;
+	e->key_lo = 0x61616161;
+	e->length = 16;
+	e->type   = 0x44;		/* TB_PROPERTY_TYPE_DIRECTORY */
+	e->value  = 2;
+
+	child_e = (void *)&block[6];
+	child_e->key_hi = 0x62626262;
+	child_e->key_lo = 0x62626262;
+	child_e->length = 16;
+	child_e->type   = 0x44;
+	child_e->value  = 2;
+
+	dir = tb_property_parse_dir(block, 500);
+	/* With the fix this returns NULL at TB_PROPERTY_MAX_DEPTH (8).
+	 * Without it, the kernel stack-guard fires ~50-80 frames in
+	 * and the kunit thread oopses. */
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
+static void tb_test_property_parse_dir_len_underflow(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+	struct entry {
+		u32 key_hi, key_lo;
+		u16 length;
+		u8 reserved;
+		u8 type;
+		u32 value;
+	} *e;
+
+	block[0] = 0x55584401;
+	block[1] = 4;
+
+	/* DIRECTORY entry with length=3.  When parsed as non-root,
+	 * content_len = dir_len - 4 underflows size_t to ~SIZE_MAX,
+	 * nentries = SIZE_MAX/4.  The for-loop walks entries past the
+	 * block, reading OOB on each iteration.
+	 */
+	e = (void *)&block[2];
+	e->key_hi = 0x61616161;
+	e->key_lo = 0x61616161;
+	e->length = 3;
+	e->type   = 0x44;
+	e->value  = 6;
+
+	dir = tb_property_parse_dir(block, 500);
+	/* With the fix: NULL.  Without: KASAN splat on
+	 * block[content_offset + i*4] for i > 124 (past the 500-dword
+	 * block). */
+	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 1/2] thunderbolt: property: harden XDomain property parser against crafted peer

[Mika Westerberg]

Hi,

On Tue, Apr 14, 2026 at 11:23:34PM -0400, Michael Bommarito wrote:
> 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()
> before PCIe tunnel authorization.

There is no PCIe tunnel authorization happening here.

> Bug A - u32 overflow in tb_property_entry_valid() (property.c:61).
> 
>     if (entry->value + entry->length > block_len)
>             return false;

Please split this patch into 3 patches that all deal with one issue at the
time.

> 
> entry->value is u32, entry->length is u16.  The sum is computed in
> u32 and wraps.  With block_len = 500 an attacker picks
> value = 0xFFFFFF00, length = 0x100; the u32 sum 0x100000000 wraps
> to 0, passing the > block_len check.  tb_property_parse() then
> runs
> 
>     parse_dwdata(property->value.data, block + entry->value,
>                  entry->length);
> 
> where block is const u32 * and entry->value is promoted to size_t
> for the pointer arithmetic.  The read source is block +
> (u64)value * 4, tens of GiB past the property-block allocation.
> Up to entry->length * 4 bytes are read from there into a freshly
> kcalloc'd property->value.data or property->value.text buffer.
> 
> Exfiltration path for TB_PROPERTY_TYPE_TEXT on the "deviceid" or
> "vendorid" keys: populate_properties() (xdomain.c:1157,1162) runs
> kstrdup(p->value.text, ...) into xd->device_name / xd->vendor_name,
> which are read back via the per-XDomain device_name and vendor_name
> sysfs attributes (xdomain.c:1730, 1763).  kstrdup stops at the
> first NUL byte in the OOB region, so the usable leak is the prefix
> up to the first zero byte at an attacker-chosen offset past the
> property block.  The attacker does not know block's KASLR/slab
> placement, so the read is untargeted in absolute terms - they pick
> a delta, not an address.  There is no generic "properties" sysfs
> blob; DATA-typed properties are parsed into property->value.data
> but never generically surfaced to userspace, so only the TEXT path
> with the two named keys is exfil-reachable.  NUL-bounded,
> untargeted, but still an attacker-directed OOB read.
> 
> Replace the u32 addition with check_add_overflow() so a wrapped
> sum is rejected.
> 
> Bug B - unbounded recursion in __tb_property_parse_dir().
> 
> A DIRECTORY entry's value field is used as dir_offset for a
> recursive call with no depth counter.  A peer that crafts a back-
> reference chain drives the parser until the 16 KiB kernel stack
> is exhausted and the guard page fires - pre-authentication remote
> DoS.  Bound the recursion to TB_PROPERTY_MAX_DEPTH = 8,
> comfortably larger than any legitimate XDomain property layout.
> 
> Bug C - size_t underflow on dir_len - 4 (property.c:184).
> 
>     content_offset = dir_offset + 4;
>     content_len = dir_len - 4; /* Length includes UUID */
> 
> dir_len arrives as a size_t sourced from entry->length (u16) on
> the non-root path.  If entry->length < 4, the subtraction
> underflows size_t to ~SIZE_MAX, nentries becomes SIZE_MAX / 4,
> and the loop walks entries past the property block on each
> iteration, reading OOB until either an entry fails validation or
> the kernel oopses on an unmapped page.  Reject dir_len < 4
> explicitly on the non-root path.
> 
> Additional hardening: move INIT_LIST_HEAD(&dir->properties) to
> immediately after dir allocation so every error-return path that
> calls tb_property_free_dir() (including the new dir_len path and
> the pre-existing dir->uuid alloc-failure path at property.c:180)
> sees a walkable empty list rather than the zero-initialized NULL
> next/prev that would oops list_for_each_entry_safe().
> 
> All three defects are OOB-read plus DoS class.  No controlled OOB
> write is reachable through the parser; parse_dwdata's destination
> is a freshly kcalloc'd buffer sized by entry->length.
> 
> Attacker model: malicious Thunderbolt/USB4 XDomain peer (cable,
> dock, in-line inspector, adjacent host).  Discovery runs as soon
> as the link is trained; PCIe tunnel authorization does not gate
> the control-plane PROPERTIES_REQUEST/RESPONSE path, and the host
> IOMMU does not mitigate because the data arrives as a control-
> plane payload the driver willingly copies into its own buffer
> before parsing.

Here too there is not PCIe tunnel. It's a tunnel but not PCIe.

Also you can disable whole XDomain stuff with passing "xdomain=0" in the
kernel command line.

> Reproduced on v7.0-rc7 + CONFIG_KASAN=y + CONFIG_USB4_KUNIT_TEST=y
> via the companion KUnit suite in the sibling patch.  Pre-fix, each
> of the three cases oopses inside __tb_property_parse_dir (Bug A
> hits a KASAN shadow-memory fault, Bug B trips the stack guard,
> Bug C OOB-reads past the property block).  Post-fix, all three
> tests return NULL cleanly and pass.
> 
> The parser sites fixed here have not been touched since the
> initial 2017 XDomain landing, per git log -p.  property.c has had
> three prior fixes by Kangjie Lu in 2019 (106204b56f60,
> e4dfdd5804cc, 6183d5a51866) for NULL-check omissions on
> kzalloc/kmemdup returns, and a 2025 documentation cleanup by Alan
> Borzeszkowski (d015642ad36d); none of those touch the bounds /
> arithmetic / recursion sites this patch addresses.  Verified via
> lei queries against lore.kernel.org/linux-usb/ for
> dfn:drivers/thunderbolt/property.c,
> dfhh:tb_property_parse_dir, dfhh:tb_property_entry_valid (0 hits
> beyond the doc cleanup); Patchwork linux-usb "thunderbolt
> property" query (0 in-flight patches); and Mika Westerberg's
> westeri/thunderbolt.git next/fixes/master branches (no pending
> bounds work on this file).

Also please tune the commit messages you get from the AI. This one is easy
to read from running git log so no need to duplicate here.

> Fixes: e69b6c02b4c3 ("thunderbolt: Add functions for parsing and creating XDomain property blocks")
> 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 | 67 +++++++++++++++++++++++++++++-----
>  1 file changed, 58 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
> index 50cbfc92fe65..57ec742ed210 100644
> --- a/drivers/thunderbolt/property.c
> +++ b/drivers/thunderbolt/property.c
> @@ -8,11 +8,21 @@
>   */
>  
>  #include <linux/err.h>
> +#include <linux/overflow.h>
>  #include <linux/slab.h>
>  #include <linux/string.h>
>  #include <linux/uuid.h>
>  #include <linux/thunderbolt.h>
>  
> +/*
> + * Bounds recursion depth when parsing a malicious XDomain property
> + * block whose DIRECTORY entries are crafted to self-refer.  The
> + * XDomain spec gives no hard limit; 8 is comfortably larger than any
> + * legitimate property layout observed in practice and leaves the
> + * kernel stack headroom.
> + */
> +#define TB_PROPERTY_MAX_DEPTH	8
> +
>  struct tb_property_entry {
>  	u32 key_hi;
>  	u32 key_lo;
> @@ -37,7 +47,7 @@ struct tb_property_dir_entry {
>  
>  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)
>  {
> @@ -52,13 +62,23 @@ 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)
> +		/*
> +		 * entry->value is u32 and entry->length is u16; the sum is
> +		 * performed in u32 and wraps for crafted inputs.  Use an
> +		 * overflow-aware check so a wrapped sum is rejected instead
> +		 * of appearing to satisfy the bound.
> +		 */

Also please tidy up this comment.

> +		if (check_add_overflow(entry->value, (u32)entry->length, &end))
> +			return false;
> +		if (end > block_len)
>  			return false;
>  		break;
>  
> @@ -93,7 +113,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;
> @@ -114,7 +135,8 @@ 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;
> @@ -159,16 +181,33 @@ 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;
>  
> +	/*
> +	 * A malicious XDomain peer can craft DIRECTORY entries whose
> +	 * offsets point back at their own container, making the recursion
> +	 * unbounded without this gate.
> +	 */

This comment is useless.

> +	if (depth > TB_PROPERTY_MAX_DEPTH)
> +		return NULL;
> +
>  	dir = kzalloc_obj(*dir);
>  	if (!dir)
>  		return NULL;
> +	/*
> +	 * Initialize the list head immediately so every error-return path
> +	 * that calls tb_property_free_dir() (the new dir_len reject and
> +	 * the existing uuid-alloc failure path) sees a walkable empty
> +	 * list rather than the zero-initialized NULL next/prev that
> +	 * would oops list_for_each_entry_safe().
> +	 */

So is this.

> +	INIT_LIST_HEAD(&dir->properties);
>  
>  	if (is_root) {
>  		content_offset = dir_offset + 2;
> @@ -181,18 +220,28 @@ static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
>  			return NULL;
>  		}
>  		content_offset = dir_offset + 4;
> +		/*
> +		 * dir_len arrives here as the u16 entry->length widened to
> +		 * size_t; values below 4 underflow size_t on the subtraction
> +		 * below and produce a gigantic content_len, driving the
> +		 * nentries loop off the block with OOB reads on each
> +		 * iteration.
> +		 */

and this.

> +		if (dir_len < 4) {
> +			tb_property_free_dir(dir);
> +			return NULL;
> +		}
>  		content_len = dir_len - 4; /* Length includes UUID */
>  	}
>  
>  	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;
>  
> -		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;
> @@ -231,7 +280,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

Re: [PATCH 1/2] thunderbolt: property: harden XDomain property parser against crafted peer

[Michael Bommarito]

On Wed, Apr 15, 2026 at 12:52 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Please split this patch into 3 patches that all deal with one issue at the
> time.

Will do.  Sorry for the verbosity and bus confusion!  Look for another version
shortly.

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

[Michael Bommarito]

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(), size_t underflow on dir_len < 4 in
__tb_property_parse_dir(), and unbounded recursion in the same.
Patch 4 is three KUnit regression cases exercising all three.
Let me know if you want me to pair the KUnit cases with each
patch instead.

My assessment is that 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.

As Mika noted, 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, each case oopses inside
__tb_property_parse_dir (KASAN shadow-memory fault for u32-wrap,
stack-guard trip for recursion, OOB read past block for dir_len
underflow).  Post-fix, all three pass without issue.

Changes since v1
----------------

v1 -> v2, addressing Mika's review (msgid
20260415045246.GR3552@black.igk.intel.com):

  - Split the single property.c hardening patch into three, one per
    bug, ordered smallest-diff-first (u32 wrap, then dir_len
    underflow, then recursion cap).  [Mika]
  - Removed the incorrect "PCIe tunnel authorization" framing from
    the commit messages and cover letter.  XDomain discovery runs
    before any tunnel is set up; the path is not PCIe-specific.
    [Mika]
  - Added an explicit operator mitigation note
    (thunderbolt.xdomain=0).  [Mika]
  - Trimmed the commit messages: dropped the per-file prior-fix
    enumeration (Kangjie Lu 2019 series, Alan Borzeszkowski 2025
    cleanup) and the lei / Patchwork / westeri-tree scoop-check
    provenance notes; that content is available via git log -p and
    does not belong in the commit message.  [Mika]
  - Dropped the long inline block comments above check_add_overflow(),
    the TB_PROPERTY_MAX_DEPTH check, the INIT_LIST_HEAD reorder, and
    the dir_len < 4 reject; the code is self-explanatory given the
    commit message.  [Mika]
  - Reworded the recursion DoS description away from "remote" (this
    is a peer-triggered DoS reachable from any adjacent XDomain peer
    over the Thunderbolt/USB4 bus, not network-reachable).
  - KUnit patch unchanged in content; commit message adjusted to say
    "sibling commits" rather than "parent commit" now that the series
    has multiple parent fixes.

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     | 127 +++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+), 9 deletions(-)

--
2.53.0

[PATCH v2 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: e69b6c02b4c3 ("thunderbolt: Add functions for parsing and creating XDomain property blocks")
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..f5ee8f531300 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, (u32)entry->length, &end) ||
+		    end > block_len)
 			return false;
 		break;
 
-- 
2.53.0

[PATCH v2 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) and computes
content_len = dir_len - 4.  If a crafted peer supplies
entry->length < 4, the subtraction underflows size_t to ~SIZE_MAX,
nentries becomes SIZE_MAX / 4, and the entry walk runs off the
property block, reading OOB on each iteration until either an entry
fails validation or the kernel oopses on an unmapped page.

Reject dir_len < 4 explicitly before the subtraction.

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

Fixes: e69b6c02b4c3 ("thunderbolt: Add functions for parsing and creating XDomain property blocks")
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 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index f5ee8f531300..274b555d27c8 100644
--- a/drivers/thunderbolt/property.c
+++ b/drivers/thunderbolt/property.c
@@ -173,6 +173,7 @@ static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
 	dir = kzalloc_obj(*dir);
 	if (!dir)
 		return NULL;
+	INIT_LIST_HEAD(&dir->properties);
 
 	if (is_root) {
 		content_offset = dir_offset + 2;
@@ -184,6 +185,10 @@ static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
 			tb_property_free_dir(dir);
 			return NULL;
 		}
+		if (dir_len < 4) {
+			tb_property_free_dir(dir);
+			return NULL;
+		}
 		content_offset = dir_offset + 4;
 		content_len = dir_len - 4; /* Length includes UUID */
 	}
@@ -191,8 +196,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 v2 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: e69b6c02b4c3 ("thunderbolt: Add functions for parsing and creating XDomain property blocks")
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 | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index 274b555d27c8..99ee7089456c 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;
@@ -199,7 +205,8 @@ 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;
@@ -238,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 v2 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 so non-root content_len = dir_len - 4 wraps size_t;
    without the fix nentries is huge and the entry walk runs OOB.

Each test asserts tb_property_parse_dir() returns NULL on the
crafted input.  With CONFIG_KASAN=y, running these on the pre-fix
kernel reproduces an oops inside __tb_property_parse_dir (KASAN
shadow-memory fault for the u32_wrap case, stack-guard trip for
recursion, OOB read past block for dir_len underflow).  Post-fix
they pass cleanly.

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-6
Assisted-by: Codex:gpt-5-4
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/thunderbolt/test.c | 127 +++++++++++++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)

diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 1f4318249c22..22f4107fcb8d 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -2852,7 +2852,134 @@ 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.
+ */
+static void tb_test_property_parse_u32_wrap(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+	struct {
+		u32 key_hi, key_lo;
+		u16 length;
+		u8 reserved;
+		u8 type;
+		u32 value;
+	} *e;
+
+	/* Root header: magic + length=6 (single entry body of 4 dwords +
+	 * 2 slack, keeps walk within block[]). */
+	block[0] = 0x55584401;
+	block[1] = 6;
+
+	/* Crafted DATA entry at block[2..5]: value = 0xFFFFFF00 and
+	 * length = 0x100 are u32/u16 such that the u32 sum 0x100000000
+	 * wraps to 0, passing the sum <= block_len guard even though
+	 * the real offset is block + 0xFFFFFF00 * 4 (~16 GiB past the
+	 * block).  The subsequent parse_dwdata() at property.c:132
+	 * copies entry->length*4 = 1024 bytes from that wild address
+	 * into a fresh kcalloc buffer.
+	 */
+	e = (void *)&block[2];
+	e->key_hi = 0x61616161;
+	e->key_lo = 0x61616161;
+	e->length = 0x100;
+	e->type   = 0x64;		/* TB_PROPERTY_TYPE_DATA */
+	e->value  = 0xFFFFFF00;
+
+	dir = tb_property_parse_dir(block, 500);
+	/* With the fix this returns NULL; without it, KASAN splats in
+	 * be32_to_cpu_array() / memcpy reading block + value*4 out of
+	 * bounds.  Assert on the safe outcome: a NULL dir. */
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
+static void tb_test_property_parse_recursion(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+	struct entry {
+		u32 key_hi, key_lo;
+		u16 length;
+		u8 reserved;
+		u8 type;
+		u32 value;
+	} *e, *child_e;
+
+	block[0] = 0x55584401;
+	block[1] = 4;		/* rootdir length = one entry */
+
+	/* DIRECTORY entry pointing at dir_offset=2 with length=16.
+	 * When parsed as non-root: content_offset = 6, content_len = 12,
+	 * nentries = 3.  The child's first entry at block[6] is also
+	 * DIRECTORY pointing at 2, so the recursion oscillates between
+	 * two dir_offsets until the kernel stack is exhausted.
+	 */
+	e = (void *)&block[2];
+	e->key_hi = 0x61616161;
+	e->key_lo = 0x61616161;
+	e->length = 16;
+	e->type   = 0x44;		/* TB_PROPERTY_TYPE_DIRECTORY */
+	e->value  = 2;
+
+	child_e = (void *)&block[6];
+	child_e->key_hi = 0x62626262;
+	child_e->key_lo = 0x62626262;
+	child_e->length = 16;
+	child_e->type   = 0x44;
+	child_e->value  = 2;
+
+	dir = tb_property_parse_dir(block, 500);
+	/* With the fix this returns NULL at TB_PROPERTY_MAX_DEPTH (8).
+	 * Without it, the kernel stack-guard fires ~50-80 frames in
+	 * and the kunit thread oopses. */
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
+static void tb_test_property_parse_dir_len_underflow(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+	struct entry {
+		u32 key_hi, key_lo;
+		u16 length;
+		u8 reserved;
+		u8 type;
+		u32 value;
+	} *e;
+
+	block[0] = 0x55584401;
+	block[1] = 4;
+
+	/* DIRECTORY entry with length=3.  When parsed as non-root,
+	 * content_len = dir_len - 4 underflows size_t to ~SIZE_MAX,
+	 * nentries = SIZE_MAX/4.  The for-loop walks entries past the
+	 * block, reading OOB on each iteration.
+	 */
+	e = (void *)&block[2];
+	e->key_hi = 0x61616161;
+	e->key_lo = 0x61616161;
+	e->length = 3;
+	e->type   = 0x44;
+	e->value  = 6;
+
+	dir = tb_property_parse_dir(block, 500);
+	/* With the fix: NULL.  Without: KASAN splat on
+	 * block[content_offset + i*4] for i > 124 (past the 500-dword
+	 * block). */
+	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 v2 1/4] thunderbolt: property: reject u32 wrap in tb_property_entry_valid()

[Mika Westerberg]

Hi Michael,

I was about to apply these but noticed few stylistic issues so can you fix
those and send v3?

On Wed, Apr 15, 2026 at 08:32:17AM -0400, Michael Bommarito wrote:
> 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

It's 0xffffff00 (e.g lower case).

Ditto everywhere.

> 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: e69b6c02b4c3 ("thunderbolt: Add functions for parsing and creating XDomain property blocks")
> 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..f5ee8f531300 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, (u32)entry->length, &end) ||
> +		    end > block_len)
>  			return false;
>  		break;
>  
> -- 
> 2.53.0

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

[Mika Westerberg]

On Wed, Apr 15, 2026 at 08:32:20AM -0400, Michael Bommarito wrote:
> 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 so non-root content_len = dir_len - 4 wraps size_t;
>     without the fix nentries is huge and the entry walk runs OOB.
> 
> Each test asserts tb_property_parse_dir() returns NULL on the
> crafted input.  With CONFIG_KASAN=y, running these on the pre-fix
> kernel reproduces an oops inside __tb_property_parse_dir (KASAN
> shadow-memory fault for the u32_wrap case, stack-guard trip for
> recursion, OOB read past block for dir_len underflow).  Post-fix
> they pass cleanly.
> 
> 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-6
> Assisted-by: Codex:gpt-5-4
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  drivers/thunderbolt/test.c | 127 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 127 insertions(+)
> 
> diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
> index 1f4318249c22..22f4107fcb8d 100644
> --- a/drivers/thunderbolt/test.c
> +++ b/drivers/thunderbolt/test.c
> @@ -2852,7 +2852,134 @@ 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.
> + */
> +static void tb_test_property_parse_u32_wrap(struct kunit *test)
> +{
> +	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
> +	struct tb_property_dir *dir;
> +	struct {
> +		u32 key_hi, key_lo;
> +		u16 length;
> +		u8 reserved;
> +		u8 type;
> +		u32 value;
> +	} *e;

This is same as tb_property_entry so probably should use that or better do
it like we have in that root_directory as array of u32.

At least do not duplicate it all over.

> +
> +	/* Root header: magic + length=6 (single entry body of 4 dwords +
> +	 * 2 slack, keeps walk within block[]). */

The block comment format is

	/*
	 * ..
	 * ..
	 */

Ditto everywhre.


> +	block[0] = 0x55584401;
> +	block[1] = 6;
> +
> +	/* Crafted DATA entry at block[2..5]: value = 0xFFFFFF00 and
> +	 * length = 0x100 are u32/u16 such that the u32 sum 0x100000000
> +	 * wraps to 0, passing the sum <= block_len guard even though
> +	 * the real offset is block + 0xFFFFFF00 * 4 (~16 GiB past the
> +	 * block).  The subsequent parse_dwdata() at property.c:132
> +	 * copies entry->length*4 = 1024 bytes from that wild address
> +	 * into a fresh kcalloc buffer.
> +	 */
> +	e = (void *)&block[2];
> +	e->key_hi = 0x61616161;
> +	e->key_lo = 0x61616161;
> +	e->length = 0x100;
> +	e->type   = 0x64;		/* TB_PROPERTY_TYPE_DATA */

This can use TB_PROPERtY_TYPE_DATA.

> +	e->value  = 0xFFFFFF00;
> +
> +	dir = tb_property_parse_dir(block, 500);
> +	/* With the fix this returns NULL; without it, KASAN splats in
> +	 * be32_to_cpu_array() / memcpy reading block + value*4 out of
> +	 * bounds.  Assert on the safe outcome: a NULL dir. */
> +	KUNIT_EXPECT_NULL(test, dir);
> +	tb_property_free_dir(dir);
> +}
> +
> +static void tb_test_property_parse_recursion(struct kunit *test)
> +{
> +	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
> +	struct tb_property_dir *dir;
> +	struct entry {
> +		u32 key_hi, key_lo;
> +		u16 length;
> +		u8 reserved;
> +		u8 type;
> +		u32 value;
> +	} *e, *child_e;
> +
> +	block[0] = 0x55584401;
> +	block[1] = 4;		/* rootdir length = one entry */
> +
> +	/* DIRECTORY entry pointing at dir_offset=2 with length=16.
> +	 * When parsed as non-root: content_offset = 6, content_len = 12,
> +	 * nentries = 3.  The child's first entry at block[6] is also
> +	 * DIRECTORY pointing at 2, so the recursion oscillates between
> +	 * two dir_offsets until the kernel stack is exhausted.
> +	 */
> +	e = (void *)&block[2];
> +	e->key_hi = 0x61616161;
> +	e->key_lo = 0x61616161;
> +	e->length = 16;
> +	e->type   = 0x44;		/* TB_PROPERTY_TYPE_DIRECTORY */

This can use TB_PROPERTY_TYPE_DIRECTORY.

> +	e->value  = 2;
> +
> +	child_e = (void *)&block[6];
> +	child_e->key_hi = 0x62626262;
> +	child_e->key_lo = 0x62626262;
> +	child_e->length = 16;
> +	child_e->type   = 0x44;
> +	child_e->value  = 2;
> +
> +	dir = tb_property_parse_dir(block, 500);
> +	/* With the fix this returns NULL at TB_PROPERTY_MAX_DEPTH (8).
> +	 * Without it, the kernel stack-guard fires ~50-80 frames in
> +	 * and the kunit thread oopses. */
> +	KUNIT_EXPECT_NULL(test, dir);
> +	tb_property_free_dir(dir);
> +}
> +
> +static void tb_test_property_parse_dir_len_underflow(struct kunit *test)
> +{
> +	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
> +	struct tb_property_dir *dir;
> +	struct entry {
> +		u32 key_hi, key_lo;
> +		u16 length;
> +		u8 reserved;
> +		u8 type;
> +		u32 value;
> +	} *e;
> +
> +	block[0] = 0x55584401;
> +	block[1] = 4;
> +
> +	/* DIRECTORY entry with length=3.  When parsed as non-root,
> +	 * content_len = dir_len - 4 underflows size_t to ~SIZE_MAX,
> +	 * nentries = SIZE_MAX/4.  The for-loop walks entries past the
> +	 * block, reading OOB on each iteration.
> +	 */
> +	e = (void *)&block[2];
> +	e->key_hi = 0x61616161;
> +	e->key_lo = 0x61616161;
> +	e->length = 3;
> +	e->type   = 0x44;

Thi can use TB_PROPERTY_TYPE_DIRECTORY.

> +	e->value  = 6;
> +
> +	dir = tb_property_parse_dir(block, 500);
> +	/* With the fix: NULL.  Without: KASAN splat on
> +	 * block[content_offset + i*4] for i > 124 (past the 500-dword
> +	 * block). */
> +	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 v2 1/4] thunderbolt: property: reject u32 wrap in tb_property_entry_valid()

[Michael Bommarito]

On Mon, Apr 27, 2026 at 1:35 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> I was about to apply these but noticed few stylistic issues so can you fix
> those and send v3?
>
> On Wed, Apr 15, 2026 at 08:32:17AM -0400, Michael Bommarito wrote:
> > 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
>
> It's 0xffffff00 (e.g lower case).
>
> Ditto everywhere.

Sure, sorry for the slow turnaround.  Coming shortly.

Thanks,
Mike Bommarito

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

[Michael Bommarito]

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

[PATCH v3 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>
---
v2 -> v3 (no code changes):
- Lowercase 0xffffff00 in commit message.
- Fix Fixes: SHA (was e69b6c02b4c3, "net: Add support for
  networking over Thunderbolt cable"; correct is cdae7c07e3e3).

 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..f5ee8f531300 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, (u32)entry->length, &end) ||
+		    end > block_len)
 			return false;
 		break;
 
-- 
2.53.0

[PATCH v3 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>
---
v2 -> v3 (material):
- Move dir_len < 4 reject before the UUID kmemdup in the non-root
  branch.  v2 ordering let dir_offset near end of block + dir_len
  in 0..3 OOB-read the kmemdup before the reject ran
  (dir_offset=497, dir_len=3, block_len=500 passes
  value+length<=block_len but kmemdup reads block[497..501]).
- Fix Fixes: SHA (was e69b6c02b4c3; correct is cdae7c07e3e3).

 drivers/thunderbolt/property.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index f5ee8f531300..90fa6f760963 100644
--- a/drivers/thunderbolt/property.c
+++ b/drivers/thunderbolt/property.c
@@ -173,11 +173,16 @@ static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
 	dir = kzalloc_obj(*dir);
 	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 +196,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 v3 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>
---
v2 -> v3:
- Fix Fixes: SHA (was e69b6c02b4c3; correct is cdae7c07e3e3).

 drivers/thunderbolt/property.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index 90fa6f760963..8db56922011b 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;
@@ -199,7 +205,8 @@ 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;
@@ -238,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 v3 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 so the non-root UUID kmemdup of 4 dwords from
    dir_offset reads past the block, and the downstream content_len
    = dir_len - 4 size_t underflow drives the entry walk OOB.

Each test asserts tb_property_parse_dir() returns NULL on the
crafted input.  On a pre-fix kernel with CONFIG_KASAN=y, u32_wrap
trips a KASAN report inside __tb_property_parse_dir() (the parser
reads ~16 GiB past the block) and recursion trips an Oops on
RIP=0 via the stack-guard.  dir_len_underflow returns NULL on
pre-fix via the downstream content_len underflow path; the UUID
kmemdup over-read happens silently because KASAN-Generic's slab
redzones do not flag a 4-byte over-read into the kmalloc-chunk
tail, so this case is the post-fix invariant pin rather than an
active pre-fix detector.  Post-fix all three pass cleanly.

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-6
Assisted-by: Codex:gpt-5-4
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2 -> v3:
- De-duplicate the on-wire entry layout: define a single
  struct tb_test_property_entry shared across all three tests
  instead of re-declaring an anonymous struct in each.
- Use TB_PROPERTY_TYPE_DATA / TB_PROPERTY_TYPE_DIRECTORY
  constants from <linux/thunderbolt.h> instead of bare 0x64 /
  0x44.
- Convert all multi-line block comments to put the opening "/*"
  on its own line per the thunderbolt subsystem's coding style.
- Lowercase 0xffffff00 in commit message + code + comments.
- Tighten dir_len_underflow: use a 7-dword (28-byte) buffer so
  the non-root kmemdup over-read targets the kmalloc-32 tail
  rather than slab slop within a kmalloc-2048 chunk.  KASAN-
  Generic still does not flag the 4-byte over-read here (slab
  redzones cover next-chunk metadata, not in-chunk tail), so
  the test remains a post-fix invariant pin; documented
  explicitly above.

 drivers/thunderbolt/test.c | 132 +++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 1f4318249c22..73de7292ee21 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -2852,7 +2852,139 @@ 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 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;
+};
+
+static void tb_test_property_parse_u32_wrap(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+	struct tb_test_property_entry *e;
+
+	/*
+	 * Root header: magic + length=6 (single entry body of 4 dwords +
+	 * 2 slack, keeps walk within block[]).
+	 */
+	block[0] = 0x55584401;
+	block[1] = 6;
+
+	/*
+	 * Crafted DATA entry at block[2..5]: value = 0xffffff00 and
+	 * length = 0x100 are u32/u16 such that the u32 sum 0x100000000
+	 * wraps to 0, passing the sum <= block_len guard even though
+	 * the real offset is block + 0xffffff00 * 4 (~16 GiB past the
+	 * block).  The subsequent parse_dwdata() copies entry->length*4
+	 * = 1024 bytes from that wild address into a fresh kcalloc
+	 * buffer.
+	 */
+	e = (void *)&block[2];
+	e->key_hi = 0x61616161;
+	e->key_lo = 0x61616161;
+	e->length = 0x100;
+	e->type   = TB_PROPERTY_TYPE_DATA;
+	e->value  = 0xffffff00;
+
+	dir = tb_property_parse_dir(block, 500);
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
+static void tb_test_property_parse_recursion(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 500 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+	struct tb_test_property_entry *e, *child_e;
+
+	block[0] = 0x55584401;
+	block[1] = 4;		/* rootdir length = one entry */
+
+	/*
+	 * DIRECTORY entry pointing at dir_offset=2 with length=16.
+	 * When parsed as non-root: content_offset = 6, content_len = 12,
+	 * nentries = 3.  The child's first entry at block[6] is also
+	 * DIRECTORY pointing at 2, so the recursion oscillates between
+	 * two dir_offsets until the kernel stack is exhausted.
+	 */
+	e = (void *)&block[2];
+	e->key_hi = 0x61616161;
+	e->key_lo = 0x61616161;
+	e->length = 16;
+	e->type   = TB_PROPERTY_TYPE_DIRECTORY;
+	e->value  = 2;
+
+	child_e = (void *)&block[6];
+	child_e->key_hi = 0x62626262;
+	child_e->key_lo = 0x62626262;
+	child_e->length = 16;
+	child_e->type   = TB_PROPERTY_TYPE_DIRECTORY;
+	child_e->value  = 2;
+
+	dir = tb_property_parse_dir(block, 500);
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
+static void tb_test_property_parse_dir_len_underflow(struct kunit *test)
+{
+	/*
+	 * Request 28 bytes (7 dwords) so KASAN-Generic tags the
+	 * 4 trailing bytes of the underlying kmalloc-32 chunk as a
+	 * slab redzone.  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, ... bytes) puts the
+	 * over-read into the slab cache tail where KASAN's generic
+	 * shadow does not flag it, and the test reduces to a
+	 * tautology because the downstream content_len = dir_len - 4
+	 * underflow also returns NULL.
+	 */
+	u32 *block = kunit_kzalloc(test, 7 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+	struct tb_test_property_entry *e;
+
+	block[0] = 0x55584401;
+	block[1] = 4;		/* rootdir length = one entry */
+
+	/*
+	 * DIRECTORY entry with length=3 pointing at dir_offset=4.
+	 * tb_property_entry_valid() permits value+length=7 <=
+	 * block_len=7.  Non-root parse begins with a kmemdup of 4
+	 * dwords from dir_offset for the UUID; with the v2 ordering
+	 * that kmemdup runs before the dir_len < 4 reject and reads
+	 * past the buffer.  With the v3 ordering the reject sits
+	 * before the kmemdup and the read never happens.
+	 */
+	e = (void *)&block[2];
+	e->key_hi = 0x61616161;
+	e->key_lo = 0x61616161;
+	e->length = 3;
+	e->type   = TB_PROPERTY_TYPE_DIRECTORY;
+	e->value  = 4;
+
+	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