Re: [PATCH 1/5] thunderbolt: Fix tunnel reference leak when the DPRX work is not started

Mika Westerberg <[email protected]>
Newsgroups org.kernel.vger.linux-usb,dev.linux.lists.asahi,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
Hi,

On Mon, Aug 17, 2026 at 09:53:58PM +0200, Sven Peter wrote:
> tb_dp_dprx_start always takes a tunnel reference which is only dropped
> by dprx_work eventually. Tunnels that have no callback don't ever queue
> that work and tb_dp_dprx_stop then has nothing to cancel. It however only
> releases the reference if cancel_delayed_work returned true and the
> reference is leaked then.

Okay but we always actually pass that callback there so I guess you are
hitting this because you have modified the caller in tb.c not to pass the
callback, right? If that's the case then I suggest mention how you actually
reproduced this whole issue.

I'm thinking we should make the callback mandatory instead as we always
need it for DP tunnels anyway. It should work the same also in Apple
silicon (one you have the DP tunneling in place).

> Fix this by only taking the reference when dprx_work is actually queued.
> 
> Fixes: d6d458d42e1e ("thunderbolt: Handle DisplayPort tunnel activation asynchronously")
> Cc: [email protected]
> Signed-off-by: Sven Peter <[email protected]>
> ---
> I didn't actually hit this on hardware but found it while fixing a domain
> leak in the same area and that fix depends on this one.
> ---
>  drivers/thunderbolt/tunnel.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
> index b7f32305f14a..50580ebdac4b 100644
> --- a/drivers/thunderbolt/tunnel.c
> +++ b/drivers/thunderbolt/tunnel.c
> @@ -1113,15 +1113,14 @@ static void tb_dp_dprx_work(struct work_struct *work)
>  
>  static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
>  {
> -	/*
> -	 * Bump up the reference to keep the tunnel around. It will be
> -	 * dropped in tb_dp_dprx_stop() once the tunnel is deactivated.
> -	 */
> -	tb_tunnel_get(tunnel);
> -
> -	tunnel->dprx_started = true;
> -
>  	if (tunnel->callback) {
> +		/*
> +		 * Bump up the reference to keep the tunnel around until the
> +		 * work has run or has been canceled.
> +		 */
> +		tb_tunnel_get(tunnel);
> +
> +		tunnel->dprx_started = true;
>  		tunnel->dprx_timeout = dprx_timeout_to_ktime(dprx_timeout);
>  		queue_delayed_work(tunnel->tb->wq, &tunnel->dprx_work, 0);
>  		return -EINPROGRESS;
> 
> -- 
> 2.55.0
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.