[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aO9vAIsmgNx4j1Sf@krikkit>
Date: Wed, 15 Oct 2025 11:53:04 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Steffen Klassert <steffen.klassert@...unet.com>
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)
2025-10-15, 09:43:12 +0200, Steffen Klassert wrote:
> 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!
Thanks for confirming. I'll post the patches (with a few other fixes I
have) today or tomorrow.
--
Sabrina
Powered by blists - more mailing lists