[PATCHSET IPSec 0/3] net: key: Fix address family validation and integer overflow in set_ipsecrequest

[PATCHSET IPSec 0/3] net: key: Fix address family validation and integer overflow in set_ipsecrequest

[clingfei]

From: Cheng Lingfei <clf700383@gmail.com>

Hi,

This patchset addresses a security issue in the PF_KEYv2 implementation where
improper address family validation could lead to integer overflows and buffer
calculation errors in the set_ipsecrequest() function.

The core problem stems from two interrelated issues:

1. The `family` parameter in set_ipsecrequest() is declared as u8 but receives
   a 16-bit value, causing truncation of the upper byte.

2. pfkey_sockaddr_len() returns 0 for unsupported address families, but the
   calling code doesn't properly validate this return value before using it in
   size calculations, leading to potential integer overflows.

The patchset is structured as follows:

Patch 1/3: Corrects the type of the family argument from u8 to u16 to prevent
           truncation of 16-bit address family values.

Patch 2/3: Adds proper validation for the return value of pfkey_sockaddr_len()
           to catch unsupported address families early.

Patch 3/3: Enhances the error handling to ensure zero-length allocations are
           properly rejected and adds appropriate error returns.

This series fixes the original issue introduced in:
Fixes: 14ad6ed30a10 ("net: allow small head cache usage with large MAX_SKB_FRAGS values")

This coordinated approach addresses all aspects of the problem discussed in
the recent thread[1, 2] and provides a comprehensive fix for the IPsec subsystem.

[1] https://lore.kernel.org/all/aP_X8sFJKWVycTn0@horms.kernel.org/
[2] https://lore.kernel.org/all/20251027205955.GA4074718@horms.kernel.org/

Thanks to all contributors who identified different facets of this issue.

Best regards.

Edward Adam Davis (1):
  key: No support for family zero

SHAURYA RANE (1):
  net: key: Validate address family in set_ipsecrequest()

clingfei (1):
  fix integer overflow in set_ipsecrequest

 net/key/af_key.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/3] fix integer overflow in set_ipsecrequest

[clingfei]

From: clingfei <clf700383@gmail.com>

syzbot reported a kernel BUG in set_ipsecrequest() due to an skb_over_panic.

The mp->new_family and mp->old_family is u16, while set_ipsecrequest receives
family as uint8_t,  causing a integer overflow and the later size_req calculation
error, which exceeds the size used in alloc_skb, and ultimately triggered the
kernel bug in skb_put.

Reported-by: syzbot+be97dd4da14ae88b6ba4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=be97dd4da14ae88b6ba4
Signed-off-by: Cheng Lingfei <clf700383@gmail.com>
---
 net/key/af_key.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 2ebde0352245..aa4bd29f27ea 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3518,7 +3518,7 @@ static int set_sadb_kmaddress(struct sk_buff *skb, const struct xfrm_kmaddress *
 
 static int set_ipsecrequest(struct sk_buff *skb,
 			    uint8_t proto, uint8_t mode, int level,
-			    uint32_t reqid, uint8_t family,
+			    u32 reqid, uint16_t family,
 			    const xfrm_address_t *src, const xfrm_address_t *dst)
 {
 	struct sadb_x_ipsecrequest *rq;
-- 
2.34.1

[PATCH 2/3] key: No support for family zero

[clingfei]

From: Edward Adam Davis <eadavis@qq.com>

When setting the extended skb data for sadb_x_ipsecrequest, the requested
extended data size exceeds the allocated skb data length, triggering the
reported bug.

Because family only supports AF_INET and AF_INET6, other values will cause
pfkey_sockaddr_fill() to fail, which in turn causes set_ipsecrequest() to
fail.

Therefore, a workaround is available here: using a family value of 0 to
resolve the issue of excessively large extended data length.

syzbot reported:
kernel BUG at net/core/skbuff.c:212!
Call Trace:
 skb_over_panic net/core/skbuff.c:217 [inline]
 skb_put+0x159/0x210 net/core/skbuff.c:2583
 skb_put_zero include/linux/skbuff.h:2788 [inline]
 set_ipsecrequest+0x73/0x680 net/key/af_key.c:3532

Fixes: 08de61beab8a ("[PFKEYV2]: Extension for dynamic update of endpoint address(es)")
Reported-by: syzbot+be97dd4da14ae88b6ba4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=be97dd4da14ae88b6ba4
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 net/key/af_key.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index aa4bd29f27ea..cfda15a5aa4d 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3526,6 +3526,9 @@ static int set_ipsecrequest(struct sk_buff *skb,
 	int socklen = pfkey_sockaddr_len(family);
 	int size_req;
 
+	if (!family)
+		return -EINVAL;
+
 	size_req = sizeof(struct sadb_x_ipsecrequest) +
 		   pfkey_sockaddr_pair_size(family);
 
-- 
2.34.1

[PATCH 3/3] net: key: Validate address family in set_ipsecrequest()

[clingfei]

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

Re: [syzbot] [net?] kernel BUG in set_ipsecrequest

[syzbot]

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.

Re: [PATCHSET IPSec 0/3] net: key: Fix address family validation and integer overflow in set_ipsecrequest

[Sabrina Dubroca]

2025-11-06, 21:56:55 +0800, clingfei wrote:
> From: Cheng Lingfei <clf700383@gmail.com>
> 
> Hi,
> 
> This patchset addresses a security issue in the PF_KEYv2 implementation where
> improper address family validation could lead to integer overflows and buffer
> calculation errors in the set_ipsecrequest() function.
> 
> The core problem stems from two interrelated issues:
> 
> 1. The `family` parameter in set_ipsecrequest() is declared as u8 but receives
>    a 16-bit value, causing truncation of the upper byte.
> 
> 2. pfkey_sockaddr_len() returns 0 for unsupported address families, but the
>    calling code doesn't properly validate this return value before using it in
>    size calculations, leading to potential integer overflows.
> 
> The patchset is structured as follows:
> 
> Patch 1/3: Corrects the type of the family argument from u8 to u16 to prevent
>            truncation of 16-bit address family values.
> 
> Patch 2/3: Adds proper validation for the return value of pfkey_sockaddr_len()
>            to catch unsupported address families early.
> 
> Patch 3/3: Enhances the error handling to ensure zero-length allocations are
>            properly rejected and adds appropriate error returns.
> 
> This series fixes the original issue introduced in:
> Fixes: 14ad6ed30a10 ("net: allow small head cache usage with large MAX_SKB_FRAGS values")

This doesn't seem right. It looks more like a mismatch between the
size computation done before allocating the skb and the space actually
needed, and commit 14ad6ed30a10 made the pre-existing bug more visible.

-- 
Sabrina

Re: [PATCH 3/3] net: key: Validate address family in set_ipsecrequest()

[Sabrina Dubroca]

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

Re: [PATCH 3/3] net: key: Validate address family in set_ipsecrequest()

[clingfei]

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.