Re: [PATCH] net: tls: Use disable_delayed_work_sync() instead of cancel_delayed_work_sync() in tls_sw_cancel_work_tx()

[Simon Horman <horms@kernel.org>]

On Mon, Feb 16, 2026 at 06:51:50PM +0900, Hyunwoo Kim wrote:
> After cancel_delayed_work_sync() is called from tls_sk_proto_close(), 
> tx_work_handler() can still be scheduled from paths such as the 
> Delayed ACK handler or ksoftirqd.
> As a result, the tx_work_handler() worker may dereference a freed 
> TLS object.
> 
> To prevent this race condition, cancel_delayed_work_sync() is 
> replaced with disable_delayed_work_sync().
> 
> Fixes: f87e62d45e51 ("net/tls: remove close callback sock unlock/lock around TX work flush")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>

Hi Hyunwoo,

Thanks for your patch(es).

Some feedback on process from my side.
You can read more about that at
https://docs.kernel.org/process/maintainer-netdev.html

* I think it would be good to mention how this problem was found.

* As a bug fix for code present in the net tree, it should be targeted
  at that tree like this.

  Subject: [PATCH net]: ...

* Looking over git history, it seems that an appropriate prefix
  for patches for this code is 'tls: '

  Subject [PATCH net]: tls: ...

* Also, please try to make the subject a bit more succinct

> ---
>  net/tls/tls_sw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 9937d4c810f2..b1fa62de9dab 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -2533,7 +2533,7 @@ void tls_sw_cancel_work_tx(struct tls_context *tls_ctx)
>  
>  	set_bit(BIT_TX_CLOSING, &ctx->tx_bitmask);
>  	set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask);
> -	cancel_delayed_work_sync(&ctx->tx_work.work);
> +	disable_delayed_work_sync(&ctx->tx_work.work);
>  }
>  
>  void tls_sw_release_resources_tx(struct sock *sk)
> -- 
> 2.43.0
> ---
> 
> Dear,
> 
> The following is a simplified scenario illustrating how each race can occur. Since tls_sw_cancel_work_tx() does not hold lock_sock(), it can race with tls_write_space().
> ```
>           cpu0                                             cpu1
> 
> tls_sk_proto_close()
>   tls_sw_cancel_work_tx()
>                                                     tls_write_space()
>                                                       tls_sw_write_space()
>                                                       if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx->tx_bitmask))
>     set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask);
>     cancel_delayed_work_sync(&ctx->tx_work.work);
>                                                       schedule_delayed_work(&tx_ctx->tx_work.work, 0);
> ```

I think that the text above would be best included in the patch description.
At least for me it is fundamental to understanding the problem.

> 
> Best regards,
> Hyunwoo Kim
> 

I see three similar patches on the mailing list.
The comments above go for them too.
And It would probably be useful to just handle one at a time,
to allow for proper feedback. Or bundle them in a patchset.

-- 
pw-bot: changes-requested