All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Bommarito <michael.bommarito@gmail.com>
To: Mika Westerberg <westeri@kernel.org>, linux-usb@vger.kernel.org
Cc: Andreas Noever <andreas.noever@gmail.com>,
	Yehezkel Bernat <YehezkelShB@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Michael Jamet <michael.jamet@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: [PATCH v4 4/4] thunderbolt: test: add KUnit regression tests for XDomain property parser
Date: Sun, 10 May 2026 19:16:59 -0400	[thread overview]
Message-ID: <20260510231715.2215605-4-michael.bommarito@gmail.com> (raw)
In-Reply-To: <cover.v4.git.michael.bommarito@gmail.com>

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

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

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

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

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

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

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

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

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

  parent reply	other threads:[~2026-05-10 23:17 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-15  3:23 [PATCH 0/2] thunderbolt: harden XDomain property parser Michael Bommarito
2026-04-15  3:23 ` [PATCH 1/2] thunderbolt: property: harden XDomain property parser against crafted peer Michael Bommarito
2026-04-15  4:52   ` Mika Westerberg
2026-04-15 11:41     ` Michael Bommarito
2026-04-15  3:23 ` [PATCH 2/2] thunderbolt: test: add KUnit regression tests for XDomain property parser Michael Bommarito
2026-04-15 12:32 ` [PATCH v2 0/4] thunderbolt: harden " Michael Bommarito
2026-04-15 12:32   ` [PATCH v2 1/4] thunderbolt: property: reject u32 wrap in tb_property_entry_valid() Michael Bommarito
2026-04-27  5:35     ` Mika Westerberg
2026-05-02 17:55       ` Michael Bommarito
2026-04-15 12:32   ` [PATCH v2 2/4] thunderbolt: property: reject dir_len < 4 to prevent size_t underflow Michael Bommarito
2026-04-15 12:32   ` [PATCH v2 3/4] thunderbolt: property: cap recursion depth in __tb_property_parse_dir() Michael Bommarito
2026-04-15 12:32   ` [PATCH v2 4/4] thunderbolt: test: add KUnit regression tests for XDomain property parser Michael Bommarito
2026-04-27  5:40     ` Mika Westerberg
2026-05-03 14:15   ` [PATCH v3 0/4] thunderbolt: harden " Michael Bommarito
2026-05-03 14:15     ` [PATCH v3 1/4] thunderbolt: property: reject u32 wrap in tb_property_entry_valid() Michael Bommarito
2026-05-04  8:57       ` Andy Shevchenko
2026-05-03 14:15     ` [PATCH v3 2/4] thunderbolt: property: reject dir_len < 4 to prevent size_t underflow Michael Bommarito
2026-05-04  8:59       ` Andy Shevchenko
2026-05-03 14:15     ` [PATCH v3 3/4] thunderbolt: property: cap recursion depth in __tb_property_parse_dir() Michael Bommarito
2026-05-04  9:01       ` Andy Shevchenko
2026-05-04 12:54         ` Michael Bommarito
2026-05-03 14:15     ` [PATCH v3 4/4] thunderbolt: test: add KUnit regression tests for XDomain property parser Michael Bommarito
2026-05-05 11:48       ` Mika Westerberg
2026-05-10 23:16     ` [PATCH v4 0/4] thunderbolt: harden " Michael Bommarito
2026-05-10 23:16       ` [PATCH v4 1/4] thunderbolt: property: reject u32 wrap in tb_property_entry_valid() Michael Bommarito
2026-05-10 23:16       ` [PATCH v4 2/4] thunderbolt: property: reject dir_len < 4 to prevent size_t underflow Michael Bommarito
2026-05-10 23:16       ` [PATCH v4 3/4] thunderbolt: property: cap recursion depth in __tb_property_parse_dir() Michael Bommarito
2026-05-10 23:16       ` Michael Bommarito [this message]
2026-05-11  9:37       ` [PATCH v4 0/4] thunderbolt: harden XDomain property parser Mika Westerberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260510231715.2215605-4-michael.bommarito@gmail.com \
    --to=michael.bommarito@gmail.com \
    --cc=YehezkelShB@gmail.com \
    --cc=andreas.noever@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=michael.jamet@intel.com \
    --cc=stable@vger.kernel.org \
    --cc=westeri@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.