Re: [PATCH net] xfrm: Fix work re-schedule after cancel in xfrm_nat_keepalive_net_fini()

[Sabrina Dubroca <sd@queasysnail.net>]

Please also CC the author, and maybe additional contributors, of the
patch that introduced the problem you're fixing.

2026-03-11, 03:16:29 +0900, Hyunwoo Kim wrote:
> After cancel_delayed_work_sync() is called from
> xfrm_nat_keepalive_net_fini(), xfrm_state_fini() flushes remaining
> states via __xfrm_state_delete(), which calls
> xfrm_nat_keepalive_state_updated() to re-schedule nat_keepalive_work.

Eyal, I'm wondering why __xfrm_state_delete() calls
xfrm_nat_keepalive_state_updated(). At this point the state has been
removed from the walk list so nat_keepalive_work() won't do
anything. Am I missing something?

> The following is a simple race scenario:
> 
>            cpu0                             cpu1
> 
> cleanup_net() [Round 1]
>   ops_undo_list()
>     xfrm_net_exit()
>       xfrm_nat_keepalive_net_fini()
>         cancel_delayed_work_sync(nat_keepalive_work);
>       xfrm_state_fini()
>         xfrm_state_flush()
>           xfrm_state_delete(x)
>             __xfrm_state_delete(x)
>               xfrm_nat_keepalive_state_updated(x)
>                 schedule_delayed_work(nat_keepalive_work);
>   rcu_barrier();
>   net_complete_free();
>   net_passive_dec(net);
>     llist_add(&net->defer_free_list, &defer_free_list);
> 
> cleanup_net() [Round 2]
>   rcu_barrier();
>   net_complete_free()
>     kmem_cache_free(net_cachep, net);
>                                      nat_keepalive_work()
>                                        // on freed net
> 
> To prevent this, cancel_delayed_work_sync() is replaced with
> disable_delayed_work_sync().
> 
> Fixes: f531d13bdfe3 ("xfrm: support sending NAT keepalives in ESP in UDP states")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  net/xfrm/xfrm_nat_keepalive.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/xfrm/xfrm_nat_keepalive.c b/net/xfrm/xfrm_nat_keepalive.c
> index ebf95d48e86c..1856beee0149 100644
> --- a/net/xfrm/xfrm_nat_keepalive.c
> +++ b/net/xfrm/xfrm_nat_keepalive.c
> @@ -261,7 +261,7 @@ int __net_init xfrm_nat_keepalive_net_init(struct net *net)
>  
>  int xfrm_nat_keepalive_net_fini(struct net *net)
>  {
> -	cancel_delayed_work_sync(&net->xfrm.nat_keepalive_work);
> +	disable_delayed_work_sync(&net->xfrm.nat_keepalive_work);
>  	return 0;
>  }
>  
> -- 
> 2.43.0
> 
> 

-- 
Sabrina