Re: [PATCH net v3] xfrm: hold state while flushing device GC list

[Steffen Klassert <steffen.klassert@secunet.com>]

On Sun, Aug 16, 2026 at 04:46:34PM +0800, Hu EnZe wrote:
> From: Hu EnZe <slamhu@tencent.com>
> 
> The device GC list does not hold a reference to the states it
> contains. During netdevice teardown, xfrm_dev_state_flush() drops
> xfrm_state_dev_gc_lock before calling xfrm_dev_state_free(), so the
> regular GC worker can concurrently destroy and free the entire
> xfrm_state. Subsequent access through the stale pointer can corrupt
> memory and cause a kernel panic.
> 
> Pin live states with xfrm_state_hold_rcu() while holding the device GC
> lock, and skip states whose reference count has already reached zero.
> Drop the acquired reference after the device state cleanup completes.
> Also use hlist_del_init() when removing dev_gclist so the node is
> marked as unhashed after removal.
> 
> Fixes: 07b87f9eea0c ("xfrm: Fix unregister netdevice hang on hardware offload.")
> Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> Assisted-by: Codex:GPT-5.6-Sol
> Signed-off-by: Hu EnZe <slamhu@tencent.com>
> ---
> v3:
>   - fix the outer email From name from "paulinsider" to "Hu EnZe"
>   - no code changes
> 
> v2: https://lore.kernel.org/netdev/20260809081151.56774-1-paulinsider@gmail.com/
>   - rebase onto the latest ipsec tree
>   - use Hu EnZe <slamhu@tencent.com> for authorship and Signed-off-by
>   - add the Assisted-by trailer for Codex
>   - clarify the netdevice teardown trigger in the commit message
>   - no code changes
> 
> v1: https://lore.kernel.org/netdev/20260801041340.92117-1-paulinsider@gmail.com/
> 
>  net/xfrm/xfrm_state.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 36a4f6793ede..01138d3d1067 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -785,7 +785,7 @@ void xfrm_dev_state_free(struct xfrm_state *x)
>  	if (dev && dev->xfrmdev_ops) {
>  		spin_lock_bh(&xfrm_state_dev_gc_lock);
>  		if (!hlist_unhashed(&x->dev_gclist))
> -			hlist_del(&x->dev_gclist);
> +			hlist_del_init(&x->dev_gclist);
>  		spin_unlock_bh(&xfrm_state_dev_gc_lock);
>  
>  		if (dev->xfrmdev_ops->xdo_dev_state_free)
> @@ -1006,8 +1006,13 @@ int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_vali
>  		xso = &x->xso;
>  
>  		if (xso->dev == dev) {
> +			/* The device GC list does not hold a reference to x. */
> +			if (!xfrm_state_hold_rcu(x))
> +				continue;

Can we avoid that additional refcount somehow?

>  			spin_unlock_bh(&xfrm_state_dev_gc_lock);
>  			xfrm_dev_state_free(x);

Maybe opencode the list removal here and create a
__xfrm_dev_state_free() function that does the rest.

> +			xfrm_state_put(x);
>  			spin_lock_bh(&xfrm_state_dev_gc_lock);
>  			goto restart_gc;
>  		}
> 
> base-commit: c12cbf56320fb633484ee0ca1fb7d68d6b64b213
> -- 
> 2.55.0