The ICM USB4 switch operation request encodes two values in
request.data_len_valid: bit 4 marks the data payload valid, while bits
3:0 hold the payload length in dwords. A zero length with the valid bit
set represents the full 16-dword data array.
icm_usb4_switch_op() sets the valid bit when a transmit payload is
present. For payloads shorter than the full 16 dwords, it then assigns
the length to the whole field and clears the valid bit that was just set.
The payload is still copied into the request, but the descriptor sent to
firmware marks that data as invalid.
This affects USB4 router operations that send short payloads through the
firmware connection manager. In particular, USB4 NVM writes can send a
short final block when the image size is not aligned to the 64-byte proxy
payload size. Firmware may then ignore or reject that final block, while
full 16-dword blocks are unaffected because they are encoded as length 0
with the valid bit set.
OR the short payload length into data_len_valid so the valid bit is
preserved.
Fixes: 9039387e16 ("thunderbolt: Add USB4 router operation proxy for firmware connection manager")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
tb_drom_parse_entry_port() validates the device-supplied header->index
against sw->config.max_port_number before indexing sw->ports[], but the
sibling field entry->dual_link_port_nr -- a 6-bit value also read from
the DROM -- indexes the same array with no such check. A malicious or
malformed Thunderbolt device can set dual_link_port_nr beyond the
allocated sw->ports[] (max_port_number + 1 entries), producing an
out-of-bounds tb_port pointer that is stored and later dereferenced.
Reject a port entry whose dual_link_port_nr exceeds max_port_number,
the same bound already applied to header->index.
Fixes: cd22e73bdf ("thunderbolt: Read port configuration from eeprom.")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Valid bandwidth group IDs range from 1 through MAX_GROUPS, while Group
ID 0 is reserved. tb_consumed_dp_bandwidth() uses the Group ID directly
to index its local group_reserved[] array.
The array currently has MAX_GROUPS entries, so its valid indices are 0
through MAX_GROUPS - 1. Group ID MAX_GROUPS therefore accesses one
element past the end, and the final group's reserved bandwidth is not
included when the array is summed.
Give group_reserved[] MAX_GROUPS + 1 entries so direct Group ID
indexing covers the reserved ID 0 and valid IDs 1 through MAX_GROUPS.
Fixes: 52a4490e89 ("thunderbolt: Reserve released DisplayPort bandwidth for a group for 10 seconds")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
The size passed to dma_unmap_page() must match the size used for the
corresponding dma_map_page() call.
Stream RX and TX buffers are mapped with TB_MAX_FRAME_SIZE when the
buffer pools are allocated. However, tbstream_ring_free() currently uses
tb_ring_frame_size() as the unmap size.
That helper returns the current frame payload size, not the DMA mapping
size. On the TX path, tbstream_dev_alloc_tx() stores a shorter payload
length in frame.size when the payload is smaller than TB_MAX_FRAME_SIZE.
This happens for a short final DATA frame, and also for the CLOSE frame,
which is allocated with SZ_256.
In those cases the buffer was mapped with TB_MAX_FRAME_SIZE, but
tb_ring_frame_size() returns the shorter frame payload length. This makes
the dma_unmap_page() size differ from the original dma_map_page() size.
Use TB_MAX_FRAME_SIZE when unmapping stream buffers so the unmap size
matches the DMA mapping size used by the buffer allocation paths.
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Fixes: 6db21d817b ("thunderbolt: Add support for USB4STREAM")
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Pull USB and Thunderbolt driver updates from Greg KH:
"Here is the big set of USB and Thunderbolt driver changes for 7.2-rc1.
Lots of little stuff in here, major highlights include:
- USB4STREAM support for Thunderbolt devices. A new way to send "raw"
data very quickly over a USB4 connection to another system directly
- Other thunderbolt updates and changes to make the stream code work
- xhci driver updates and additions
- typec driver updates and additions
- usb gadget driver updates and fixes for reported issues
- zh_CN documentation translation of the USB documentation
- usb-serial driver updates
- dts cleanups for some USB platforms
- other minor USB driver updates and tweaks
All of these have been in linux-next for over a week with no reported
issues, most of them for many many weeks"
* tag 'usb-7.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb: (131 commits)
usb: ucsi: huawei_gaokun: support mode switching
thunderbolt: debugfs: Fix sideband write size check
thunderbolt: debugfs: Fix margining error counter buffer leak
usb: host: xhci-rcar: Split R-Car Gen2 and Gen3 .plat_start() handling
usb: host: xhci-rcar: Remove SET_XHCI_PLAT_PRIV_FOR_RCAR() macro
usb: xhci: allocate internal DCBAA mirror dynamically
usb: xhci: allocate DCBAA based on host controller max slots
usb: xhci: refactor DCBAA struct
xhci: Prevent queuing new commands if xhci is inaccessible
xhci: dbc: detect and recover hung DbC during enumeraton
xhci: dbc: add timestamps to DbC state changes in a new helper.
xhci: dbc: add helper to set and clear DbC DCE enable bit
xhci: dbc: serialize enabling and disabling dbc
xhci: dbc: Fix sysfs ABI Documentation for xhci dbc states
usb: xhci: Improve Soft Retries after short transfers
usb: xhci: Remove isochronous URB_SHORT_NOT_OK handling
usb: xhci: Remove skip_isoc_td()
usb: xhci: Simplify xhci_quiesce()
usb: xhci: remove legacy 'num_trbs_free' tracking
usb: xhci: fix typo in xhci_set_port_power() comment
...
Mika writes:
thunderbolt: Changes for v7.2 merge window
This includes following USB4/Thunderbolt changes for the v7.2 merge
window:
- Make the driver more compliant with the connection manager guide.
- Improvements over Thunderbolt XDomain service handling.
- USB4STREAM driver.
- Split out PCIe bits into pci.c to allow the driver to work on
non-PCIe hosts as well.
- Various fixes and improvements.
All these have been in linux-next with no reported issues.
* tag 'thunderbolt-for-v7.2-rc1' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt: (41 commits)
thunderbolt: debugfs: Fix sideband write size check
thunderbolt: debugfs: Fix margining error counter buffer leak
thunderbolt: test: Release third DP tunnel
thunderbolt: Prevent XDomain delayed work use-after-free on disconnect
thunderbolt: test: Add KUnit tests for property parser bounds checks
thunderbolt: Add some more descriptive probe error messages
thunderbolt: Require nhi->ops be valid
thunderbolt: Separate out common NHI bits
thunderbolt: Move pci_device out of tb_nhi
thunderbolt: Increase Notification Timeout to 255 ms for USB4 routers
thunderbolt: Increase timeout for Configuration Ready bit
thunderbolt: Verify Router Ready bit is set after router enumeration
thunderbolt: Verify PCIe adapter in detect state before tunnel setup
thunderbolt: Activate path hops from source to destination
thunderbolt: Fix lane bonding log when bonding not possible
thunderbolt: Don't access path config space on Lane 1 adapters in tb_switch_reset_host()
thunderbolt: Improve multi-display DisplayPort tunnel allocation
docs: admin-guide: thunderbolt: Add instructions how to use USB4STREAM
thunderbolt: Add support for USB4STREAM
thunderbolt: Add support for ConfigFS
...
sb_regs_write() looks up the matching sideband register entry before
validating the number of bytes to write.
However, the size check uses sb_regs->size, which is the size of the
first entry in the register table, instead of the matched entry. This
rejects valid writes to larger sideband registers such as USB4_SB_DEBUG
or USB4_SB_DATA.
Use the matched register entry for the size check.
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
When USB4 lane margining debugfs write support is enabled,
margining_error_counter_write() copies the user input with
validate_and_copy_from_user(). This allocates a temporary page that is
only needed while parsing the requested error counter mode.
The function currently returns without freeing that page. This leaks one
page per write to the error_counter debugfs file, including successful
writes and writes that later fail while taking the domain lock or because
software margining is not enabled.
Free the temporary page once parsing has completed, and also before
returning from the invalid-input path.
Fixes: 10904df3f2 ("thunderbolt: Improve software receiver lane margining")
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
tb_test_tunnel_3dp() allocates three DisplayPort tunnels
but only releases the first two before returning. Release the
third tunnel as well to keep the test cleanup balanced.
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
tb_xdp_handle_request() runs on system_wq and queues
xd->state_work via queue_delayed_work() in three request handlers:
PROPERTIES_CHANGED_REQUEST, UUID_REQUEST (via start_handshake),
and LINK_STATE_CHANGE_REQUEST. Similarly, update_xdomain() queues
xd->properties_changed_work when local properties change.
Concurrently, tb_xdomain_remove() calls stop_handshake() which does
cancel_delayed_work_sync() on both delayed works. Later,
tb_xdomain_unregister() calls device_unregister() which eventually
frees the xdomain. Since commit 559c1e1e01 ("thunderbolt: Run
tb_xdp_handle_request() in system workqueue") moved the request
handler off tb->wq, the handler and the remove path are no longer
serialized. If queue_delayed_work() executes after
cancel_delayed_work_sync() but before the xdomain is freed, the
delayed work fires on a freed object.
Add xd->removing that tb_xdomain_remove() sets under xd->lock
before calling stop_handshake(). Each external queue site holds
the same lock and checks removing before calling
queue_delayed_work(). This provides the mutual exclusion needed:
either the queue site acquires the lock first and queues work that
the subsequent cancel will see, or the remove path acquires the
lock first and the queue site observes removing == true and skips
the queue.
Fixes: 559c1e1e01 ("thunderbolt: Run tb_xdp_handle_request() in system workqueue")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
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: cdae7c07e3 ("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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
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: 8e1de70425 ("thunderbolt: Add support for XDomain lane bonding")
Fixes: cdae7c07e3 ("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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
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: cdae7c07e3 ("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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
__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: cdae7c07e3 ("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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
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: cdae7c07e3 ("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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Currently there's a lot of silent error-return paths in various places
where nhi_probe() can fail. Sprinkle some prints to make it clearer
where the problem is.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Because of how fundamental ops->init_interrupts() is, it no longer
makes sense to consider cases where nhi->ops is NULL.
Drop some boilerplate around it and add a single sanity-check in
nhi_probe() instead.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Add a new file encapsulating most of the PCI NHI specifics
(intentionally leaving some odd cookies behind to make the layering
simpler). Most notably, separate out nhi_probe() to make it easier to
register other types of NHIs.
Also, fold in Intel Icelake (nhi_ops.c) support to contain all
PCIe-related bits in pci.c.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Not all USB4/TB implementations are based on a PCIe-attached
controller. In order to make way for these, start off with moving the
pci_device reference out of the main tb_nhi structure.
Encapsulate the existing struct in a new tb_nhi_pci, that shall also
house all properties that relate to the parent bus. Similarly, any
other type of controller will be expected to contain tb_nhi as a
member.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Currently we set the Notification Timeout field in ROUTER_CS_4 for USB4
routers to 10 ms, which is unnecessarily short and may cause unnecessary
retransmissions of Hot Plug packets by the router in case of slow
software response.
Increase the timeout to 255 ms, aligning with Thunderbolt 3 routers
and providing adequate time for software to process Hot Plug Events.
While there, fix the comment describing the Notification Timeout
field to match the USB4 specification.
Signed-off-by: Gil Fine <gil.fine@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
After setting the Configuration Valid bit (ROUTER_CS_5.CV), the USB4
Connection Manager guide specifies a 500 ms timeout for the router to
set the Configuration Ready bit (ROUTER_CS_6.CR). The current timeout
is shorter than specified. While there, fix the kernel-doc typo.
Increase the timeout to match the CM guide recommendation.
Signed-off-by: Gil Fine <gil.fine@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
The USB4 Connection Manager guide specifies that after enumerating a
router, the Connection Manager shall verify that the Router Ready bit
(ROUTER_CS_6.RR) has been set to ensure hardware configuration has completed.
Currently, this step is missing from the enumeration sequence.
Add this check to follow the Connection Manager guide more closely.
Signed-off-by: Gil Fine <gil.fine@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
The USB4 Connection Manager guide suggests that a PCIe downstream and
PCIe upstream adapters of the USB4 router is in the Detect state before
setting up a PCIe tunnel.
Add this check by verifying the LTSSM field in ADP_PCIE_CS_0 before
tunnel setup.
Signed-off-by: Gil Fine <gil.fine@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Currently, path activation starts from the last hop (destination adapter)
and iterates backwards to the first hop (source adapter). This does not
follow the order suggested in the USB4 Connection Manager guide and could
potentially cause issues with tunnelled protocols.
Reverse the activation order to start from the first hop (source adapter)
and end at the last hop (destination adapter), as suggested in the
Connection Manager guide.
Adjust the rollback in the failure path to deactivate from the first
hop, since hops are now activated starting at the source.
Fix kernel-doc accordingly.
Signed-off-by: Gil Fine <gil.fine@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Currently if lane bonding is not possible or not supported, we continue
and read the updated number of Total Buffers from lane adapters unnecessarily
and incorrectly log the bonding as successful.
Fix this by bailing out early when bonding is not possible, avoiding
the unnecessary read and the misleading log message.
Signed-off-by: Gil Fine <gil.fine@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
USB4 Lane 1 adapters do not have accessible path config space. Skip the
path config space cleanup in tb_switch_reset_host() for these ports. The
check is for USB4 switches only. Thunderbolt 1-3 Lane 1 adapters stay as
is because we do need to program their path config space.
Co-developed-by: Rene Sapiens <rene.sapiens@linux.intel.com>
Signed-off-by: Rene Sapiens <rene.sapiens@linux.intel.com>
Signed-off-by: Pooja Katiyar <pooja.katiyar@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
When 3 monitors are connected through Thunderbolt dock to the system at
once, one of the monitors might fail to establish DisplayPort tunnel.
This happens during DP bandwidth negotiation - each monitor takes
maximum bandwidth that is supported and there might not be enough for
3rd display. In this case Thunderbolt driver drops DP tunnel and
'forgets' about it but with DP bandwidth allocation mode, that comes in
later, some bandwidth might be freed.
Make Thunderbolt driver check again if DP tunnel can be established
after DP bandwidth consumption changed.
Signed-off-by: Alan Borzeszkowski <alan.borzeszkowski@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Introduce USB4STREAM protocol and Linux implementation. This allows two
(or more) hosts to transfer data directly over Thunderbolt/USB4 cable
through a character device without need to go through the network stack.
Any application that supports read(2) and write(2) in some form should
be able to use the device without changes. The data is sent out to the
other side over a tunnel inside Thunderbolt/USB4 fabric. The character
device is called /dev/tbstreamX where X is the minor number starting
from 0.
All stream devices need to be configured first. This is done through
ConfigFS interface. There can be multiple streams at the same time (this
depends on number of DMA rings and available HopIDs) and a single stream
supports traffic in both directions. For example there could be an
application that uses one stream as control channel and another one as
bi-directional data channel.
A real use-case for this is to take a backup as a part of recovery
initramfs tooling (no need to setup networking or have ssh or similar
tooling as part of the initramfs). Say we want to backup the disk of
host1 to host2. First Thunderbolt/USB4 cable is connected between the
hosts (there can be devices in the middle too) then the receiving side
configures the stream:
host2 # mkdir /sys/kernel/config/thunderbolt/stream/0-1.0
host2 # mkdir /sys/kernel/config/thunderbolt/stream/0-1.0/backup
host2 # echo -1 > /sys/kernel/config/thunderbolt/stream/0-1.0/backup/in_hopid
host2 # echo -1 > /sys/kernel/config/thunderbolt/stream/0-1.0/backup/out_hopid
We use automatic HopID allocation (writing -1 to HopIDs) for simplicity.
From this point forward the /dev/tbstream0 can be used pretty much as
regular file:
host2 # dd if=/dev/tbstream0 of=/tmp/host1.nvme0n1.backup-$(date +%F) bs=256k
The host that is being backed up then configures the stream accordingly:
host1 # mkdir /sys/kernel/config/thunderbolt/stream/0-503.0
host1 # mkdir /sys/kernel/config/thunderbolt/stream/0-503.0/backup
Here we take advantage of the fact that host2 also announces the active
streams through XDomain properties so the name "backup" gives us the
HopIDs. It is also possible to configure them manually in the same way
we did for host2.
Then it is just a matter of copying the data over:
host1 # dd if=/dev/nvme0n1 of=/dev/tbstream0 bs=256k
Similarly it is possible to transfer parts of the filesystem. For
example copy contents of mydir over to the host2:
host2 # gunzip < /dev/tbstream0 | tar xf -
host1 # tar cf - mydir | gzip > /dev/tbstream0
Other end of the spectrum use-case is "borrowing" laptop (host1) camera
to desktop (host2):
host2 # gst-launch-1.0 filesrc location=/dev/tbstream0 ! jpegdec ! videoconvert ! \
autovideosink
host1 # gst-launch-1.0 v4l2src device=/dev/video0 ! video/x-raw,width=1920,height=1080 ! \
jpegenc quality=90 ! filesink location=/dev/tbstream0
Once the streams are no longer needed they can be removed:
host1 # cd /sys/kernel/config/thunderbolt/stream/
host1 # rmdir -p 0-503.0/backup
host2 # cd /sys/kernel/config/thunderbolt/stream
host2 # rmdir -p 0-1.0/backup
Co-developed-by: Alan Borzeszkowski <alan.borzeszkowski@linux.intel.com>
Signed-off-by: Alan Borzeszkowski <alan.borzeszkowski@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
This adds ConfigFS support to USB4/Thunderbolt bus. By itself this just
creates the subsystem but it exposes functions that can be used to
register groups under it.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
This allows the caller to wait for the ring to be empty. We are going to
need this in the upcoming userspace tunneling support.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Instead of the core driver programming fixed value for throttling let
the service drivers to specify the interval if they need this.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
The XDomain properties can be useful for service drivers, for example to
implement a registry for the services they expose. So far there has been
no need for service drivers to specify these but with the USB4STREAM
driver that we are going to use them.
This adds remote and local side properties that the service drivers have
access to. Remote side is read-only but the local side can be changed by
a service driver. Also provide a mechanism to notify the remote side
that there are changes.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
This allows merging one XDomain property directory into another. We are
going to use this in the subsequent patch.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
A DIRECTORY entry's value field is used as the dir_offset for a
recursive call into __tb_property_parse_dir() with no depth counter.
A crafted peer that chains DIRECTORY entries into a back-reference
loop drives the parser until the kernel stack is exhausted and the
guard page fires. Any untrusted XDomain peer (cable, dock, in-line
inspector, adjacent host) that reaches the PROPERTIES_REQUEST
control-plane exchange can trigger this without authentication.
Thread a depth counter through tb_property_parse() and
__tb_property_parse_dir(), and reject blocks that exceed
TB_PROPERTY_MAX_DEPTH = 8. That is comfortably larger than any
observed legitimate XDomain layout.
Operators who do not need XDomain host-to-host discovery can disable
the path entirely with thunderbolt.xdomain=0 on the kernel command
line.
Fixes: cdae7c07e3 ("thunderbolt: Add support for XDomain properties")
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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
On the non-root path, __tb_property_parse_dir() takes dir_len from
entry->length (u16 widened to size_t). Two distinct OOB conditions
follow when entry->length < 4:
1. The non-root path begins with kmemdup(&block[dir_offset],
sizeof(*dir->uuid), ...) which always reads 4 dwords from
dir_offset. tb_property_entry_valid() only enforces
dir_offset + entry->length <= block_len, so a crafted entry
with dir_offset close to the end of the property block and
entry->length in 0..3 passes that gate but lets the UUID copy
run off the block (e.g. dir_offset = 497, dir_len = 3 in a
500-dword block reads block[497..501]).
2. After the kmemdup, content_len = dir_len - 4 underflows size_t
to ~SIZE_MAX, nentries becomes SIZE_MAX / 4, and the entry
walk runs OOB on each iteration until an entry fails
validation or the kernel oopses on an unmapped page.
Reject dir_len < 4 on the non-root path *before* the UUID kmemdup,
which closes both holes.
Also move INIT_LIST_HEAD(&dir->properties) up to immediately after
the dir allocation so the new error-return path (and the existing
uuid-alloc failure path) calling tb_property_free_dir() sees a
walkable list rather than the zero-initialized NULL next/prev that
list_for_each_entry_safe() would oops on.
Fixes: cdae7c07e3 ("thunderbolt: Add support for XDomain properties")
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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
entry->value is u32 and entry->length is u16; the sum is performed in
u32 and wraps. A malicious XDomain peer can pick
value = 0xffffff00, length = 0x100 so the sum 0x100000000 wraps to 0
and passes the > block_len check. tb_property_parse() then passes
entry->value to parse_dwdata() as a dword offset into the property
block, reading attacker-directed memory far past the allocation.
For TEXT-typed entries with the "deviceid" or "vendorid" keys this
lands in xd->device_name / xd->vendor_name and is readable back via
the per-XDomain device_name / vendor_name sysfs attributes; the leak
is NUL-bounded (kstrdup() stops at the first zero byte) and
untargeted (the attacker picks a delta, not an absolute address).
DATA-typed entries are parsed into property->value.data but not
generically surfaced to userspace.
Use check_add_overflow() so a wrapped sum is rejected.
Fixes: cdae7c07e3 ("thunderbolt: Add support for XDomain properties")
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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Firmware connection manager supports only one DMA tunnel per XDomain
connection. Firmware prior Intel Titan Ridge failed the operation
directly but the same does not happen anymore on Titan Ridge and
forward. For this reason add an explicit check, and fail the operation
accordingly in the driver.
Signed-off-by: Alan Borzeszkowski <alan.borzeszkowski@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Currently we call device_unregister() for services and the XDomain
itself with tb->lock held. This prevents the service drivers from
calling any functions that may take it. For this reason separate
removing the XDomain from the topology data structures (where we need
the lock) from unregistering the device from the bus (where remove
callbacks of the drivers are being called).
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
We add them as part of the register path so to keep it symmetric remove
them as part of the unregister path. This also removes them even if the
service itself is not yet released (but is unregistered), thus allowing
new register with the same service name to happen.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
We don't actually need to store the debugfs directory pointer inside
struct dma_test. Instead we can use the debugfs_lookup_and_remove()
which also handles the case if the debugfs directory is already removed
by the core driver (for example when cable is disconnected).
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
This is needed because we release the service ID in tb_service_release()
and the ID array is owned by the parent XDomain.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
We should not call nhi_shutdown() before the domain structure and the
control channel rings are completely released. Otherwise we might
release resources like the nhi->msix_ida that are still referenced in
tb_domain_release(). For this reason wait for the tb_domain_release() to
be completed before continuing to nhi_shutdown() and eventually
releasing of the rest of the data structures.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Similarly what we do with the firmware connection manager. This makes
tb_xdp_handle_request() return error to the remote host. However, we
need to make sure we keep the uuid alive so that we can reply until the
whole domain is released.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
If tb_cfg_request() fails setting up the request (for example the
control channel is shut down already) it returns an error without
calling the callback. To avoid leaking that memory, call
tb_cfg_request_put() if tb_cfg_request() fails.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
We process hotplug events in a workqueue that may run after the domain
has been removed by tb_domain_remove(). For example if user unloads the
driver while at the same time plugging a device router we may have
scheduled tb_handle_hotplug() to run. Avoid possible UAF in this case by
taking the domain reference before scheduling the hotplug handler in
tb_queue_hotplug().
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
The USB4 v2 Inter-Domain spec "unified" the lane bonding flow so that
when the other end (with higher UUID) is not yet set the target link
width accordingly it is expected to reply with ERROR_NOT_READY.
Implement this for Linux.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
