[PATCH 0/6] thunderbolt: harden XDomain property exchange

[PATCH 0/6] thunderbolt: harden XDomain property exchange

[Michael Bommarito]

This series fixes 4 memory-safety defects and 1 data-handling
hardening issue in the Thunderbolt XDomain property exchange path
(property.c and xdomain.c) and adds KUnit regression tests.
All are reachable from an adjacent Thunderbolt peer without
authentication.  The XDomain protocol runs automatically on cable
insertion regardless of the configured security level, unless
disabled with thunderbolt.xdomain=0.

Patches:

  1/6 - reject zero-length property entries in validator
  2/6 - bound root directory content to block size
  3/6 - clamp XDomain response data copy to allocation size
  4/6 - validate XDomain request packet size before type cast
  5/6 - limit XDomain response copy to actual frame size
  6/6 - add KUnit tests for property parser bounds checks

Tested with KASAN on v7.1-rc3 and over Thunderbolt 4 hardware.
KUnit regression tests (patch 6) confirm the fixes and existing
tb_test_property_* tests pass on the patched tree.

Based-on: thunderbolt/fixes (928abe19fbf01)

Michael Bommarito (6):
  thunderbolt: reject zero-length property entries in validator
  thunderbolt: bound root directory content to block size
  thunderbolt: clamp XDomain response data copy to allocation size
  thunderbolt: validate XDomain request packet size before type cast
  thunderbolt: limit XDomain response copy to actual frame size
  thunderbolt: test: add KUnit tests for property parser bounds checks

 drivers/thunderbolt/property.c |  6 ++++++
 drivers/thunderbolt/test.c     | 40 ++++++++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/xdomain.c  | 14 +++++++++++---
 3 files changed, 57 insertions(+), 3 deletions(-)

[PATCH 1/6] thunderbolt: reject zero-length property entries in validator

[Michael Bommarito]

tb_property_entry_valid() accepts entries with length == 0 for
DIRECTORY, DATA, and TEXT types.  A zero-length TEXT entry passes
validation but causes an underflow in the null-termination logic:

  property->value.text[property->length * 4 - 1] = '\0';

When property->length is 0 this writes to offset -1 relative to
the allocation.

Reject zero-length entries early in the validator since they have no
valid representation in the XDomain property protocol.

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

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index da2c59a17db5c..5cbc1c4f159c2 100644
--- a/drivers/thunderbolt/property.c
+++ b/drivers/thunderbolt/property.c
@@ -60,6 +60,8 @@ static bool tb_property_entry_valid(const struct tb_property_entry *entry,
 	case TB_PROPERTY_TYPE_DIRECTORY:
 	case TB_PROPERTY_TYPE_DATA:
 	case TB_PROPERTY_TYPE_TEXT:
+		if (!entry->length)
+			return false;
 		if (entry->length > block_len)
 			return false;
 		if (check_add_overflow(entry->value, entry->length, &end) ||

base-commit: 928abe19fbf0127003abcb1ea69cabc1c897d0ab
-- 
2.53.0

[PATCH 2/6] thunderbolt: bound root directory content to block size

[Michael Bommarito]

__tb_property_parse_dir() does not check that content_offset +
content_len fits within block_len for the root directory case.
When rootdir->length equals or exceeds block_len - 2, the entry
loop reads past the allocated property block.

Add a bounds check after computing content_offset and content_len
to reject directories whose content extends past the block.

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

diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
index 5cbc1c4f159c2..59beab43f90a6 100644
--- a/drivers/thunderbolt/property.c
+++ b/drivers/thunderbolt/property.c
@@ -187,6 +187,10 @@ static struct tb_property_dir *__tb_property_parse_dir(const u32 *block,
 	if (is_root) {
 		content_offset = dir_offset + 2;
 		content_len = dir_len;
+		if (content_offset + content_len > block_len) {
+			tb_property_free_dir(dir);
+			return NULL;
+		}
 	} else {
 		if (dir_len < 4) {
 			tb_property_free_dir(dir);
-- 
2.53.0

[PATCH 3/6] thunderbolt: clamp XDomain response data copy to allocation size

[Michael Bommarito]

tb_xdp_properties_request() derives the per-packet copy length from
the response header without checking that it fits in the previously
allocated data buffer.  A malicious peer can set its length field
larger than the declared data_length, causing memcpy to write past
the kcalloc allocation.

Clamp the per-packet copy length so that the cumulative offset
never exceeds data_len.

Fixes: cdae7c07e3e3 ("thunderbolt: Add support for XDomain properties")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Confirmed over Thunderbolt 4 cable (Framework -> Dell, stock Ubuntu
26.04 7.0.0-15-generic).

Also reproduced with KASAN on QEMU (7.1.0-rc3):

  BUG: KASAN: slab-out-of-bounds in
       tb_test_synthetic_overflow.cold+0x131/0x29a
  Write of size 192 at addr ffff888002110200

 drivers/thunderbolt/xdomain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 754808c43f006..4099419c74795 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -393,6 +393,8 @@ static int tb_xdp_properties_request(struct tb_ctl *ctl, u64 route,
 			}
 		}
 
+		if (req.offset + len > data_len)
+			len = data_len - req.offset;
 		memcpy(data + req.offset, res->data, len * 4);
 		req.offset += len;
 	} while (!data_len || req.offset < data_len);
-- 
2.53.0

[PATCH 4/6] thunderbolt: validate XDomain request packet size before type cast

[Michael Bommarito]

tb_xdp_handle_request() casts the received packet buffer to
protocol-specific structs without verifying that the allocation
is large enough for the target type.  A peer can send a minimal
XDomain packet that passes the generic header length check but is
shorter than the struct accessed after the cast, causing out-of-
bounds reads from the kmemdup allocation.

Plumb the packet length through xdomain_request_work and validate
it against the expected struct size before each cast.

Fixes: 8e1de7042596 ("thunderbolt: Add support for XDomain lane bonding")
Fixes: cdae7c07e3e3 ("thunderbolt: Add support for XDomain properties")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Stock KASAN splat reproduced on QEMU (7.1.0-rc3).  A test module
allocates a 32-byte packet (tb_xdp_header only) and casts to
tb_xdp_link_state_change (36 bytes).  The read past the allocation
fires immediately:

  BUG: KASAN: slab-out-of-bounds in
       tb_test_xdp_short_packet_cast_trigger.cold+0x118/0x12d
  Read of size 1 at addr ffff888002110260
  located 0 bytes to the right of allocated 32-byte region

Also exercised over Thunderbolt 4 cable with 258 truncated-packet
injections (PROPERTIES_REQUEST 68 -> 32 bytes).

 drivers/thunderbolt/xdomain.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 4099419c74795..9d54e3ccc8278 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -55,6 +55,7 @@ static const char * const state_names[] = {
 struct xdomain_request_work {
 	struct work_struct work;
 	struct tb_xdp_header *pkg;
+	size_t pkg_len;
 	struct tb *tb;
 };
 
@@ -733,6 +734,7 @@ static void tb_xdp_handle_request(struct work_struct *work)
 	struct xdomain_request_work *xw = container_of(work, typeof(*xw), work);
 	const struct tb_xdp_header *pkg = xw->pkg;
 	const struct tb_xdomain_header *xhdr = &pkg->xd_hdr;
+	size_t pkg_len = xw->pkg_len;
 	struct tb *tb = xw->tb;
 	struct tb_ctl *ctl = tb->ctl;
 	struct tb_xdomain *xd;
@@ -764,7 +766,7 @@ static void tb_xdp_handle_request(struct work_struct *work)
 	switch (pkg->type) {
 	case PROPERTIES_REQUEST:
 		tb_dbg(tb, "%llx: received XDomain properties request\n", route);
-		if (xd) {
+		if (xd && pkg_len >= sizeof(struct tb_xdp_properties)) {
 			ret = tb_xdp_properties_response(tb, ctl, xd, sequence,
 				(const struct tb_xdp_properties *)pkg);
 		}
@@ -818,7 +820,8 @@ static void tb_xdp_handle_request(struct work_struct *work)
 		tb_dbg(tb, "%llx: received XDomain link state change request\n",
 		       route);
 
-		if (xd && xd->state == XDOMAIN_STATE_BONDING_UUID_HIGH) {
+		if (xd && xd->state == XDOMAIN_STATE_BONDING_UUID_HIGH &&
+		    pkg_len >= sizeof(struct tb_xdp_link_state_change)) {
 			const struct tb_xdp_link_state_change *lsc =
 				(const struct tb_xdp_link_state_change *)pkg;
 
@@ -870,6 +873,7 @@ tb_xdp_schedule_request(struct tb *tb, const struct tb_xdp_header *hdr,
 		kfree(xw);
 		return false;
 	}
+	xw->pkg_len = size;
 	xw->tb = tb_domain_get(tb);
 
 	schedule_work(&xw->work);
-- 
2.53.0

[PATCH 5/6] thunderbolt: limit XDomain response copy to actual frame size

[Michael Bommarito]

tb_xdomain_copy() copies req->response_size bytes from the received
packet buffer regardless of the actual frame size.  When a short
response arrives, this reads past the valid frame data in the DMA
pool buffer into stale contents from previous transactions.

Use the minimum of frame size and expected response size for the
copy length.

Fixes: cdae7c07e3e3 ("thunderbolt: Add support for XDomain properties")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
The DMA pool buffer (ctl.c:340) is always 256 bytes, so a short
frame does not cause an out-of-bounds read from the buffer itself.
The real impact is that bytes past the valid frame contain stale
data from previous DMA transactions, which are copied into the
response struct and interpreted as protocol fields.

Confirmed on QEMU (7.1.0-rc3).

 drivers/thunderbolt/xdomain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 9d54e3ccc8278..1fd1cf4295a2a 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -123,7 +123,9 @@ static bool tb_xdomain_match(const struct tb_cfg_request *req,
 static bool tb_xdomain_copy(struct tb_cfg_request *req,
 			    const struct ctl_pkg *pkg)
 {
-	memcpy(req->response, pkg->buffer, req->response_size);
+	size_t len = min_t(size_t, pkg->frame.size, req->response_size);
+
+	memcpy(req->response, pkg->buffer, len);
 	req->result.err = 0;
 	return true;
 }
-- 
2.53.0

[PATCH 6/6] thunderbolt: test: add KUnit tests for property parser bounds checks

[Michael Bommarito]

Add regression tests for the zero-length entry and root directory
bounds fixes:

- tb_test_property_parse_zero_length: TEXT entry with length 0
  must be rejected by the validator.

- tb_test_property_parse_rootdir_overflow: root directory whose
  content_offset + content_len exceeds block_len must be rejected.

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

diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 1f4318249c226..345f39ecd233f 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -2852,6 +2852,44 @@ static void tb_test_property_copy(struct kunit *test)
 	tb_property_free_dir(src);
 }
 
+static void tb_test_property_parse_zero_length(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 6 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+
+	KUNIT_ASSERT_NOT_NULL(test, block);
+
+	block[0] = 0x55584401;	/* rootdir magic */
+	block[1] = 0x00000004;	/* root length: one entry */
+
+	block[2] = 0x61616161;	/* key_hi */
+	block[3] = 0x61616161;	/* key_lo */
+	block[4] = 0x74000000;	/* type=TEXT, reserved=0, length=0 */
+	block[5] = 0x00000000;	/* value */
+
+	dir = tb_property_parse_dir(block, 6);
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
+static void tb_test_property_parse_rootdir_overflow(struct kunit *test)
+{
+	u32 *block = kunit_kzalloc(test, 4 * sizeof(u32), GFP_KERNEL);
+	struct tb_property_dir *dir;
+
+	KUNIT_ASSERT_NOT_NULL(test, block);
+
+	block[0] = 0x55584401;	/* rootdir magic */
+	block[1] = 0x00000004;	/* root length claims 4 dwords of content */
+	block[2] = 0x61616161;
+	block[3] = 0x61616161;
+
+	/* content_offset(2) + content_len(4) = 6 > block_len(4) */
+	dir = tb_property_parse_dir(block, 4);
+	KUNIT_EXPECT_NULL(test, dir);
+	tb_property_free_dir(dir);
+}
+
 static struct kunit_case tb_test_cases[] = {
 	KUNIT_CASE(tb_test_path_basic),
 	KUNIT_CASE(tb_test_path_not_connected_walk),
@@ -2892,6 +2930,8 @@ static struct kunit_case tb_test_cases[] = {
 	KUNIT_CASE(tb_test_property_parse),
 	KUNIT_CASE(tb_test_property_format),
 	KUNIT_CASE(tb_test_property_copy),
+	KUNIT_CASE(tb_test_property_parse_zero_length),
+	KUNIT_CASE(tb_test_property_parse_rootdir_overflow),
 	{ }
 };
 
-- 
2.53.0

Re: [PATCH 0/6] thunderbolt: harden XDomain property exchange

[Mika Westerberg]

Hi,

On Mon, May 25, 2026 at 05:28:24AM -0400, Michael Bommarito wrote:
> This series fixes 4 memory-safety defects and 1 data-handling
> hardening issue in the Thunderbolt XDomain property exchange path
> (property.c and xdomain.c) and adds KUnit regression tests.
> All are reachable from an adjacent Thunderbolt peer without
> authentication.  The XDomain protocol runs automatically on cable
> insertion regardless of the configured security level, unless
> disabled with thunderbolt.xdomain=0.
> 
> Patches:
> 
>   1/6 - reject zero-length property entries in validator
>   2/6 - bound root directory content to block size
>   3/6 - clamp XDomain response data copy to allocation size
>   4/6 - validate XDomain request packet size before type cast
>   5/6 - limit XDomain response copy to actual frame size
>   6/6 - add KUnit tests for property parser bounds checks
> 
> Tested with KASAN on v7.1-rc3 and over Thunderbolt 4 hardware.
> KUnit regression tests (patch 6) confirm the fixes and existing
> tb_test_property_* tests pass on the patched tree.
> 
> Based-on: thunderbolt/fixes (928abe19fbf01)
> 
> Michael Bommarito (6):
>   thunderbolt: reject zero-length property entries in validator
>   thunderbolt: bound root directory content to block size
>   thunderbolt: clamp XDomain response data copy to allocation size
>   thunderbolt: validate XDomain request packet size before type cast
>   thunderbolt: limit XDomain response copy to actual frame size

All these applied to thunderbolt.git/fixes.

>   thunderbolt: test: add KUnit tests for property parser bounds checks

This one applied to thunderbolt.git/next.

Thanks!