From: SHAURYA RANE <ssrane_b23@ee.vjti.ac.in>

Hi syzbot,

Please test the following patch.

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master

Thanks,
Shaurya Rane

From 123c5ac9ba261681b58a6217409c94722fde4249 Mon Sep 17 00:00:00 2001
From: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
Date: Sun, 19 Oct 2025 23:18:30 +0530
Subject: [PATCH] net: key: Validate address family in set_ipsecrequest()

syzbot reported a kernel BUG in set_ipsecrequest() due to an
skb_over_panic when processing XFRM_MSG_MIGRATE messages.

The root cause is that set_ipsecrequest() does not validate the
address family parameter before using it to calculate buffer sizes.
When an unsupported family value (such as 0) is passed,
pfkey_sockaddr_len() returns 0, leading to incorrect size calculations.

In pfkey_send_migrate(), the buffer size is calculated based on
pfkey_sockaddr_pair_size(), which uses pfkey_sockaddr_len(). When
family=0, this returns 0, so only sizeof(struct sadb_x_ipsecrequest)
(16 bytes) is allocated per entry. However, set_ipsecrequest() is
called multiple times in a loop (once for old_family, once for
new_family, for each migration bundle), repeatedly calling skb_put_zero()
with 16 bytes each time.

This causes the tail pointer to exceed the end pointer of the skb,
triggering skb_over_panic:
  tail: 0x188 (392 bytes)
  end:  0x180 (384 bytes)

Fix this by validating that pfkey_sockaddr_len() returns a non-zero
value before proceeding with buffer operations. This ensures proper
size calculations and prevents buffer overflow. Checking socklen
instead of just family==0 provides comprehensive validation for all
unsupported address families.

Reported-by: syzbot+be97dd4da14ae88b6ba4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=be97dd4da14ae88b6ba4
Fixes: 08de61beab8a ("[PFKEYV2]: Extension for dynamic update of
endpoint address(es)")
Signed-off-by: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
---
 net/key/af_key.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index cfda15a5aa4d..93c20a31e03d 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3529,7 +3529,11 @@ static int set_ipsecrequest(struct sk_buff *skb,
 	if (!family)
 		return -EINVAL;
 
-	size_req = sizeof(struct sadb_x_ipsecrequest) +
+    /* Reject invalid/unsupported address families */
+    if (!socklen)
+        return -EINVAL;
+
+    size_req = sizeof(struct sadb_x_ipsecrequest) +
 		   pfkey_sockaddr_pair_size(family);
 
 	rq = skb_put_zero(skb, size_req);
-- 
2.34.1

note: There are a few issues with the format of this patch, and the
subject prefix should be "[PATCH ipsec n/3]" for all the patches in
the series. But I'm also not sure if this is the right way to fix this
syzbot report.


2025-11-06, 21:56:58 +0800, clingfei wrote:
> From: SHAURYA RANE <ssrane_b23@ee.vjti.ac.in>


From here:

> Hi syzbot,
> 
> Please test the following patch.
> 
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> 
> Thanks,
> Shaurya Rane
> 
> From 123c5ac9ba261681b58a6217409c94722fde4249 Mon Sep 17 00:00:00 2001
> From: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
> Date: Sun, 19 Oct 2025 23:18:30 +0530
> Subject: [PATCH] net: key: Validate address family in set_ipsecrequest()

to here should be removed.


> syzbot reported a kernel BUG in set_ipsecrequest() due to an
> skb_over_panic when processing XFRM_MSG_MIGRATE messages.
> 
> The root cause is that set_ipsecrequest() does not validate the
> address family parameter before using it to calculate buffer sizes.
> When an unsupported family value (such as 0) is passed,
> pfkey_sockaddr_len() returns 0, leading to incorrect size calculations.
> 
> In pfkey_send_migrate(), the buffer size is calculated based on
> pfkey_sockaddr_pair_size(), which uses pfkey_sockaddr_len(). When
> family=0, this returns 0, so only sizeof(struct sadb_x_ipsecrequest)
> (16 bytes) is allocated per entry. However, set_ipsecrequest() is
> called multiple times in a loop (once for old_family, once for
> new_family, for each migration bundle), repeatedly calling skb_put_zero()
> with 16 bytes each time.

So the root of the problem is a mismatch between allocation size and
the actual size needed. Unexpected families are not good, sure, but
would not cause a panic if the sizes were handled correctly.

OTOH, for this old code which is being deprecated, maybe it doesn't
matter to fix it "properly". (but see below)


> This causes the tail pointer to exceed the end pointer of the skb,
> triggering skb_over_panic:
>   tail: 0x188 (392 bytes)
>   end:  0x180 (384 bytes)
> 
> Fix this by validating that pfkey_sockaddr_len() returns a non-zero
> value before proceeding with buffer operations. This ensures proper
> size calculations and prevents buffer overflow. Checking socklen
> instead of just family==0 provides comprehensive validation for all
> unsupported address families.
> 
> Reported-by: syzbot+be97dd4da14ae88b6ba4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=be97dd4da14ae88b6ba4
> Fixes: 08de61beab8a ("[PFKEYV2]: Extension for dynamic update of
> endpoint address(es)")
> Signed-off-by: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
> ---
>  net/key/af_key.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index cfda15a5aa4d..93c20a31e03d 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -3529,7 +3529,11 @@ static int set_ipsecrequest(struct sk_buff *skb,
>  	if (!family)
>  		return -EINVAL;
>  
> -	size_req = sizeof(struct sadb_x_ipsecrequest) +
> +    /* Reject invalid/unsupported address families */

Steffen, AFAICT the whole migrate code has no family
validation. Shouldn't we check {old,new}_family to be one of
{AF_INET,AF_INET6} in xfrm_migrate_check? This should take care of the
problems that this series tries to address, and avoid having objects
installed in the kernel with unexpected families (which would match
what validate_tmpl does).


Looking quickly at xfrm_migrate_state_find, it also seems to compare
addresses without checking that both addresses are of the same
family. That seems a bit wrong, but changing the behavior of that old
code is maybe too risky.



> +    if (!socklen)
> +        return -EINVAL;
> +
> +    size_req = sizeof(struct sadb_x_ipsecrequest) +

nit: tabs should be used, not spaces

>  		   pfkey_sockaddr_pair_size(family);
>  
>  	rq = skb_put_zero(skb, size_req);

-- 
Sabrina

Sabrina Dubroca <sd@queasysnail.net> 于2025年11月7日周五 01:17写道：
>
> note: There are a few issues with the format of this patch, and the
> subject prefix should be "[PATCH ipsec n/3]" for all the patches in
> the series. But I'm also not sure if this is the right way to fix this
> syzbot report.
>
>
> 2025-11-06, 21:56:58 +0800, clingfei wrote:
> > From: SHAURYA RANE <ssrane_b23@ee.vjti.ac.in>
>
>
> From here:
>
> > Hi syzbot,
> >
> > Please test the following patch.
> >
> > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> >
> > Thanks,
> > Shaurya Rane
> >
> > From 123c5ac9ba261681b58a6217409c94722fde4249 Mon Sep 17 00:00:00 2001
> > From: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
> > Date: Sun, 19 Oct 2025 23:18:30 +0530
> > Subject: [PATCH] net: key: Validate address family in set_ipsecrequest()
>
> to here should be removed.
>

Sorry for the incorrect format of this patch. I will fix it in later patches.

>
> > syzbot reported a kernel BUG in set_ipsecrequest() due to an
> > skb_over_panic when processing XFRM_MSG_MIGRATE messages.
> >
> > The root cause is that set_ipsecrequest() does not validate the
> > address family parameter before using it to calculate buffer sizes.
> > When an unsupported family value (such as 0) is passed,
> > pfkey_sockaddr_len() returns 0, leading to incorrect size calculations.
> >
> > In pfkey_send_migrate(), the buffer size is calculated based on
> > pfkey_sockaddr_pair_size(), which uses pfkey_sockaddr_len(). When
> > family=0, this returns 0, so only sizeof(struct sadb_x_ipsecrequest)
> > (16 bytes) is allocated per entry. However, set_ipsecrequest() is
> > called multiple times in a loop (once for old_family, once for
> > new_family, for each migration bundle), repeatedly calling skb_put_zero()
> > with 16 bytes each time.
>
> So the root of the problem is a mismatch between allocation size and
> the actual size needed. Unexpected families are not good, sure, but
> would not cause a panic if the sizes were handled correctly.
>
> OTOH, for this old code which is being deprecated, maybe it doesn't
> matter to fix it "properly". (but see below)
>

I agree that the root cause of the problem is a mismatch between
 the allocation size and the actual size needed. I'm not familiar with the
kernel network stack, and I'm unsure if unexpected families might cause
other problems. But, regarding this specific issue, avoiding integer overflow
is sufficient to ensure consistency in size allocation and usage.

>
> > This causes the tail pointer to exceed the end pointer of the skb,
> > triggering skb_over_panic:
> >   tail: 0x188 (392 bytes)
> >   end:  0x180 (384 bytes)
> >
> > Fix this by validating that pfkey_sockaddr_len() returns a non-zero
> > value before proceeding with buffer operations. This ensures proper
> > size calculations and prevents buffer overflow. Checking socklen
> > instead of just family==0 provides comprehensive validation for all
> > unsupported address families.
> >
> > Reported-by: syzbot+be97dd4da14ae88b6ba4@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=be97dd4da14ae88b6ba4
> > Fixes: 08de61beab8a ("[PFKEYV2]: Extension for dynamic update of
> > endpoint address(es)")
> > Signed-off-by: Shaurya Rane <ssrane_b23@ee.vjti.ac.in>
> > ---
> >  net/key/af_key.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/key/af_key.c b/net/key/af_key.c
> > index cfda15a5aa4d..93c20a31e03d 100644
> > --- a/net/key/af_key.c
> > +++ b/net/key/af_key.c
> > @@ -3529,7 +3529,11 @@ static int set_ipsecrequest(struct sk_buff *skb,
> >       if (!family)
> >               return -EINVAL;
> >
> > -     size_req = sizeof(struct sadb_x_ipsecrequest) +
> > +    /* Reject invalid/unsupported address families */
>
> Steffen, AFAICT the whole migrate code has no family
> validation. Shouldn't we check {old,new}_family to be one of
> {AF_INET,AF_INET6} in xfrm_migrate_check? This should take care of the
> problems that this series tries to address, and avoid having objects
> installed in the kernel with unexpected families (which would match
> what validate_tmpl does).
>
>
> Looking quickly at xfrm_migrate_state_find, it also seems to compare
> addresses without checking that both addresses are of the same
> family. That seems a bit wrong, but changing the behavior of that old
> code is maybe too risky.
>
>
>
> > +    if (!socklen)
> > +        return -EINVAL;
> > +
> > +    size_req = sizeof(struct sadb_x_ipsecrequest) +
>
> nit: tabs should be used, not spaces
>
> >                  pfkey_sockaddr_pair_size(family);
> >
> >       rq = skb_put_zero(skb, size_req);
>
> --
> Sabrina

I think the check on socklen is trying to reject unexpected families,
but I am not sure if it is too late, and this check can only take
effect when the
type of family is handled successfully.

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+be97dd4da14ae88b6ba4@syzkaller.appspotmail.com
Tested-by: syzbot+be97dd4da14ae88b6ba4@syzkaller.appspotmail.com

Tested on:

commit:         b54a8e13 Merge branch 'bpf-indirect-jumps'
git tree:       bpf-next
console output: https://syzkaller.appspot.com/x/log.txt?x=10dae114580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=e46b8a1c645465a9
dashboard link: https://syzkaller.appspot.com/bug?extid=be97dd4da14ae88b6ba4
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
patch:          https://syzkaller.appspot.com/x/patch.diff?x=106ae114580000

Note: testing is done by a robot and is best-effort only.