| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Mika writes:
thunderbolt: Fixes for v7.3-rc6
This includes following USB4/Thunderbolt fixes:
- Fix XDomain property parser to reject oversized properties
responses.
- Revert a quirk that causes deadlock at shutdown.
- Fix USB4STREAM to announce FMODE_NOWAIT.
- Add a quirk that disables CL states for Anker Prime TB5 dock to
avoid link instability.
All these have been in linux-next with no reported issues.
* tag 'thunderbolt-for-v7.3-rc6' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt:
thunderbolt: Disable CL states for the Anker Prime TB5 dock
thunderbolt: stream: Announce support for FMODE_NOWAIT
Revert "thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers"
thunderbolt: Reject oversized XDomain properties responses
|
|
The Anker Prime TB5 dock identifies itself in the DROM as 01ea:83b5.
Its router config space is an Intel Barlow Ridge 80G hub (8087:5786).
The upstream lane adapter advertises CL0s, CL1, and CL2. With CLx left
enabled, TMU uni-directional LowRes setup fails with -ENOTCONN, the USB3
and DisplayPort tunnels are aborted, and the router reconnects in a loop.
Loading thunderbolt with clx=0 keeps the link up on this machine.
Match the DROM id together with the Barlow Ridge hub id and disable CL
states for this dock only. QUIRK_NO_CLX takes the same early-out as the
module parameter, without disabling CLx for every other router.
Closes: https://lore.kernel.org/linux-usb/SJ0PR15MB4696D491504DB667AA8E0617A3812@SJ0PR15MB4696.namprd15.prod.outlook.com/
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Kurt Lieber <kurt@lieber.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Commit 42bc6935339b ("thunderbolt: stream: Support IOCB_NOWAIT in
non-blocking I/O as well") added support for IOCB_NOWAIT but forgot to
actually announce it as part of the file->f_mode. Add this now so users
such as io_uring can actually take advantage of IOCB_NOWAIT.
Fixes: 42bc6935339b ("thunderbolt: stream: Support IOCB_NOWAIT in non-blocking I/O as well")
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
This commit has caused a deadlock at shutdown. A proper fix with
another approach will be coming later. Revert commit
f1de1fc5f632cdeae1f5c2984572ab710d4dfcaa for now.
Reported-by: juan.martinez@amd.com
Closes: https://lore.kernel.org/linux-usb/20260825214237.4179813-1-juan.martinez@amd.com/
Cc: Sanath S <Sanath.S@amd.com>
Cc: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
tb_xdp_properties_request() allocates room for 45 data dwords in its
252-byte response buffer. The XDomain length field is six bits wide,
however, and a malicious peer can set it to 63. After the fixed response
fields are subtracted, the driver treats this as 48 data dwords.
Commit 322e93448d90 ("thunderbolt: Clamp XDomain response data copy to
allocation size") only bounds the copy against data_len. If data_len is
at least 48, memcpy() reads 192 bytes from the 180-byte res->data array,
causing a 12-byte heap out-of-bounds read. Commit 4db2bd2ed478
("thunderbolt: Limit XDomain response copy to actual frame size") limits
the earlier copy but does not constrain this header-derived length.
Reject response data lengths that exceed the allocated source buffer
before copying them into the assembled property block.
Fixes: d1ff70241a27 ("thunderbolt: Add support for XDomain discovery protocol")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Mika writes:
thunderbolt: Fixes for v7.3-rc3
This includes following USB4/Thunderbolt fixes:
- Fix various issues around asynchronous DisplayPort tunnel activation
when there is no graphics driver doing doing the capability
exchange.
- Fix potential NULL pointer dereference when XDomain connection is
removed.
- Fix use-after-free when control channel request is canceled.
- Fix lockdep false positive.
- Revert a commit that causes XDomain properties ping-pong.
All these have been in linux-next with no reported issues.
* tag 'thunderbolt-for-v7.3-rc3' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt:
Revert "thunderbolt: xdomain: Notify peers after enumeration"
thunderbolt: Use separate lock class for each ring
thunderbolt: Fix KASAN reported use-after-free when request is canceled
thunderbolt: Fix NULL dereference in tb_remove_work()
thunderbolt: Tear down inactive DP tunnels when the domain is stopped
thunderbolt: Mark discovered tunnels as active
thunderbolt: Don't access a DP tunnel after its DPRX read was canceled
thunderbolt: Fix domain reference leak when DPRX read is canceled
thunderbolt: Make the DP tunnel activation callback mandatory
thunderbolt: Hold a router reference for each allocated HopID
|
|
This is another run of the Coccinelle script for converting kmalloc()
family of allocations to kmalloc_obj() via the existing rules in
scripts/coccinelle/api/kmalloc_objs.cocci
This catches both the set of kmalloc() uses added since the first
kmalloc_obj() conversions in v7.0 and adds a large group missed in the
first pass due to Coccinelle not interacting well with the cleanup.h
scoped_...() family of macros[1]. I worked around this with spatch's
"--macro-file" argument to a file with all the scoped_...() macros mapped
to Coccinelle's YACFE_ITERATOR[2] as that was the closest viable control
flow indicator I could find.
Build tested allmodconfig on x86, arm64, arm, loongarch, mips, powerpc,
riscv, and s390 with no new warnings.
Link: https://lore.kernel.org/lkml/202609021314.8A9C0B8@keescook/ [1]
Link: https://github.com/coccinelle/coccinelle/blob/master/standard.h [2]
Signed-off-by: Kees Cook <kees+treewide@kernel.org>
|
|
This reverts commit e027dba038f0008df9bc9575f5c3e803e90636c6.
Alan noticed that this causes the peers send change properties to each
other continuously.
Reported-by: Borzeszkowski, Alan <alan.borzeszkowski@intel.com>
Cc: Milo Chen <cmh79479@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
When connected to another host and then unplugging cable lockdep
triggers following:
======================================================
WARNING: possible circular locking dependency detected
7.1.0-rc2+ #1775 Tainted: G U
------------------------------------------------------
kworker/u16:6/312 is trying to acquire lock:
ffff8881179c70a8 ((work_completion)(&ring->work)){+.+.}-{0:0}, at: __flush_work+0x3cf/0xd10
but task is already holding lock:
ffff8881a8b810b0 (&net->connection_lock){+.+.}-{4:4}, at: tbnet_tear_down+0x110/0x720 [thunderbolt_net]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&net->connection_lock){+.+.}-{4:4}:
__mutex_lock+0x19a/0x2490
mutex_lock_nested+0x1b/0x30
tbnet_handle_packet+0x74c/0xd70 [thunderbolt_net]
tb_xdomain_handle_request+0x37c/0x4b0 [thunderbolt]
tb_domain_event_cb+0xc9/0x140 [thunderbolt]
tb_ctl_handle_event+0xd6/0x2c0 [thunderbolt]
tb_ctl_rx_callback+0x22c/0xa10 [thunderbolt]
ring_work+0x715/0xcb0 [thunderbolt]
process_one_work+0x902/0x1790
worker_thread+0x5cd/0xfe0
kthread+0x339/0x420
ret_from_fork+0x79a/0x9d0
ret_from_fork_asm+0x1a/0x30
-> #0 ((work_completion)(&ring->work)){+.+.}-{0:0}:
__lock_acquire+0x1592/0x2640
lock_acquire+0x1a3/0x300
__flush_work+0x3e9/0xd10
flush_work+0x21/0x30
tb_ring_stop+0x240/0x840 [thunderbolt]
tbnet_tear_down+0x2ff/0x720 [thunderbolt_net]
tbnet_stop+0x47/0x1a0 [thunderbolt_net]
__dev_close_many+0x19e/0x4e0
netif_close_many+0x1e8/0x640
unregister_netdevice_many_notify+0x6d3/0x22d0
unregister_netdevice_queue+0x2b9/0x3a0
unregister_netdev+0x1c/0x70
tbnet_remove+0x52/0xb0 [thunderbolt_net]
tb_service_remove+0x8a/0xe0 [thunderbolt]
device_remove+0xc5/0x190
device_release_driver_internal+0x3db/0x590
device_release_driver+0x12/0x20
bus_remove_device+0x2c1/0x580
device_del+0x3d9/0x9f0
device_unregister+0x17/0xc0
unregister_service+0x46/0x60 [thunderbolt]
device_for_each_child_reverse+0xfa/0x180
tb_xdomain_unregister+0x57/0xe0 [thunderbolt]
unregister_unplugged_xdomain+0x101/0x1a0 [thunderbolt]
bus_for_each_dev+0x111/0x1a0
tb_domain_unregister_unplugged_xdomains+0x98/0xe0 [thunderbolt]
tb_handle_hotplug+0xc3/0x2bb0 [thunderbolt]
process_one_work+0x902/0x1790
worker_thread+0x5cd/0xfe0
kthread+0x339/0x420
ret_from_fork+0x79a/0x9d0
ret_from_fork_asm+0x1a/0x30
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&net->connection_lock);
lock((work_completion)(&ring->work));
lock(&net->connection_lock);
lock((work_completion)(&ring->work));
This in fact is false positive because they involve unrelated rings (and
unrelated work structures). In the first one it is ring 0 which is used
for control traffic and in the second it is dealing with another ring
used for the high-speed traffic.
Fix this by using separate lock class for each ring worker.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Alan reported that when doing stress testing sometimes KASAN notices
use-after-free during control channel operation (stripped down keeping
the relevant parts):
BUG: KASAN: slab-use-after-free in tb_cfg_request_sync+0x240/0x250 [thunderbolt]
Read of size 24 at addr ffff88811067f290 by task kworker/u40:2/1760
<TASK>
tb_cfg_request_sync+0x240/0x250 [thunderbolt]
tb_cfg_read_raw+0x367/0x510 [thunderbolt]
tb_cfg_read+0xec/0x240 [thunderbolt]
tb_port_get_link_generation+0x258/0x420 [thunderbolt]
tb_usb3_consumed_bandwidth+0x1c1/0x2c0 [thunderbolt]
tb_tunnel_consumed_bandwidth+0xfd/0x910 [thunderbolt]
tb_available_bandwidth+0x5f2/0xeb0 [thunderbolt]
tb_recalc_estimated_bandwidth+0x2a0/0x1bc0 [thunderbolt]
tb_handle_dp_bandwidth_request+0x1897/0x5e20 [thunderbolt]
process_one_work+0x675/0x1230
worker_thread+0x5e6/0xf70
kthread+0x365/0x470
ret_from_fork+0x54d/0x710
ret_from_fork_asm+0x1a/0x30
</TASK>
Allocated by task 1760:
__kmalloc_cache_noprof+0x1ee/0x550
tb_cfg_read_raw+0x1d3/0x510 [thunderbolt]
tb_cfg_read+0xec/0x240 [thunderbolt]
tb_port_get_link_generation+0x258/0x420 [thunderbolt]
tb_usb3_consumed_bandwidth+0x1c1/0x2c0 [thunderbolt]
tb_tunnel_consumed_bandwidth+0xfd/0x910 [thunderbolt]
tb_available_bandwidth+0x5f2/0xeb0 [thunderbolt]
tb_recalc_estimated_bandwidth+0x2a0/0x1bc0 [thunderbolt]
tb_handle_dp_bandwidth_request+0x1897/0x5e20 [thunderbolt]
process_one_work+0x675/0x1230
worker_thread+0x5e6/0xf70
kthread+0x365/0x470
ret_from_fork+0x54d/0x710
ret_from_fork_asm+0x1a/0x30
Freed by task 926:
kfree+0x18f/0x4a0
tb_cfg_request_put+0xb7/0xe0 [thunderbolt]
tb_cfg_request_work+0x82/0x120 [thunderbolt]
process_one_work+0x675/0x1230
worker_thread+0x5e6/0xf70
kthread+0x365/0x470
ret_from_fork+0x54d/0x710
ret_from_fork_asm+0x1a/0x30
Second to last potentially related work creation:
__queue_work+0x575/0xd00
queue_work_on+0x77/0x80
tb_cfg_request_cancel+0xc7/0x260 [thunderbolt]
tb_cfg_request_sync+0x1f6/0x250 [thunderbolt]
tb_cfg_read_raw+0x367/0x510 [thunderbolt]
tb_cfg_read+0xec/0x240 [thunderbolt]
tb_port_get_link_generation+0x258/0x420 [thunderbolt]
tb_usb3_consumed_bandwidth+0x1c1/0x2c0 [thunderbolt]
tb_tunnel_consumed_bandwidth+0xfd/0x910 [thunderbolt]
tb_available_bandwidth+0x5f2/0xeb0 [thunderbolt]
tb_recalc_estimated_bandwidth+0x2a0/0x1bc0 [thunderbolt]
tb_handle_dp_bandwidth_request+0x1897/0x5e20 [thunderbolt]
process_one_work+0x675/0x1230
worker_thread+0x5e6/0xf70
kthread+0x365/0x470
ret_from_fork+0x54d/0x710
ret_from_fork_asm+0x1a/0x30
The last stack trace is helpful because it shows that we are cancelling
a request and looking at tb_cfg_request_cancel() what might happen is
that tb_cfg_request_work() completes right before tb_cfg_request_cancel()
starts and because of this it will call schedule_work() queueing the
same work to run again. However, it is already removed from the
request_queue and reference count is dropped so when
tb_cfg_request_work() triggers again it will access memory that is
already released.
Fix this so that we first make sure a cancelled request is not handed
away from tb_cfg_request_find() or scheduled to run. Then instead of
relying on the worker to clean up the request we will do it in
tb_cfg_request_cancel() after the work is canceled from running. Make
tb_cfg_request_dequeue() release the request only if it was actually
removed from the queue.
Reported-by: Alan Borzeszkowski <alan.borzeszkowski@linux.intel.com>
Fixes: d7f781bfdbf4 ("thunderbolt: Rework control channel to be more reliable")
Cc: stable@vger.kernel.org
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
There is a slight race between tb_remove_work() and tb_domain_remove()
which leads to dereferencing a NULL tb->root_switch pointer inside
tb_free_unplugged_xdomains():
Thread A Thread B
tb_remove_work()
tb_domain_remove()
mutex_lock(&tb->lock)
tb_stop()
/* doesn't cancel a running callback */
cancel_delayed_work(&tcm->remove_work)
...
tb_switch_remove(tb->root_switch)
tb->root_switch = NULL
mutex_unlock(&tb->lock)
mutex_lock(&tb->lock)
...
/* without checking ->root_switch */
tb_free_unplugged_xdomains(tb->root_switch)
mutex_unlock(&tb->lock)
Commit a8937f35cf39 ("thunderbolt: Remove XDomain from the bus without
holding tb->lock") doesn't seem right to move tb_free_unplugged_xdomains()
out of the &tb->lock section and the check for tb->root_switch, in
particular. It states:
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).
tb_free_unplugged_xdomains() belongs to the former group of functions
requiring the lock. And it also calls tb_xdomain_remove() which should
only be called with &tb->lock held.
Found by Linux Verification Center (linuxtesting.org) with Svace static
analysis tool.
Fixes: a8937f35cf39 ("thunderbolt: Remove XDomain from the bus without holding tb->lock")
Cc: stable@vger.kernel.org
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
tb_stop() only tears down DMA tunnels so a DP tunnel that is still
waiting for dprx_work to complete keeps that work queued while the
routers are removed and the control channel is stopped. The work only
stops once the DPRX timeout has passed and because it requeues itself
until then the flush_workqueue() in tb_domain_remove() won't wait for its
final run. The callback then runs against a domain that is already torn
down. A reference to that domain is kept so the completion waiting for
that domain to disappear in unbind will block until the timeout is
eventually reached.
Tear down DP tunnels that are not active yet as well which also cancels
that work. Tunnels for displays that are already alive are untouched and
keep working.
Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Discovered tunnels that have been activated by whatever was running
before us stay in TB_TUNNEL_INACTIVE until hibernation restore such that
anything depending on tb_tunnel_is_active() skips them. Mark them active
during discovery instead. These now also emit TUNNEL_EVENT=activated
uevents during discovery.
Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
tb_dp_dprx_work() checks ->dprx_canceled before it takes tb->lock so it
misses a tb_dp_dprx_stop() that could not cancel the already running
work. It then polls the DPRX capabilities and runs the callback for a
tunnel that has already been torn down while the domain is suspending or
going away.
This can be hit by cancelling the DPRX read from outside the ordered
tb->wq: During suspend tb_disconnect_and_release_dp() does just this and
with a later patch tb_stop() will do it as well. The latter in combination
with the Apple NHI where the DPRX read never completed is how I hit
this.
Check the flag with tb->lock held instead and check it again in
tb_dp_tunnel_active() because the callback runs after the lock has been
dropped again.
Also clear the flag in tb_dp_dprx_start() so that it only ever describes
the work that is currently in flight.
Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
tb_tunnel_one_dp() takes a domain reference which is only dropped once
tb_dp_tunnel_active() has run on the work queue. If that work is
cancelled that reference is leaked. Since
commit f5cc545f5969 ("thunderbolt: Wait for tb_domain_release() to
complete when driver is removed") instead of just leaking memory this
now also blocks in the completion wait forever when unbinding the
driver.
This can be triggered whenever a DP tunnel is torn down before the DPRX
read has completed, e.g. by unplugging within the timeout, and then
unbinding the driver.
That reference only exists to keep the domain around while the DPRX work
is scheduled so let the work itself own it: take it in tb_dp_dprx_start()
and drop it in both places that end the work. Get/put are then paired
inside the same file and it doesn't matter anymore if the callback ever
runs.
Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
tb_tunnel_alloc_dp() takes an optional callback which is run from
dprx_work once the DPRX capabilities read has completed. Without that
callback tb_dp_dprx_start() reads the capabilities synchronously and
never queues the work. It however always takes a tunnel reference which
is only dropped by dprx_work itself or by tb_dp_dprx_stop() when
cancel_delayed_work() actually canceled that work. That reference is
thus leaked for every tunnel without a callback.
The only tunnels without one are those from tb_tunnel_discover_dp(),
which are activated again when restoring from hibernation.
Pass the callback to tb_tunnel_discover_dp() as well and drop the
synchronous path such that the DPRX capabilities are always read from
dprx_work. Hibernation restore then also no longer blocks for up to 12
seconds while waiting for that read to complete.
Also fix up the KUnit tests.
Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
tb_stop() drops the reference to all DP tunnels but does not deactivate
them, thus nothing cancels a dprx_work still in flight (which holds its
own tunnel reference) and the tunnel can outlive tb_switch_remove(). The
HopID releases in tb_path_free() then operate on freed IDAs and trigger
warnings like
ida_free called for id=8 which is not allocated.
This can be triggered by unbinding the driver while a DP tunnel is still
waiting for the DPRX capabilities read to finish. On the Apple NHI
unplugging the cable runs into just that reliably because the read can
never finish right now and because the unplug powers down the entire
USB4 complex and removes the NHI device.
Take a router reference whenever an input or output HopID is allocated
and drop it again after the HopID is released. This keeps the ports and
their HopID IDAs alive for as long as they are used.
The KUnit tests allocate routers without ever registering their devices
so initialize the embedded struct device there as well and drop its
initial reference when the test is finished.
Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Mika writes:
thunderbolt: Changes for v7.3 merge window
This includes following USB4/Thunderbolt changes for the v7.3 merge
window:
- Assert Downstream Port Reset for Thunderbolt 3 devices during
shutdown to avoid unnecessary delays over warm reset.
- Tidy up Thunderbolt service ->probe callbacks.
- USB4STREAM improvements.
- AMD host interface quirk to fix Tx ring hang on teardown of a DMA tunnel.
- Minor fixes and cleanups.
All these have been in linux-next with no reported issues.
* tag 'thunderbolt-for-v7.3-rc1' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt:
thunderbolt: Clamp DMA tunnel credits to what a hop register can hold
thunderbolt: Use min() for the DMA path credit cap
thunderbolt: debugfs: Replace get_zeroed_page() with kzalloc()
thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers
thunderbolt: stream: Add support for busy polling
thunderbolt: Make interrupt optional for rings
thunderbolt: stream: Support IOCB_NOWAIT in non-blocking I/O as well
thunderbolt: stream: Fix possible short reads/writes
thunderbolt: stream: Restore consumer if copying from iter fails
thunderbolt: Remove redundant dev_err_probe()
docs: admin-guide: thunderbolt: Fix sentence structure
thunderbolt: xdomain: Notify peers after enumeration
thunderbolt: Drop comma after device id array terminator
thunderbolt: Assert that a service driver has a probe callback
thunderbolt: Stop passing matched device ID to .probe()
thunderbolt: Assert downstream port reset on shutdown
|
|
struct tb_regs_hop::initial_credits is 7 bits wide, but neither of the
values tb_tunnel_alloc_dma() picks from is bounded by that: the
dma_credits module parameter has no upper limit, and neither does the
host router's baMaxHI. A larger count survives until tb_path_activate()
copies it into the register and keeps the low bits, leaving the path on
a credit count nobody asked for.
Clamp it in tb_tunnel_alloc_dma(), the only entry point for DMA tunnels;
every step below it can only lower the value further. Carry the count in
an unsigned int while at it.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Fan Ye <fy15309206903@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
tb_dma_reserve_credits() caps the request against what the adapter has
left by decrementing one credit at a time. The other arm of the same
if() already caps with min(port->total_credits, credits); use min()
here too.
No functional change: the object code is unchanged.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Fan Ye <fy15309206903@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
validate_and_copy_from_user() allocates a page to store data from
userspace via get_zeroed_page(), and then returns it as a buffer.
Neither the function itself nor its callers require struct page access.
This buffer can easily be allocated with kzalloc() as there is nothing
special about it that requires going through the page allocator.
kzalloc(), which internally reduces to kmalloc() with __GFP_ZERO,
provides a better API and kfree() does not need to know the size of the
freed object. Additionally it removes the casts of (void *) and
(unsigned long) which only obfuscate the code.
Replace get_zeroed_page() with kzalloc() and free_page() with kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com/
Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Some AMD USB4 host routers have a bug in the Host Interface where
DMA path setup and teardown cycles may cause the Tx ring to hang.
Fix this by issuing a Host Interface Reset on every DMA path teardown
for affected routers. The Host Interface Reset brings the registers in
the memory BAR to their default state and clears the End-to-End Flow
Control state, preventing the hang condition.
Co-developed-by: Sanath S <Sanath.S@amd.com>
Signed-off-by: Sanath S <Sanath.S@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Using interrupts and scheduling workers increase latency so latency
critical applications may want to avoid that. Make this possible in
USB4STREAM by adding a new ConfigFS attribute: busy_poll that, when
activated switches the rings to polling mode. The cost for lower latency
is that this burns more CPU cycles and things like poll(2) cannot be
used.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
For some use-cases it does make sense to poll the rings directly instead
of relying on the interrupt. For this reason add a new flag RING_FLAG_NO_INTERRUPT
that can be used to allocate ring in polled mode.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
For read_iter/write_iter() it is also possible to pass IOCB_NOWAIT with
the kiocb to indicate non-blocking read/write. For instance io_uring
does this. So take this into account on read and write paths.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Since copy_page_{to|from}_iter() advances the iterator and makes
iov_iter_count() reflect the remaining bytes, subtracting nbytes from it
makes it count it twice resulting in possible short reads/writes on a
read/write spanning multiple frames.
Fix this by using iov_iter_count() directly.
Fixes: 6db21d817b43 ("thunderbolt: Add support for USB4STREAM")
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
In tbstream_dev_alloc_tx() if copying data from iterator fails we leave
the consumer pointer as is wasting one entry in the ring. Fix this by
restoring the consumer back in case of failure.
Fixes: 6db21d817b43 ("thunderbolt: Add support for USB4STREAM")
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Both Woody and Marek reported following crash:
BUG: unable to handle page fault for address: fffffffffffffff8
Call Trace:
<TASK>
device_release+0x43/0x90
kobject_cleanup+0x3c/0x180
icm_probe+0x19c/0x550 [thunderbolt]
nhi_probe+0x1a4/0x370 [thunderbolt]
local_pci_probe+0x41/0x90
pci_call_probe+0x5b/0x1a0
...
This only triggers on the error path when icm_probe() fails and the
domain structure is released, it tries to complete() uninitialized
completion.
Fix this by initializing the completion earlier.
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Closes: https://lore.kernel.org/linux-usb/amdezCBiW4fd_DuB@mail-itl/
Reported-by: Woody Suwalski <terraluna977@gmail.com>
Tested_by: Woody Suwalski <terraluna977@gmail.com>
Closes: https://lore.kernel.org/linux-usb/62caf7f8-b403-d0dd-15bc-b31b56f71c28@gmail.com/
Fixes: f5cc545f5969 ("thunderbolt: Wait for tb_domain_release() to complete when driver is removed")
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err_probe() calls.
Signed-off-by: Pan Chuang <panchuang@vivo.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
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: 9039387e166e ("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>
|
|
Service drivers may register local XDomain properties while discovery is
still in progress. This can cause the properties changed notification to be
sent before the peer is ready to act on it.
If the peer has already read the local property block before the service
was registered, it may keep using the old property generation and miss the
newly registered service. With ThunderboltIP this can leave the network
service half-discovered after a warm reboot and the login request
eventually times out.
Queue another properties changed notification after the XDomain reaches
ENUMERATED so the peer can re-read the final local properties.
Signed-off-by: Milo Chen <cmh79479@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
The usual style for other device id arrays doesn't have a comma after
the initializer.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
tb_service_probe() calls the driver's probe function unconditionally.
Check at driver register time that this callback is valid to prevent a
NULL pointer exception.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
No driver makes use of that parameter, so drop it and don't spend the
effort to determine the matching entry.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
On shutdown the connection manager tears down the router tree without
signalling connected devices. A Thunderbolt 3 device directly connected
to a USB4 host never receives a disconnect indication and during shutdown
this can cause polling the dead link for up to 60 seconds. On some
platforms this behavior leads to a warm reset instead of a shutdown due
to this timeout.
Fix this by asserting PORT_CS_19.DPR on each connected downstream port
before tearing down the router tree. This drives SBTX low (USB4 spec
section 6.9), causing the device to detect SBRX low and transition to
Uninitialized Unplugged state immediately.
Always do this on system shutdown/reboot by forcing host_reset in the
PCI ->shutdown callback. On plain driver unload only do it when the host
router was actually reset on load (host_reset=1), since in that case the
tunnels are not preserved across reload anyway; with host_reset=0 the
tunnels are kept alive across unload/reload so the links are left intact.
Restrict the reset to Thunderbolt 3 devices.
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Co-developed-by: Sanath S <Sanath.S@amd.com>
Signed-off-by: Sanath S <Sanath.S@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.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: cd22e73bdf5e ("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: 52a4490e89d7 ("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: 6db21d817b43 ("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: 10904df3f20c ("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 559c1e1e0134 ("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: 559c1e1e0134 ("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: 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>
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: 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>
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: 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>
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: 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>
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: 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>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
Greg Kroah-Hartman
Kurt Lieber
Mika Westerberg
Mario Limonciello
Daehyeon Ko
Kees Cook
Fedor Pchelkin
Sven Peter
Fan Ye
Mahad Ibrahim
Basavaraj Natikar
Pan Chuang
Xu Rao
Milo Chen
Uwe Kleine-König (The Capable Hub)
Bryam Vargas
Linus Torvalds
Michael Bommarito