Re: [syzbot] [net?] WARNING in xfrm_state_migrate (2)

[Sabrina Dubroca <sd@queasysnail.net>]

2025-10-05, 10:39:46 -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    4b946f6bb7d6 selftests/bpf: Fix realloc size in bpf_get_ad..
> git tree:       bpf
> console output: https://syzkaller.appspot.com/x/log.txt?x=13be46e2580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=8f1ac8502efee0ee
> dashboard link: https://syzkaller.appspot.com/bug?extid=5cd6299ede4d4f70987b
> compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
> 
> Unfortunately, I don't have any reproducer for this issue yet.
> 
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/f0ef71bdead6/disk-4b946f6b.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/0c8251d5df12/vmlinux-4b946f6b.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/29bad3cdad16/bzImage-4b946f6b.xz
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+5cd6299ede4d4f70987b@syzkaller.appspotmail.com
> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 30386 at net/xfrm/xfrm_state.c:800 __xfrm_state_destroy net/xfrm/xfrm_state.c:800 [inline]
> WARNING: CPU: 0 PID: 30386 at net/xfrm/xfrm_state.c:800 xfrm_state_put include/net/xfrm.h:928 [inline]
> WARNING: CPU: 0 PID: 30386 at net/xfrm/xfrm_state.c:800 xfrm_state_migrate+0x13bc/0x1b10 net/xfrm/xfrm_state.c:2165

Steffen, this looks like we simply forgot to set XFRM_STATE_DEAD
before the final put() in the error path of xfrm_state_migrate (and
xfrm_state_clone_and_setup):


diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 9ea1d45b79e3..7ae10fac7b31 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2074,6 +2074,7 @@ static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
 	return x;
 
  error:
+	x->km.state = XFRM_STATE_DEAD;
 	xfrm_state_put(x);
 out:
 	return NULL;
@@ -2163,6 +2164,7 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 
 	return xc;
 error:
+	xc->km.state = XFRM_STATE_DEAD;
 	xfrm_state_put(xc);
 	return NULL;
 }


Does that look reasonable? The state was never add()/insert()'ed, so
it goes through put()/destroy() without delete() first that would set
XFRM_STATE_DEAD.


It also looks like we're missing a xfrm_dev_state_delete if
xfrm_state_migrate -> xfrm_state_add fails, since
xfrm_dev_state_delete gets called during __xfrm_state_delete, and this
new state will only see xfrm_state_put/__xfrm_state_destroy:

@@ -2159,10 +2159,13 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 		xfrm_state_insert(xc);
 	} else {
 		if (xfrm_state_add(xc) < 0)
-			goto error;
+			goto error_add;
 	}
 
 	return xc;
+error_add:
+	if (xuo)
+		xfrm_dev_state_delete(xc);
 error:
 	xc->km.state = XFRM_STATE_DEAD;
 	xfrm_state_put(xc);


-- 
Sabrina