* Re: [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input
2026-03-29 16:17 [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input Sun Jian
@ 2026-03-29 16:52 ` bot+bpf-ci
2026-03-30 4:23 ` sun jian
2026-04-02 2:17 ` Martin KaFai Lau
2 siblings, 0 replies; 7+ messages in thread
From: bot+bpf-ci @ 2026-03-29 16:52 UTC (permalink / raw)
To: sun.jian.kdev, ast, daniel, andrii
Cc: martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni, horms, bpf,
netdev, linux-kernel, sun.jian.kdev, syzbot+619b9ef527f510a57cfc,
ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 654 bytes --]
> 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>
This patch fixes a bug that has existed since bpf_prog_test_run_skb()
was first introduced. Should it carry a Fixes: tag pointing at the
original commit?
Fixes: 1cf1cae963c2 ("bpf: introduce BPF_PROG_TEST_RUN command")
---
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/23713682616
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input
2026-03-29 16:17 [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input Sun Jian
2026-03-29 16:52 ` bot+bpf-ci
@ 2026-03-30 4:23 ` sun jian
2026-04-02 2:17 ` Martin KaFai Lau
2 siblings, 0 replies; 7+ messages in thread
From: sun jian @ 2026-03-30 4:23 UTC (permalink / raw)
To: ast, daniel, andrii
Cc: martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni, horms, bpf,
netdev, linux-kernel, syzbot+619b9ef527f510a57cfc
Hi,
Syzbot has tested v2 and the reproducer did not trigger the issue.
Tested-by: syzbot+619b9ef527f510a57cfc@syzkaller.appspotmail.com
On Mon, Mar 30, 2026 at 12:18 AM Sun Jian <sun.jian.kdev@gmail.com> wrote:
>
> bpf_prog_test_run_skb() derives skb->protocol from the Ethernet header
> through eth_type_trans(), but it does not verify that the provided
> linear input is long enough to contain the corresponding L3 base header.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input
2026-03-29 16:17 [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input Sun Jian
2026-03-29 16:52 ` bot+bpf-ci
2026-03-30 4:23 ` sun jian
@ 2026-04-02 2:17 ` Martin KaFai Lau
2026-04-02 2:54 ` sun jian
2 siblings, 1 reply; 7+ messages in thread
From: Martin KaFai Lau @ 2026-04-02 2:17 UTC (permalink / raw)
To: Sun Jian
Cc: ast, daniel, andrii, eddyz87, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni, horms,
bpf, netdev, linux-kernel, syzbot+619b9ef527f510a57cfc
On Mon, Mar 30, 2026 at 12:17:51AM +0800, Sun Jian wrote:
> bpf_prog_test_run_skb() derives skb->protocol from the Ethernet header
> through eth_type_trans(), but it does not verify that the provided
> linear input is long enough to contain the corresponding L3 base header.
>
> This can result in an inconsistent skb being passed to test_run helpers
> such as bpf_skb_adjust_room(), where inferred protocol offsets can lead
> to operating on uninitialized memory, triggering KMSAN errors.
>
> To reject such malformed test input, we check that the linear head is
> sufficiently large to contain the corresponding L3 base header (IPv4
> or IPv6) before running the program.
>
> 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>
> ---
> v2:
> - Ensured that the linear head is large enough to accommodate the corresponding L3 base header (IPv4 or IPv6), before running the program.
>
> Link: <https://lore.kernel.org/bpf/129d235b04aca276c0a57c7c3646ce48644458cdc85d9b92b25f405e2d58a9ae@mail.kernel.org/>
>
> net/bpf/test_run.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 178c4738e63b..4790bee535b9 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -1118,6 +1118,25 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> skb->protocol = eth_type_trans(skb, dev);
> skb_reset_network_header(skb);
>
> + switch (skb->protocol) {
The implementation is poor.
> + case htons(ETH_P_IP):
> + if (skb_headlen(skb) < sizeof(struct iphdr)) {
> + ret = -EINVAL;
> + goto out;
> + }
> + break;
> +#if IS_ENABLED(CONFIG_IPV6)
> + case htons(ETH_P_IPV6):
> + if (skb_headlen(skb) < sizeof(struct ipv6hdr)) {
> + ret = -EINVAL;
> + goto out;
> + }
> + break;
> +#endif
> + default:
> + break;
> + }
> +
> switch (skb->protocol) {
There is an exact same switch and it has the same check
on skb_headlen(skb).
A selftest is needed. Check if the tests in empty_skb.c need to be
changed also. imo, This can be bpf-next.
pw-bot: cr
> case htons(ETH_P_IP):
> sk->sk_family = AF_INET;
>
> base-commit: cbfffcca2bf0622b601b7eaf477aa29035169184
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input
2026-04-02 2:17 ` Martin KaFai Lau
@ 2026-04-02 2:54 ` sun jian
2026-04-02 5:13 ` Martin KaFai Lau
0 siblings, 1 reply; 7+ messages in thread
From: sun jian @ 2026-04-02 2:54 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: ast, daniel, andrii, eddyz87, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni, horms,
bpf, netdev, linux-kernel, syzbot+619b9ef527f510a57cfc
On Thu, Apr 2, 2026 at 10:17 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On Mon, Mar 30, 2026 at 12:17:51AM +0800, Sun Jian wrote:
> > bpf_prog_test_run_skb() derives skb->protocol from the Ethernet header
> > through eth_type_trans(), but it does not verify that the provided
> > linear input is long enough to contain the corresponding L3 base header.
> >
> > This can result in an inconsistent skb being passed to test_run helpers
> > such as bpf_skb_adjust_room(), where inferred protocol offsets can lead
> > to operating on uninitialized memory, triggering KMSAN errors.
> >
> > To reject such malformed test input, we check that the linear head is
> > sufficiently large to contain the corresponding L3 base header (IPv4
> > or IPv6) before running the program.
> >
> > 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>
> > ---
> > v2:
> > - Ensured that the linear head is large enough to accommodate the corresponding L3 base header (IPv4 or IPv6), before running the program.
> >
> > Link: <https://lore.kernel.org/bpf/129d235b04aca276c0a57c7c3646ce48644458cdc85d9b92b25f405e2d58a9ae@mail.kernel.org/>
> >
> > net/bpf/test_run.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index 178c4738e63b..4790bee535b9 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -1118,6 +1118,25 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> > skb->protocol = eth_type_trans(skb, dev);
> > skb_reset_network_header(skb);
> >
> > + switch (skb->protocol) {
>
> The implementation is poor.
>
> > + case htons(ETH_P_IP):
> > + if (skb_headlen(skb) < sizeof(struct iphdr)) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + break;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + case htons(ETH_P_IPV6):
> > + if (skb_headlen(skb) < sizeof(struct ipv6hdr)) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + break;
> > +#endif
> > + default:
> > + break;
> > + }
> > +
> > switch (skb->protocol) {
>
> There is an exact same switch and it has the same check
> on skb_headlen(skb).
>
> A selftest is needed. Check if the tests in empty_skb.c need to be
> changed also. imo, This can be bpf-next.
>
> pw-bot: cr
>
> > case htons(ETH_P_IP):
> > sk->sk_family = AF_INET;
> >
> > base-commit: cbfffcca2bf0622b601b7eaf477aa29035169184
> > --
> > 2.43.0
> >
Ack, I'll respin a v2.
BTW, v1 was mainly meant as a minimal proof of the fix, so I
kept the existing structure intact.
Sun Jian
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input
2026-04-02 2:54 ` sun jian
@ 2026-04-02 5:13 ` Martin KaFai Lau
2026-04-02 7:35 ` sun jian
0 siblings, 1 reply; 7+ messages in thread
From: Martin KaFai Lau @ 2026-04-02 5:13 UTC (permalink / raw)
To: sun jian
Cc: ast, daniel, andrii, eddyz87, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni, horms,
bpf, netdev, linux-kernel, syzbot+619b9ef527f510a57cfc
On Thu, Apr 02, 2026 at 10:54:41AM +0800, sun jian wrote:
> Ack, I'll respin a v2.
>
> BTW, v1 was mainly meant as a minimal proof of the fix, so I
> kept the existing structure intact.
This is already v2.
The minimal proof of the fix is a selftest for a tricky case
like this, instead of spamming the list, and now also the
AI-review tokens, with an unlandable patch.
It is a few line change, and I don't see how duplicating the
existing switch case makes the RFC review easier.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input
2026-04-02 5:13 ` Martin KaFai Lau
@ 2026-04-02 7:35 ` sun jian
0 siblings, 0 replies; 7+ messages in thread
From: sun jian @ 2026-04-02 7:35 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: ast, daniel, andrii, eddyz87, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni, horms,
bpf, netdev, linux-kernel, syzbot+619b9ef527f510a57cfc
On Thu, Apr 2, 2026 at 1:13 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On Thu, Apr 02, 2026 at 10:54:41AM +0800, sun jian wrote:
> > Ack, I'll respin a v2.
> >
> > BTW, v1 was mainly meant as a minimal proof of the fix, so I
> > kept the existing structure intact.
>
> This is already v2.
>
> The minimal proof of the fix is a selftest for a tricky case
> like this, instead of spamming the list, and now also the
> AI-review tokens, with an unlandable patch.
>
> It is a few line change, and I don't see how duplicating the
> existing switch case makes the RFC review easier.
You‘re right, I just noticed that I mixed up the version number.
I'll respin a v3 and include the selftest.
Sun Jian
^ permalink raw reply [flat|nested] 7+ messages in thread