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

[Hyunwoo Kim <imv4bel@gmail.com>]

On Tue, Feb 17, 2026 at 03:37:01PM +0000, Simon Horman wrote:
> 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.

Hi Simon,

Thank you for the feedback.

This issue was found during a manual code audit. I will add this 
information to the commit message.


> 
> * 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.

Understood. I will add the race scenario description to the patch.

> 
> > 
> > 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.

Since the core of this bug pattern is espintcp, I will submit a revised 
espintcp v2 patch shortly. 
I would appreciate it if you could review the espintcp v2 patch. 
Once the espintcp review is done, I will apply the feedback to the 
remaining strparser and tls patches as well.

Best regards,
Hyunwoo Kim

> 
> -- 
> pw-bot: changes-requested