[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250923153105.GA2912318@black.igk.intel.com>
Date: Tue, 23 Sep 2025 17:31:05 +0200
From: Mika Westerberg <mika.westerberg@...ux.intel.com>
To: Duoming Zhou <duoming@....edu.cn>
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
YehezkelShB@...il.com, westeri@...nel.org, michael.jamet@...el.com,
andreas.noever@...il.com
Subject: Re: [PATCH v2] thunderbolt: Fix use-after-free in tb_dp_dprx_work
Hi,
On Tue, Sep 23, 2025 at 01:13:57PM +0800, Duoming Zhou wrote:
> 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>
Because it is pretty late in the cycle, I would like to get some exposure
for this one before it goes to stable trees. I applied this to my
thunderbolt.git/next now. Once it is pulled into mainline this will be
picked up by the stable machinery too.
Thanks!
Powered by blists - more mailing lists