[PATCH bpf-next v3] bpf: reject short IPv4/IPv6 inputs in bpf_prog_test_run_skb

[PATCH bpf-next v3] bpf: reject short IPv4/IPv6 inputs in bpf_prog_test_run_skb

[Sun Jian]

bpf_prog_test_run_skb() calls eth_type_trans() first and then uses
skb->protocol to initialize sk family and address fields for the test
run.

For IPv4 and IPv6 packets, it may access ip_hdr(skb) or ipv6_hdr(skb)
to initialize sk fields. Reject the input earlier if the Ethernet frame
carries IPv4/IPv6 EtherType but the L3 header is too short.

Fold the IPv4/IPv6 header length checks into the existing protocol
switch and return -EINVAL before accessing the network headers.

Also extend empty_skb selftests with ETH_HLEN-sized packets carrying
IPv4/IPv6 EtherType but no L3 header.

Reported-by: syzbot+619b9ef527f510a57cfc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=619b9ef527f510a57cfc
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
Changes in v3:
- Rework the fix by moving the checks into the existing protocol switch
  in bpf_prog_test_run_skb() instead of duplicating the switch.
- Add empty_skb selftests for ETH_HLEN-sized packets with IPv4/IPv6
  EtherType but no L3 header.
- Retarget to bpf-next.

Link: https://lore.kernel.org/bpf/20260329161751.1914272-1-sun.jian.kdev@gmail.com/

 net/bpf/test_run.c                            | 20 ++++++++-----
 .../selftests/bpf/prog_tests/empty_skb.c      | 29 +++++++++++++++++++
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 178c4738e63b..300e2bfc5a62 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -1120,19 +1120,23 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 
 	switch (skb->protocol) {
 	case htons(ETH_P_IP):
-		sk->sk_family = AF_INET;
-		if (sizeof(struct iphdr) <= skb_headlen(skb)) {
-			sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
-			sk->sk_daddr = ip_hdr(skb)->daddr;
+		if (skb_headlen(skb) < sizeof(struct iphdr)) {
+			ret = -EINVAL;
+			goto out;
 		}
+		sk->sk_family = AF_INET;
+		sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
+		sk->sk_daddr = ip_hdr(skb)->daddr;
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case htons(ETH_P_IPV6):
-		sk->sk_family = AF_INET6;
-		if (sizeof(struct ipv6hdr) <= skb_headlen(skb)) {
-			sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
-			sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
+		if (skb_headlen(skb) < sizeof(struct ipv6hdr)) {
+			ret = -EINVAL;
+			goto out;
 		}
+		sk->sk_family = AF_INET6;
+		sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
+		sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
 		break;
 #endif
 	default:
diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
index 438583e1f2d1..d53567e9cd77 100644
--- a/tools/testing/selftests/bpf/prog_tests/empty_skb.c
+++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
@@ -12,6 +12,8 @@ void test_empty_skb(void)
 	struct bpf_program *prog;
 	char eth_hlen_pp[15];
 	char eth_hlen[14];
+	char ipv4_eth_hlen[14];
+	char ipv6_eth_hlen[14];
 	int veth_ifindex;
 	int ipip_ifindex;
 	int err;
@@ -46,6 +48,24 @@ void test_empty_skb(void)
 			.err = -EINVAL,
 		},
 
+		/* ETH_HLEN-sized packets with IPv4/IPv6 EtherType but
+		 * no L3 header are rejected.
+		 */
+		{
+			.msg = "veth short IPv4 ingress packet",
+			.data_in = ipv4_eth_hlen,
+			.data_size_in = sizeof(ipv4_eth_hlen),
+			.ifindex = &veth_ifindex,
+			.err = -EINVAL,
+		},
+		{
+			.msg = "veth short IPv6 ingress packet",
+			.data_in = ipv6_eth_hlen,
+			.data_size_in = sizeof(ipv6_eth_hlen),
+			.ifindex = &veth_ifindex,
+			.err = -EINVAL,
+		},
+
 		/* ETH_HLEN-sized packets:
 		 * - can not be redirected at LWT_XMIT
 		 * - can be redirected at TC to non-tunneling dest
@@ -108,6 +128,15 @@ void test_empty_skb(void)
 	SYS(out, "ip addr add 192.168.1.1/16 dev ipip0");
 	ipip_ifindex = if_nametoindex("ipip0");
 
+	memset(ipv4_eth_hlen, 0, sizeof(ipv4_eth_hlen));
+	memset(ipv6_eth_hlen, 0, sizeof(ipv6_eth_hlen));
+
+	ipv4_eth_hlen[12] = 0x08;
+	ipv4_eth_hlen[13] = 0x00;
+
+	ipv6_eth_hlen[12] = 0x86;
+	ipv6_eth_hlen[13] = 0xdd;
+
 	bpf_obj = empty_skb__open_and_load();
 	if (!ASSERT_OK_PTR(bpf_obj, "open skeleton"))
 		goto out;
-- 
2.43.0

Re: [PATCH bpf-next v3] bpf: reject short IPv4/IPv6 inputs in bpf_prog_test_run_skb

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 4cd6b3ea1815a..2bc04feadfabe 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -1137,19 +1137,23 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>
> 	switch (skb->protocol) {
> 	case htons(ETH_P_IP):
> -		sk->sk_family = AF_INET;
> -		if (sizeof(struct iphdr) <= skb_headlen(skb)) {
> -			sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
> -			sk->sk_daddr = ip_hdr(skb)->daddr;
> +		if (skb_headlen(skb) < sizeof(struct iphdr)) {
> +			ret = -EINVAL;
> +			goto out;
> 		}
> +		sk->sk_family = AF_INET;
> +		sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
> +		sk->sk_daddr = ip_hdr(skb)->daddr;
> 		break;

[ ... ]

This fixes a bug that has existed since fa5cb548ced6 ("bpf: Setup
socket family and addresses in bpf_prog_test_run_skb") which
introduced the sk_family assignment and ip_hdr()/ipv6_hdr() access
without rejecting packets too short to contain L3 headers.

Should this carry a Fixes: tag?  Something like:

Fixes: fa5cb548ced6 ("bpf: Setup socket family and addresses in bpf_prog_test_run_skb")

This was also raised in the v2 review thread:
https://lore.kernel.org/bpf/20260329161751.1914272-1-sun.jian.kdev@gmail.com/


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23910290394

Re: [PATCH bpf-next v3] bpf: reject short IPv4/IPv6 inputs in bpf_prog_test_run_skb

[Martin KaFai Lau]

On Fri, Apr 03, 2026 at 12:01:47AM +0800, Sun Jian wrote:
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 178c4738e63b..300e2bfc5a62 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -1120,19 +1120,23 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>  
>  	switch (skb->protocol) {
>  	case htons(ETH_P_IP):
> -		sk->sk_family = AF_INET;
> -		if (sizeof(struct iphdr) <= skb_headlen(skb)) {
> -			sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
> -			sk->sk_daddr = ip_hdr(skb)->daddr;
> +		if (skb_headlen(skb) < sizeof(struct iphdr)) {
> +			ret = -EINVAL;
> +			goto out;
>  		}
> +		sk->sk_family = AF_INET;
> +		sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
> +		sk->sk_daddr = ip_hdr(skb)->daddr;
>  		break;
>  #if IS_ENABLED(CONFIG_IPV6)
>  	case htons(ETH_P_IPV6):
> -		sk->sk_family = AF_INET6;
> -		if (sizeof(struct ipv6hdr) <= skb_headlen(skb)) {
> -			sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
> -			sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
> +		if (skb_headlen(skb) < sizeof(struct ipv6hdr)) {
> +			ret = -EINVAL;
> +			goto out;
>  		}
> +		sk->sk_family = AF_INET6;
> +		sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
> +		sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
>  		break;
>  #endif
>  	default:
> diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> index 438583e1f2d1..d53567e9cd77 100644
> --- a/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> +++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> @@ -12,6 +12,8 @@ void test_empty_skb(void)
>  	struct bpf_program *prog;
>  	char eth_hlen_pp[15];
>  	char eth_hlen[14];
> +	char ipv4_eth_hlen[14];
> +	char ipv6_eth_hlen[14];

The eth_hlen_pp and eth_hlen needs to memset zero now.

Instead of adding two more ethhdrs, just reuse the current eth_hlen and
define it as 'struct ethhdr eth_hlen;' instead of a char array.
Add 'h_proto' to the anonymous 'struct { } tests[]'.  Initialize
the eth_hlen.h_proto based on the tests[i].h_proto.

Also, this test does not actually reproduce the reading uninit memeory.
It needs a bpf prog to actually trigger it by calling bpf_skb_adjust_room()
based on the report in the "Closes" link. The test should be able
to trigger it without the change in test_run.c

The ai-review has flagged again that it is missing a "Fixes" tag. This
probably started since the bpf_skb_adjust_room helper was introduced.

The selftests should also be in a separate patch 2 following the patch 1
changes in test_run.c.

pw-bot: cr

Re: [PATCH bpf-next v3] bpf: reject short IPv4/IPv6 inputs in bpf_prog_test_run_skb

[sun jian]

On Tue, Apr 7, 2026 at 2:58 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On Fri, Apr 03, 2026 at 12:01:47AM +0800, Sun Jian wrote:
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index 178c4738e63b..300e2bfc5a62 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -1120,19 +1120,23 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> >
> >       switch (skb->protocol) {
> >       case htons(ETH_P_IP):
> > -             sk->sk_family = AF_INET;
> > -             if (sizeof(struct iphdr) <= skb_headlen(skb)) {
> > -                     sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
> > -                     sk->sk_daddr = ip_hdr(skb)->daddr;
> > +             if (skb_headlen(skb) < sizeof(struct iphdr)) {
> > +                     ret = -EINVAL;
> > +                     goto out;
> >               }
> > +             sk->sk_family = AF_INET;
> > +             sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
> > +             sk->sk_daddr = ip_hdr(skb)->daddr;
> >               break;
> >  #if IS_ENABLED(CONFIG_IPV6)
> >       case htons(ETH_P_IPV6):
> > -             sk->sk_family = AF_INET6;
> > -             if (sizeof(struct ipv6hdr) <= skb_headlen(skb)) {
> > -                     sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
> > -                     sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
> > +             if (skb_headlen(skb) < sizeof(struct ipv6hdr)) {
> > +                     ret = -EINVAL;
> > +                     goto out;
> >               }
> > +             sk->sk_family = AF_INET6;
> > +             sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
> > +             sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
> >               break;
> >  #endif
> >       default:
> > diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> > index 438583e1f2d1..d53567e9cd77 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
> > @@ -12,6 +12,8 @@ void test_empty_skb(void)
> >       struct bpf_program *prog;
> >       char eth_hlen_pp[15];
> >       char eth_hlen[14];
> > +     char ipv4_eth_hlen[14];
> > +     char ipv6_eth_hlen[14];
>
> The eth_hlen_pp and eth_hlen needs to memset zero now.
>
> Instead of adding two more ethhdrs, just reuse the current eth_hlen and
> define it as 'struct ethhdr eth_hlen;' instead of a char array.
> Add 'h_proto' to the anonymous 'struct { } tests[]'.  Initialize
> the eth_hlen.h_proto based on the tests[i].h_proto.
>
> Also, this test does not actually reproduce the reading uninit memeory.
> It needs a bpf prog to actually trigger it by calling bpf_skb_adjust_room()
> based on the report in the "Closes" link. The test should be able
> to trigger it without the change in test_run.c
>
> The ai-review has flagged again that it is missing a "Fixes" tag. This
> probably started since the bpf_skb_adjust_room helper was introduced.
>
> The selftests should also be in a separate patch 2 following the patch 1
> changes in test_run.c.
>
> pw-bot: cr

Ack. I'll split this into two patches and rework the selftest accordingly.