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

Michael Bommarito
2026-04-15T03:23:53Z

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