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

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

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.