[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