linux-kernel - Re: [syzbot] [net?] WARNING in xfrm_state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aO9QkNNkZ1JLnnIl@secunet.com>
Date: Wed, 15 Oct 2025 09:43:12 +0200
From: Steffen Klassert <steffen.klassert@...unet.com>
To: Sabrina Dubroca <sd@...asysnail.net>
CC: syzbot <syzbot+5cd6299ede4d4f70987b@...kaller.appspotmail.com>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <herbert@...dor.apana.org.au>,
	<horms@...nel.org>, <kuba@...nel.org>, <linux-kernel@...r.kernel.org>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>,
	<syzkaller-bugs@...glegroups.com>
Subject: Re: [syzbot] [net?] WARNING in xfrm_state_migrate (2)

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@...kaller.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!