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

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

[Hu EnZe]

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;
+
 			spin_unlock_bh(&xfrm_state_dev_gc_lock);
 			xfrm_dev_state_free(x);
+			xfrm_state_put(x);
 			spin_lock_bh(&xfrm_state_dev_gc_lock);
 			goto restart_gc;
 		}

base-commit: c12cbf56320fb633484ee0ca1fb7d68d6b64b213
-- 
2.55.0

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

[Steffen Klassert]

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