Re: [PATCH] xfrm: protect __xfrm_state_delete against double-unhash of byseq/byspi

Re: [PATCH] xfrm: protect __xfrm_state_delete against double-unhash of byseq/byspi

[Michal Kosiorek]

v2 addresses your two points:
  - Added Fixes: tags (fe9f1d8779cb for byseq, 7b4dc3600e48 for byspi).
  - Rebased on ipsec.git master (HEAD fa90a3145c03), `git apply --check`
    clean. Same hunk applies to torvalds/master without changes.

Also added `Cc: stable@vger.kernel.org` per stable-kernel-rules.html
Option 1, since the Fixes: tags otherwise leave AUTOSEL doing the
disambiguation work alone.

Patch follows.

---

KASAN reproduces a slab-use-after-free in __xfrm_state_delete()'s
hlist_del_rcu calls under syzkaller load on linux-6.12.y stable
(reproduced on 6.12.47, also reachable via the same code path on
torvalds/master and on the ipsec tree). Nine unique signatures cluster
in the xfrm_state lifecycle, the load-bearing one being:

  BUG: KASAN: slab-use-after-free in __hlist_del
include/linux/list.h:990 [inline]
  BUG: KASAN: slab-use-after-free in hlist_del_rcu
include/linux/rculist.h:516 [inline]
  BUG: KASAN: slab-use-after-free in __xfrm_state_delete net/xfrm/xfrm_state.c
  Write of size 8 at addr ffff8881198bcb70 by task kworker/u8:9/435

  Workqueue: netns cleanup_net
  Call Trace:
   __hlist_del / hlist_del_rcu
   __xfrm_state_delete
   xfrm_state_delete
   xfrm_state_flush
   xfrm_state_fini
   ops_exit_list
   cleanup_net

The other observed signatures hit the same slab object from
__xfrm_state_lookup, xfrm_alloc_spi, __xfrm_state_insert and an OOB
write variant of __xfrm_state_delete, all on the byseq/byspi
hash chains.

__xfrm_state_delete() guards its byseq and byspi unhashes with
value-based predicates:

if (x->km.seq)
hlist_del_rcu(&x->byseq);
if (x->id.spi)
hlist_del_rcu(&x->byspi);

while everywhere else in the file (e.g. state_cache, state_cache_input)
the safer hlist_unhashed() check is used. xfrm_alloc_spi() sets
x->id.spi = newspi inside xfrm_state_lock and then immediately inserts
into byspi, but a path that observes x->id.spi != 0 outside of
xfrm_state_lock can still skip-or-hit the byspi unhash inconsistently
with whether x is actually on the list. The same holds for x->km.seq
versus byseq, and the bydst/bysrc unhashes have no predicate at all,
so a second __xfrm_state_delete() on the same object writes through
LIST_POISON pprev.

The defensive change here:

  - Use hlist_del_init_rcu() instead of hlist_del_rcu() on bydst,
    bysrc, byseq and byspi so a second deletion is a no-op rather
    than a write through LIST_POISON pprev. The byseq/byspi nodes
    are already initialised in xfrm_state_alloc().
  - Test hlist_unhashed() rather than the value predicate for
    byseq/byspi, so the unhash decision tracks list state rather than
    mutable scalar fields.

Empirical verification: applied this patch on top of v6.12.47, rebuilt,
and re-ran the same syzkaller harness for 1h16m on a previously-crashy
configuration that produced ~100 hits each of slab-use-after-free
Read in xfrm_alloc_spi / Read in __xfrm_state_lookup / Write in
__xfrm_state_delete. After the patch, 7.1M execs across 32 VMs at
~1550 exec/sec produced zero xfrm_state UAF/OOB hits. /proc/slabinfo
confirms the xfrm_state slab is actively allocated and freed during
the run (~143 KiB resident), so the fuzzer is still exercising those
code paths -- they just no longer crash.

Reproduction:

  - Linux 6.12.47 x86_64 + KASAN_GENERIC + KASAN_INLINE + KCOV
  - syzkaller @ 746545b8b1e4c3a128db8652b340d3df90ce61db
  - 32 QEMU/KVM VMs x 2 vCPU on AWS c5.metal bare metal
  - 9 unique signatures collected in ~9h, all within xfrm_state
    lifecycle

Fixes: fe9f1d8779cb ("xfrm: add state hashtable keyed by seq")
Fixes: 7b4dc3600e48 ("[XFRM]: Do not add a state whose SPI is zero to
the SPI hash.")
Reported-by: Michal Kosiorek <mkosiorek121@gmail.com>
Tested-by: Michal Kosiorek <mkosiorek121@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Michal Kosiorek <mkosiorek121@gmail.com>
---
 net/xfrm/xfrm_state.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 1748d374abca..686014d39429 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -818,17 +818,17 @@ int __xfrm_state_delete(struct xfrm_state *x)

  spin_lock(&net->xfrm.xfrm_state_lock);
  list_del(&x->km.all);
- hlist_del_rcu(&x->bydst);
- hlist_del_rcu(&x->bysrc);
- if (x->km.seq)
- hlist_del_rcu(&x->byseq);
+ hlist_del_init_rcu(&x->bydst);
+ hlist_del_init_rcu(&x->bysrc);
+ if (!hlist_unhashed(&x->byseq))
+ hlist_del_init_rcu(&x->byseq);
  if (!hlist_unhashed(&x->state_cache))
  hlist_del_rcu(&x->state_cache);
  if (!hlist_unhashed(&x->state_cache_input))
  hlist_del_rcu(&x->state_cache_input);

- if (x->id.spi)
- hlist_del_rcu(&x->byspi);
+ if (!hlist_unhashed(&x->byspi))
+ hlist_del_init_rcu(&x->byspi);
  net->xfrm.state_num--;
  xfrm_nat_keepalive_state_updated(x);
  spin_unlock(&net->xfrm.xfrm_state_lock);
-- 
2.54.0


śr., 29 kwi 2026 o 09:08 Steffen Klassert
<steffen.klassert@secunet.com> napisał(a):
>
> On Tue, Apr 28, 2026 at 09:53:45AM +0200, Michal Kosiorek wrote:
> ...
> >
> > Reported-by: Michal Kosiorek <mkosiorek121@gmail.com>
> > Tested-by: Michal Kosiorek <mkosiorek121@gmail.com>
> > Signed-off-by: Michal Kosiorek <mkosiorek121@gmail.com>
>
> Please add a 'Fixes:' tag so the patch can be backported
> to the stable trees.
>
> > ---
> >  net/xfrm/xfrm_state.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> > --- a/net/xfrm/xfrm_state.c
> > +++ b/net/xfrm/xfrm_state.c
> > @@ -758,16 +758,16 @@ int __xfrm_state_delete(struct xfrm_state *x)
> >
> >   spin_lock(&net->xfrm.xfrm_state_lock);
> >   list_del(&x->km.all);
> > - hlist_del_rcu(&x->bydst);
> > - hlist_del_rcu(&x->bysrc);
> > - if (x->km.seq)
> > - hlist_del_rcu(&x->byseq);
> > + hlist_del_init_rcu(&x->bydst);
> > + hlist_del_init_rcu(&x->bysrc);
> > + if (!hlist_unhashed(&x->byseq))
> > + hlist_del_init_rcu(&x->byseq);
> >   if (!hlist_unhashed(&x->state_cache))
> >   hlist_del_rcu(&x->state_cache);
> >   if (!hlist_unhashed(&x->state_cache_input))
> >   hlist_del_rcu(&x->state_cache_input);
> >
> > - if (x->id.spi)
> > - hlist_del_rcu(&x->byspi);
> > + if (!hlist_unhashed(&x->byspi))
> > + hlist_del_init_rcu(&x->byspi);
> >   net->xfrm.state_num--;
> >   xfrm_nat_keepalive_state_updated(x);
> >   spin_unlock(&net->xfrm.xfrm_state_lock);
>
> This does not allpy to the ipsec tree. Please
> rebase on the ipsec tree and resend.
>
> Thanks!

Re: [PATCH] xfrm: protect __xfrm_state_delete against double-unhash of byseq/byspi

[Steffen Klassert]

On Wed, Apr 29, 2026 at 10:29:10AM +0200, Michal Kosiorek wrote:
...
> 
> Fixes: fe9f1d8779cb ("xfrm: add state hashtable keyed by seq")
> Fixes: 7b4dc3600e48 ("[XFRM]: Do not add a state whose SPI is zero to
> the SPI hash.")
> Reported-by: Michal Kosiorek <mkosiorek121@gmail.com>
> Tested-by: Michal Kosiorek <mkosiorek121@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Michal Kosiorek <mkosiorek121@gmail.com>
> ---
>  net/xfrm/xfrm_state.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 1748d374abca..686014d39429 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -818,17 +818,17 @@ int __xfrm_state_delete(struct xfrm_state *x)
> 
>   spin_lock(&net->xfrm.xfrm_state_lock);
>   list_del(&x->km.all);
> - hlist_del_rcu(&x->bydst);
> - hlist_del_rcu(&x->bysrc);
> - if (x->km.seq)
> - hlist_del_rcu(&x->byseq);
> + hlist_del_init_rcu(&x->bydst);
> + hlist_del_init_rcu(&x->bysrc);
> + if (!hlist_unhashed(&x->byseq))
> + hlist_del_init_rcu(&x->byseq);
>   if (!hlist_unhashed(&x->state_cache))
>   hlist_del_rcu(&x->state_cache);
>   if (!hlist_unhashed(&x->state_cache_input))
>   hlist_del_rcu(&x->state_cache_input);
> 
> - if (x->id.spi)
> - hlist_del_rcu(&x->byspi);
> + if (!hlist_unhashed(&x->byspi))
> + hlist_del_init_rcu(&x->byspi);
>   net->xfrm.state_num--;
>   xfrm_nat_keepalive_state_updated(x);
>   spin_unlock(&net->xfrm.xfrm_state_lock);

This looks still odd, the indentation seems to be wrong.
It does not apply, maybe your mail client malformed
the patch.

[PATCH ipsec v2] xfrm: defensively unhash xfrm_state lists in __xfrm_state_delete

[Michal Kosiorek]

KASAN reproduces a slab-use-after-free in __xfrm_state_delete()'s
hlist_del_rcu calls under syzkaller load on linux-6.12.y stable
(reproduced on 6.12.47, also reachable via the same code path on
torvalds/master and on the ipsec tree). Nine unique signatures cluster
in the xfrm_state lifecycle, the load-bearing one being:

  BUG: KASAN: slab-use-after-free in __hlist_del include/linux/list.h:990 [inline]
  BUG: KASAN: slab-use-after-free in hlist_del_rcu include/linux/rculist.h:516 [inline]
  BUG: KASAN: slab-use-after-free in __xfrm_state_delete net/xfrm/xfrm_state.c
  Write of size 8 at addr ffff8881198bcb70 by task kworker/u8:9/435

  Workqueue: netns cleanup_net
  Call Trace:
   __hlist_del / hlist_del_rcu
   __xfrm_state_delete
   xfrm_state_delete
   xfrm_state_flush
   xfrm_state_fini
   ops_exit_list
   cleanup_net

The other observed signatures hit the same slab object from
__xfrm_state_lookup, xfrm_alloc_spi, __xfrm_state_insert and an OOB
write variant of __xfrm_state_delete, all on the byseq/byspi
hash chains.

__xfrm_state_delete() guards its byseq and byspi unhashes with
value-based predicates:

	if (x->km.seq)
		hlist_del_rcu(&x->byseq);
	if (x->id.spi)
		hlist_del_rcu(&x->byspi);

while everywhere else in the file (e.g. state_cache, state_cache_input)
the safer hlist_unhashed() check is used. xfrm_alloc_spi() sets
x->id.spi = newspi inside xfrm_state_lock and then immediately inserts
into byspi, but a path that observes x->id.spi != 0 outside of
xfrm_state_lock can still skip-or-hit the byspi unhash inconsistently
with whether x is actually on the list. The same holds for x->km.seq
versus byseq, and the bydst/bysrc unhashes have no predicate at all,
so a second __xfrm_state_delete() on the same object writes through
LIST_POISON pprev.

The defensive change here:

  - Use hlist_del_init_rcu() instead of hlist_del_rcu() on bydst,
    bysrc, byseq and byspi so a second deletion is a no-op rather
    than a write through LIST_POISON pprev. The byseq/byspi nodes
    are already initialised in xfrm_state_alloc().
  - Test hlist_unhashed() rather than the value predicate for
    byseq/byspi, so the unhash decision tracks list state rather than
    mutable scalar fields.

Empirical verification: applied this patch on top of v6.12.47, rebuilt,
and re-ran the same syzkaller harness for 1h16m on a previously-crashy
configuration that produced ~100 hits each of slab-use-after-free
Read in xfrm_alloc_spi / Read in __xfrm_state_lookup / Write in
__xfrm_state_delete. After the patch, 7.1M execs across 32 VMs at
~1550 exec/sec produced zero xfrm_state UAF/OOB hits. /proc/slabinfo
confirms the xfrm_state slab is actively allocated and freed during
the run (~143 KiB resident), so the fuzzer is still exercising those
code paths -- they just no longer crash.

Reproduction:

  - Linux 6.12.47 x86_64 + KASAN_GENERIC + KASAN_INLINE + KCOV
  - syzkaller @ 746545b8b1e4c3a128db8652b340d3df90ce61db
  - 32 QEMU/KVM VMs x 2 vCPU on AWS c5.metal bare metal
  - 9 unique signatures collected in ~9h, all within xfrm_state
    lifecycle

Fixes: fe9f1d8779cb ("xfrm: add state hashtable keyed by seq")
Fixes: 7b4dc3600e48 ("[XFRM]: Do not add a state whose SPI is zero to the SPI hash.")
Reported-by: Michal Kosiorek <mkosiorek121@gmail.com>
Tested-by: Michal Kosiorek <mkosiorek121@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Michal Kosiorek <mkosiorek121@gmail.com>
---

Resending v2 via git send-email -- the previous post had been sent
through Gmail's web client which stripped all tabs from the diff
hunk and made the patch un-applyable. Apologies for the noise. No
other changes versus the prior v2 send.

 net/xfrm/xfrm_state.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 1748d374abca..686014d39429 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -818,17 +818,17 @@ int __xfrm_state_delete(struct xfrm_state *x)
 
 		spin_lock(&net->xfrm.xfrm_state_lock);
 		list_del(&x->km.all);
-		hlist_del_rcu(&x->bydst);
-		hlist_del_rcu(&x->bysrc);
-		if (x->km.seq)
-			hlist_del_rcu(&x->byseq);
+		hlist_del_init_rcu(&x->bydst);
+		hlist_del_init_rcu(&x->bysrc);
+		if (!hlist_unhashed(&x->byseq))
+			hlist_del_init_rcu(&x->byseq);
 		if (!hlist_unhashed(&x->state_cache))
 			hlist_del_rcu(&x->state_cache);
 		if (!hlist_unhashed(&x->state_cache_input))
 			hlist_del_rcu(&x->state_cache_input);
 
-		if (x->id.spi)
-			hlist_del_rcu(&x->byspi);
+		if (!hlist_unhashed(&x->byspi))
+			hlist_del_init_rcu(&x->byspi);
 		net->xfrm.state_num--;
 		xfrm_nat_keepalive_state_updated(x);
 		spin_unlock(&net->xfrm.xfrm_state_lock);
-- 
2.54.0

Re: [PATCH ipsec v2] xfrm: defensively unhash xfrm_state lists in __xfrm_state_delete

[Steffen Klassert]

On Wed, Apr 29, 2026 at 10:54:51AM +0200, Michal Kosiorek wrote:
...
> 
> Fixes: fe9f1d8779cb ("xfrm: add state hashtable keyed by seq")
> Fixes: 7b4dc3600e48 ("[XFRM]: Do not add a state whose SPI is zero to the SPI hash.")
> Reported-by: Michal Kosiorek <mkosiorek121@gmail.com>
> Tested-by: Michal Kosiorek <mkosiorek121@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Michal Kosiorek <mkosiorek121@gmail.com>

Applied, thanks Michal!