Re: [PATCH 4/5] thunderbolt: Don't access a DP tunnel after its DPRX read was canceled

[Mika Westerberg <mika.westerberg@linux.intel.com>]

Hi,

On Mon, Aug 17, 2026 at 09:54:01PM +0200, Sven Peter wrote:
> 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 is being torn down and touches routers that may already be
> gone after an unplug.

Also here it would be good to describe how you reproduce 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>
> ---
>  drivers/thunderbolt/tb.c     | 12 ++++++++++++
>  drivers/thunderbolt/tunnel.c | 11 +++++++++--
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index fb9da53fe391..e368a6b53f64 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -1910,6 +1910,18 @@ static void tb_dp_tunnel_active(struct tb_tunnel *tunnel, void *data)
>  	struct tb *tb = data;
>  
>  	mutex_lock(&tb->lock);
> +
> +	/*
> +	 * If the DPRX read was canceled the tunnel is already being torn
> +	 * down by whoever canceled it. Do not touch the adapters here
> +	 * because the routers may be gone by now.
> +	 */
> +	if (tunnel->dprx_canceled) {
> +		tb_tunnel_dbg(tunnel, "DPRX read canceled, not activating\n");
> +		mutex_unlock(&tb->lock);
> +		return;
> +	}
> +
>  	if (tb_tunnel_is_active(tunnel)) {
>  		int consumed_up, consumed_down, ret;
>  
> diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
> index 82d9c0b556dd..52fa90786ff8 100644
> --- a/drivers/thunderbolt/tunnel.c
> +++ b/drivers/thunderbolt/tunnel.c
> @@ -1090,8 +1090,14 @@ static void tb_dp_dprx_work(struct work_struct *work)
>  	struct tb_tunnel *tunnel = container_of(work, typeof(*tunnel), dprx_work.work);
>  	struct tb *tb = tunnel->tb;
>  
> +	/*
> +	 * The DPRX read can be canceled while this work is waiting for
> +	 * tb->lock. Check the flag only once it is held: while the lock is
> +	 * held the tunnel cannot be torn down under us and the adapters are
> +	 * safe to access.
> +	 */
> +	mutex_lock(&tb->lock);
>  	if (!tunnel->dprx_canceled) {
> -		mutex_lock(&tb->lock);
>  		if (tb_dp_is_usb4(tunnel->src_port->sw) &&
>  		    tb_dp_wait_dprx(tunnel, TB_DPRX_WAIT_TIMEOUT)) {
>  			if (ktime_before(ktime_get(), tunnel->dprx_timeout)) {
> @@ -1103,8 +1109,8 @@ static void tb_dp_dprx_work(struct work_struct *work)
>  		} else {
>  			tb_tunnel_set_active(tunnel, true);
>  		}
> -		mutex_unlock(&tb->lock);
>  	}
> +	mutex_unlock(&tb->lock);
>  
>  	if (tunnel->callback)
>  		tunnel->callback(tunnel, tunnel->callback_data);
> @@ -1123,6 +1129,7 @@ static int tb_dp_dprx_start(struct tb_tunnel *tunnel)
>  		tb_domain_get(tunnel->tb);
>  
>  		tunnel->dprx_started = true;
> +		tunnel->dprx_canceled = false;
>  		tunnel->dprx_timeout = dprx_timeout_to_ktime(dprx_timeout);
>  		queue_delayed_work(tunnel->tb->wq, &tunnel->dprx_work, 0);
>  		return -EINPROGRESS;
> 
> -- 
> 2.55.0
>