[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20250923051357.12206-1-duoming@zju.edu.cn>
Date: Tue, 23 Sep 2025 13:13:57 +0800
From: Duoming Zhou <duoming@....edu.cn>
To: linux-usb@...r.kernel.org
Cc: linux-kernel@...r.kernel.org,
mika.westerberg@...ux.intel.com,
YehezkelShB@...il.com,
westeri@...nel.org,
michael.jamet@...el.com,
andreas.noever@...il.com,
Duoming Zhou <duoming@....edu.cn>
Subject: [PATCH v2] thunderbolt: Fix use-after-free in tb_dp_dprx_work
The original code relies on cancel_delayed_work() in tb_dp_dprx_stop(),
which does not ensure that the delayed work item tunnel->dprx_work has
fully completed if it was already running. This leads to use-after-free
scenarios where tb_tunnel is deallocated by tb_tunnel_put(), while
tunnel->dprx_work remains active and attempts to dereference tb_tunnel
in tb_dp_dprx_work().
A typical race condition is illustrated below:
CPU 0 | CPU 1
tb_dp_tunnel_active() |
tb_deactivate_and_free_tunnel()| tb_dp_dprx_start()
tb_tunnel_deactivate() | queue_delayed_work()
tb_dp_activate() |
tb_dp_dprx_stop() | tb_dp_dprx_work() //delayed worker
cancel_delayed_work() |
tb_tunnel_put(tunnel); |
| tunnel = container_of(...); //UAF
| tunnel-> //UAF
Replacing cancel_delayed_work() with cancel_delayed_work_sync() is
not feasible as it would introduce a deadlock: both tb_dp_dprx_work()
and the cleanup path acquire tb->lock, and cancel_delayed_work_sync()
would wait indefinitely for the work item that cannot proceed.
Instead, implement proper reference counting:
- If cancel_delayed_work() returns true (work is pending), we release
the reference in the stop function.
- If it returns false (work is executing or already completed), the
reference is released in delayed work function itself.
This ensures the tb_tunnel remains valid during work item execution
while preventing memory leaks.
This bug was found by static analysis.
Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
Signed-off-by: Duoming Zhou <duoming@....edu.cn>
---
Changes in v2:
- Drop the local variable used to check the return value of cancel_delayed_work().
drivers/thunderbolt/tunnel.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index d52efe3f658c..8333fc7f3d55 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -1073,6 +1073,7 @@ static void tb_dp_dprx_work(struct work_struct *work)
if (tunnel->callback)
tunnel->callback(tunnel, tunnel->callback_data);
+ tb_tunnel_put(tunnel);
}
static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
@@ -1100,8 +1101,8 @@ static void tb_dp_dprx_stop(struct tb_tunnel *tunnel)
if (tunnel->dprx_started) {
tunnel->dprx_started = false;
tunnel->dprx_canceled = true;
- cancel_delayed_work(&tunnel->dprx_work);
- tb_tunnel_put(tunnel);
+ if (cancel_delayed_work(&tunnel->dprx_work))
+ tb_tunnel_put(tunnel);
}
}
--
2.34.1
Powered by blists - more mailing lists