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

[Michael Bommarito <michael.bommarito@gmail.com>]

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