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

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

On Tue, Oct 14, 2025 at 03:26:53PM +0200, Sabrina Dubroca wrote:
> 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.

Right. Looks like this is broken since the migrate API exists.

> 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);

This is correct as well. Thanks for catching these!