[PATCH v2] xfrm: fix TOCTOU race in per-CPU state cache add path

[PATCH v2] xfrm: fix TOCTOU race in per-CPU state cache add path

[Ashish Kunwar]

[-- Attachment #1.1: Type: text/plain, Size: 2283 bytes --]

xfrm_input_state_lookup() checks x->km.state == XFRM_STATE_VALID
without holding xfrm_state_lock, then acquires the lock and adds the
state to the per-CPU state_cache_input list. A concurrent
__xfrm_state_delete() on another CPU can set km.state to DEAD and
remove the state from the cache between the check and the lock
acquisition. The lookup then re-adds the now-DEAD state to the cache.

After GC frees the xfrm_state, the per-CPU cache holds a dangling
pointer. The next inbound packet that iterates the cache dereferences
freed slab memory.

The same pattern exists in xfrm_state_find() for the outbound
state_cache.

Fix both by re-checking km.state inside the lock before adding the
state to the cache. For xfrm_input_state_lookup, a DEAD state must
not be returned to the caller, so drop the refcount and return NULL.
For xfrm_state_find, fold the state check into the existing
conditional so the cache insertion is simply skipped.

Fixes: 81a331a0e72 ("xfrm: add an inbound state cache")
Signed-off-by: Ashish Kunwar <ashishkunwar280@gmail.com>
---
v2: Rebased, fixed outbound path to use proper conditional instead of
empty statement. Submitted to netdev per Steffen's feedback.
---
net/xfrm/xfrm_state.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 1748d37..d8a9cd1 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1230,6 +1230,12 @@ struct xfrm_state *xfrm_input_state_lookup(struct
net *net, u32 mark,

if (x && x->km.state == XFRM_STATE_VALID) {
spin_lock_bh(&net->xfrm.xfrm_state_lock);
+ if (x->km.state != XFRM_STATE_VALID) {
+ spin_unlock_bh(&net->xfrm.xfrm_state_lock);
+ xfrm_state_put(x);
+ x = NULL;
+ goto out;
+ }
if (hlist_unhashed(&x->state_cache_input)) {
hlist_add_head_rcu(&x->state_cache_input, state_cache_input);
} else {
@@ -1622,7 +1628,8 @@ out:
if (x && x->km.state == XFRM_STATE_VALID && !cached &&
(!(pol->flags & XFRM_POLICY_CPU_ACQUIRE) || x->pcpu_num == pcpu_id)) {
spin_lock_bh(&net->xfrm.xfrm_state_lock);
- if (hlist_unhashed(&x->state_cache))
+ if (x->km.state == XFRM_STATE_VALID &&
+ hlist_unhashed(&x->state_cache))
hlist_add_head_rcu(&x->state_cache, &pol->state_cache_list);
spin_unlock_bh(&net->xfrm.xfrm_state_lock);
}
-- 
2.50.1

[-- Attachment #1.2: Type: text/html, Size: 2786 bytes --]

[-- Attachment #2: v2-0001-xfrm-fix-TOCTOU-race-in-state-cache-add-path.patch --]
[-- Type: text/x-patch, Size: 2449 bytes --]

From 1f399093355a643f1f27bebea97539b09886592d Mon Sep 17 00:00:00 2001
From: Ashish Kunwar <ashishkunwar280@gmail.com>
Date: Mon, 27 Jul 2026 17:34:22 +0530
Subject: [PATCH] xfrm: fix TOCTOU race in per-CPU state cache add path

xfrm_input_state_lookup() checks x->km.state == XFRM_STATE_VALID
without holding xfrm_state_lock, then acquires the lock and adds the
state to the per-CPU state_cache_input list. A concurrent
__xfrm_state_delete() on another CPU can set km.state to DEAD and
remove the state from the cache between the check and the lock
acquisition. The lookup then re-adds the now-DEAD state to the cache.

After GC frees the xfrm_state, the per-CPU cache holds a dangling
pointer. The next inbound packet that iterates the cache dereferences
freed slab memory.

The same pattern exists in xfrm_state_find() for the outbound
state_cache.

Fix both by re-checking km.state inside the lock before adding the
state to the cache. For xfrm_input_state_lookup, a DEAD state must
not be returned to the caller, so drop the refcount and return NULL.
For xfrm_state_find, fold the state check into the existing
conditional so the cache insertion is simply skipped.

Fixes: 81a331a0e72 ("xfrm: add an inbound state cache")
Signed-off-by: Ashish Kunwar <ashishkunwar280@gmail.com>
---
 net/xfrm/xfrm_state.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 1748d37..d8a9cd1 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1230,6 +1230,12 @@ struct xfrm_state *xfrm_input_state_lookup(struct net *net, u32 mark,
 
 	if (x && x->km.state == XFRM_STATE_VALID) {
 		spin_lock_bh(&net->xfrm.xfrm_state_lock);
+		if (x->km.state != XFRM_STATE_VALID) {
+			spin_unlock_bh(&net->xfrm.xfrm_state_lock);
+			xfrm_state_put(x);
+			x = NULL;
+			goto out;
+		}
 		if (hlist_unhashed(&x->state_cache_input)) {
 			hlist_add_head_rcu(&x->state_cache_input, state_cache_input);
 		} else {
@@ -1622,7 +1628,8 @@ out:
 	if (x && x->km.state == XFRM_STATE_VALID && !cached &&
 	    (!(pol->flags & XFRM_POLICY_CPU_ACQUIRE) || x->pcpu_num == pcpu_id)) {
 		spin_lock_bh(&net->xfrm.xfrm_state_lock);
-		if (hlist_unhashed(&x->state_cache))
+		if (x->km.state == XFRM_STATE_VALID &&
+		    hlist_unhashed(&x->state_cache))
 			hlist_add_head_rcu(&x->state_cache, &pol->state_cache_list);
 		spin_unlock_bh(&net->xfrm.xfrm_state_lock);
 	}
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] xfrm: fix TOCTOU race in per-CPU state cache add path

[Steffen Klassert]

On Mon, Jul 27, 2026 at 05:46:21PM +0530, Ashish Kunwar wrote:
> xfrm_input_state_lookup() checks x->km.state == XFRM_STATE_VALID
> without holding xfrm_state_lock, then acquires the lock and adds the
> state to the per-CPU state_cache_input list. A concurrent
> __xfrm_state_delete() on another CPU can set km.state to DEAD and
> remove the state from the cache between the check and the lock
> acquisition. The lookup then re-adds the now-DEAD state to the cache.
> 
> After GC frees the xfrm_state, the per-CPU cache holds a dangling
> pointer. The next inbound packet that iterates the cache dereferences
> freed slab memory.
> 
> The same pattern exists in xfrm_state_find() for the outbound
> state_cache.
> 
> Fix both by re-checking km.state inside the lock before adding the
> state to the cache. For xfrm_input_state_lookup, a DEAD state must
> not be returned to the caller, so drop the refcount and return NULL.
> For xfrm_state_find, fold the state check into the existing
> conditional so the cache insertion is simply skipped.
> 
> Fixes: 81a331a0e72 ("xfrm: add an inbound state cache")
> Signed-off-by: Ashish Kunwar <ashishkunwar280@gmail.com>
> ---
> v2: Rebased, fixed outbound path to use proper conditional instead of
> empty statement. Submitted to netdev per Steffen's feedback.
> ---
> net/xfrm/xfrm_state.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 1748d37..d8a9cd1 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -1230,6 +1230,12 @@ struct xfrm_state *xfrm_input_state_lookup(struct
> net *net, u32 mark,
> 
> if (x && x->km.state == XFRM_STATE_VALID) {
> spin_lock_bh(&net->xfrm.xfrm_state_lock);
> + if (x->km.state != XFRM_STATE_VALID) {
> + spin_unlock_bh(&net->xfrm.xfrm_state_lock);
> + xfrm_state_put(x);
> + x = NULL;
> + goto out;
> + }
> if (hlist_unhashed(&x->state_cache_input)) {
> hlist_add_head_rcu(&x->state_cache_input, state_cache_input);
> } else {
> @@ -1622,7 +1628,8 @@ out:
> if (x && x->km.state == XFRM_STATE_VALID && !cached &&
> (!(pol->flags & XFRM_POLICY_CPU_ACQUIRE) || x->pcpu_num == pcpu_id)) {
> spin_lock_bh(&net->xfrm.xfrm_state_lock);
> - if (hlist_unhashed(&x->state_cache))
> + if (x->km.state == XFRM_STATE_VALID &&
> + hlist_unhashed(&x->state_cache))
> hlist_add_head_rcu(&x->state_cache, &pol->state_cache_list);
> spin_unlock_bh(&net->xfrm.xfrm_state_lock);
> }

Your mailer removed all indentations.