Skip to content

Commit 67600cc

Browse files
stonezdmwesteri
authored andcommitted
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: d6d458d ("thunderbolt: Handle DisplayPort tunnel activation asynchronously") Cc: [email protected] Signed-off-by: Duoming Zhou <[email protected]> Signed-off-by: Mika Westerberg <[email protected]>
1 parent ea6bb47 commit 67600cc

File tree

1 file changed

+3
-2
lines changed

1 file changed

+3
-2
lines changed

drivers/thunderbolt/tunnel.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,6 +1079,7 @@ static void tb_dp_dprx_work(struct work_struct *work)
10791079

10801080
if (tunnel->callback)
10811081
tunnel->callback(tunnel, tunnel->callback_data);
1082+
tb_tunnel_put(tunnel);
10821083
}
10831084

10841085
static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
@@ -1106,8 +1107,8 @@ static void tb_dp_dprx_stop(struct tb_tunnel *tunnel)
11061107
if (tunnel->dprx_started) {
11071108
tunnel->dprx_started = false;
11081109
tunnel->dprx_canceled = true;
1109-
cancel_delayed_work(&tunnel->dprx_work);
1110-
tb_tunnel_put(tunnel);
1110+
if (cancel_delayed_work(&tunnel->dprx_work))
1111+
tb_tunnel_put(tunnel);
11111112
}
11121113
}
11131114

0 commit comments

Comments
 (0)