* [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
@ 2026-10-28 9:19 Jamal Hadi Salim
2026-10-28 10:19 ` bot+bpf-ci
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-10-28 9:19 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Eric Dumazet, bpf, stable, Daniel Borkmann,
Alexei Starovoitov, David Ahern, Martin KaFai Lau,
Martin KaFai Lau, John Fastabend, Stanislav Fomichev,
Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
Ihor Solodrai, David S . Miller, Jakub Kicinski, Paolo Abeni,
Simon Horman, Jiri Pirko, Victor Nogueira
The issue:
This is a follow-up to the short link-layer header fixes[Link]. After
vlan pop_eth, a privileged SCHED_ACT BPF program can call
bpf_skb_change_head(skb, 1, 0) and then bpf_redirect_neigh().
__bpf_redirect_neigh() calls skb_pull(skb, sizeof(*ethh)) after checking
only that mac_header < network_header. It does not check that the MAC
header is long enough or that it is linear.
A SCHED_ACT BPF program can do:
bpf_skb_change_head(skb, 1, 0);
bpf_redirect_neigh(lo, NULL, 0, 0);
after vlan pop_eth. The skb then has mac_len == 1 and only one byte of
linear data; skb_pull(ETH_HLEN) trips the skb->len < skb->data_len BUG in
__skb_pull():
kernel BUG at include/linux/skbuff.h:2839
RIP: skb_pull+0xc5/0x100
Call Trace: skb_do_redirect -> ...
The Fix:
Drop skbs whose MAC header is shorter than ETH_HLEN, and use
pskb_may_pull() to ensure the Ethernet header is linear before reading or
pulling it. Re-read ethh after pskb_may_pull(), which may reallocate
skb->head.
Fixes: b4ab31414970 ("bpf: Add redirect_neigh helper as redirect drop-in")
Link: https://lore.kernel.org/netdev/20261027082604.1485903-1-edumazet@kernel.org/
Cc: stable@vger.kernel.org
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/core/filter.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 70dc621672f2..4d344ec09736 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2432,10 +2432,14 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
static int __bpf_redirect_neigh(struct sk_buff *skb, struct net_device *dev,
struct bpf_nh_params *nh)
{
- struct ethhdr *ethh = eth_hdr(skb);
+ struct ethhdr *ethh;
if (unlikely(skb->mac_header >= skb->network_header))
goto out;
+ if (unlikely(skb->mac_len < ETH_HLEN ||
+ !pskb_may_pull(skb, ETH_HLEN)))
+ goto out;
+ ethh = eth_hdr(skb);
bpf_push_mac_rcsum(skb);
if (is_multicast_ether_addr(ethh->h_dest))
goto out;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
2026-10-28 9:19 [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh() Jamal Hadi Salim
@ 2026-10-28 10:19 ` bot+bpf-ci
2026-10-28 11:40 ` Eric Dumazet
2026-10-28 11:54 ` Jamal Hadi Salim
2026-10-29 9:19 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: bot+bpf-ci @ 2026-10-28 10:19 UTC (permalink / raw)
To: jhs, netdev
Cc: jhs, edumazet, bpf, stable, daniel, ast, dsahern, kafai,
martin.lau, john.fastabend, sdf, andrii, eddyz87, memxor, song,
yonghong.song, jolsa, emil, ihor.solodrai, davem, kuba, pabeni,
horms, jiri, victor, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3853 bytes --]
> bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
TL;DR: The new skb->mac_len < ETH_HLEN check in __bpf_redirect_neigh() may
wrongly drop valid packets on tc egress and netkit, where mac_len is often 0;
a regression that would reach stable.
> diff --git a/net/core/filter.c b/net/core/filter.c
> index b7ac145b68b1..95a8a728a71d 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
[ ... ]
> @@ -2451,10 +2451,14 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
> static int __bpf_redirect_neigh(struct sk_buff *skb, struct net_device *dev,
> struct bpf_nh_params *nh)
> {
> - struct ethhdr *ethh = eth_hdr(skb);
> + struct ethhdr *ethh;
>
> if (unlikely(skb->mac_header >= skb->network_header))
> goto out;
> + if (unlikely(skb->mac_len < ETH_HLEN ||
> + !pskb_may_pull(skb, ETH_HLEN)))
> + goto out;
Does the new skb->mac_len < ETH_HLEN test drop valid packets on tc egress
and netkit?
skb->mac_len is only set on the receive path, where
__netif_receive_skb_core() calls skb_reset_mac_len(). The transmit path
only calls skb_reset_mac_header() in __dev_queue_xmit(), and nothing in
ip_finish_output2(), neigh_output(), eth_header(), sch_handle_egress(),
tcx_run(), cls_bpf_classify() (egress branch), netkit_xmit() or
netkit_prep_forward() writes mac_len.
A packet built by a local socket starts with mac_len == 0, because
skbuff_clear() zeroes everything before 'tail', and skb_clone() (used by TCP
transmit) copies that 0.
For example, with tc egress and a locally generated IPv4/IPv6 packet:
ip_finish_output2()
neigh_output()
dev_queue_xmit()
__dev_queue_xmit() <- skb_reset_mac_header() only
sch_handle_egress()
tc_run() / tcx_run()
prog returns bpf_redirect_neigh() = TC_ACT_REDIRECT
skb_do_redirect()
__bpf_redirect_neigh()
Here mac_header < network_header passes, because an Ethernet header was
pushed, but mac_len == 0 < ETH_HLEN, so the skb goes to 'out', gets
kfree_skb() and -ENOTSUPP is returned.
The same happens with netkit:
netkit_xmit()
netkit_run()
NETKIT_REDIRECT
skb_do_redirect()
__bpf_redirect_neigh()
with mac_len still 0. Netkit programs are BPF_PROG_TYPE_SCHED_CLS, and
tc_cls_act_func_proto() allows BPF_FUNC_redirect_neigh. Forwarded traffic
that arrived on a layer 3 device (mac_len 0 on receive) and leaves through
an Ethernet device's tc egress would be dropped as well.
Before this patch these paths worked, since the Ethernet header is linear
(dev_hard_header() pushes it into the head) and skb_pull(skb, ETH_HLEN) was
fine. bpf_out_neigh_v4/v6() calling sock_confirm_neigh(skb, neigh) also
suggests the helper is meant to handle locally generated skbs.
Elsewhere the kernel already treats mac_len as unreliable at egress.
act_mirred uses:
mac_len = at_ingress ? skb->mac_len : skb_network_offset(skb);
and act_mpls uses skb_network_offset(skb) when not at ingress.
The bpf_redirect_neigh selftests (test_tc_neigh, test_tc_neigh_fib,
test_tc_dtime) all attach at tc ingress, where mac_len is valid, so they
would not catch this.
Would testing the real MAC header length work instead, for example
skb_mac_header_len(skb) < ETH_HLEN (network_header - mac_header), or
skb_network_offset(skb) < ETH_HLEN with skb->data at the MAC header here?
That would still reject the 1-byte header from the commit message (pop_eth
followed by bpf_skb_change_head(skb, 1, 0)) without dropping egress and
netkit traffic. Since the patch carries Cc: stable, this regression would
also reach stable kernels.
---
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/40666397744
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
2026-10-28 10:19 ` bot+bpf-ci
@ 2026-10-28 11:40 ` Eric Dumazet
2026-10-28 11:53 ` Jamal Hadi Salim
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2026-10-28 11:40 UTC (permalink / raw)
To: bot+bpf-ci
Cc: jhs, netdev, bpf, stable, daniel, ast, dsahern, kafai, martin.lau,
john.fastabend, sdf, andrii, eddyz87, memxor, song, yonghong.song,
jolsa, emil, ihor.solodrai, davem, kuba, pabeni, horms, jiri,
victor, martin.lau, mason
Le mer. 28 oct. 2026 à 11:19, <bot+bpf-ci@kernel.org> a écrit :
>
> > bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
>
> TL;DR: The new skb->mac_len < ETH_HLEN check in __bpf_redirect_neigh() may
> wrongly drop valid packets on tc egress and netkit, where mac_len is often 0;
> a regression that would reach stable.
>
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index b7ac145b68b1..95a8a728a71d 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
>
> [ ... ]
>
> > @@ -2451,10 +2451,14 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
> > static int __bpf_redirect_neigh(struct sk_buff *skb, struct net_device *dev,
> > struct bpf_nh_params *nh)
> > {
> > - struct ethhdr *ethh = eth_hdr(skb);
> > + struct ethhdr *ethh;
> >
> > if (unlikely(skb->mac_header >= skb->network_header))
> > goto out;
> > + if (unlikely(skb->mac_len < ETH_HLEN ||
> > + !pskb_may_pull(skb, ETH_HLEN)))
> > + goto out;
>
> Does the new skb->mac_len < ETH_HLEN test drop valid packets on tc egress
> and netkit?
>
The bot is right, sorry Jamal, I missed this when looking at your
first version.
skb->mac_len is only set on the receive path. On tc egress (and netkit),
it is usually 0 for locally generated packets, so this would drop all
bpf_redirect_neigh() traffic from egress programs.
In both cases skb->data is at the MAC header when __bpf_redirect_neigh()
runs (sch_handle_ingress() pushes skb->mac_len before skb_do_redirect()),
so you can use skb_network_offset() instead:
if (unlikely(skb_network_offset(skb) < ETH_HLEN ||
!pskb_may_pull(skb, ETH_HLEN)))
goto out;
This still rejects the 1-byte header from bpf_skb_change_head(skb, 1, 0),
and also covers the existing mac_header >= network_header test.
> skb->mac_len is only set on the receive path, where
> __netif_receive_skb_core() calls skb_reset_mac_len(). The transmit path
> only calls skb_reset_mac_header() in __dev_queue_xmit(), and nothing in
> ip_finish_output2(), neigh_output(), eth_header(), sch_handle_egress(),
> tcx_run(), cls_bpf_classify() (egress branch), netkit_xmit() or
> netkit_prep_forward() writes mac_len.
>
> A packet built by a local socket starts with mac_len == 0, because
> skbuff_clear() zeroes everything before 'tail', and skb_clone() (used by TCP
> transmit) copies that 0.
>
> For example, with tc egress and a locally generated IPv4/IPv6 packet:
>
> ip_finish_output2()
> neigh_output()
> dev_queue_xmit()
> __dev_queue_xmit() <- skb_reset_mac_header() only
> sch_handle_egress()
> tc_run() / tcx_run()
> prog returns bpf_redirect_neigh() = TC_ACT_REDIRECT
> skb_do_redirect()
> __bpf_redirect_neigh()
>
> Here mac_header < network_header passes, because an Ethernet header was
> pushed, but mac_len == 0 < ETH_HLEN, so the skb goes to 'out', gets
> kfree_skb() and -ENOTSUPP is returned.
>
> The same happens with netkit:
>
> netkit_xmit()
> netkit_run()
> NETKIT_REDIRECT
> skb_do_redirect()
> __bpf_redirect_neigh()
>
> with mac_len still 0. Netkit programs are BPF_PROG_TYPE_SCHED_CLS, and
> tc_cls_act_func_proto() allows BPF_FUNC_redirect_neigh. Forwarded traffic
> that arrived on a layer 3 device (mac_len 0 on receive) and leaves through
> an Ethernet device's tc egress would be dropped as well.
>
> Before this patch these paths worked, since the Ethernet header is linear
> (dev_hard_header() pushes it into the head) and skb_pull(skb, ETH_HLEN) was
> fine. bpf_out_neigh_v4/v6() calling sock_confirm_neigh(skb, neigh) also
> suggests the helper is meant to handle locally generated skbs.
>
> Elsewhere the kernel already treats mac_len as unreliable at egress.
> act_mirred uses:
>
> mac_len = at_ingress ? skb->mac_len : skb_network_offset(skb);
>
> and act_mpls uses skb_network_offset(skb) when not at ingress.
>
> The bpf_redirect_neigh selftests (test_tc_neigh, test_tc_neigh_fib,
> test_tc_dtime) all attach at tc ingress, where mac_len is valid, so they
> would not catch this.
>
> Would testing the real MAC header length work instead, for example
> skb_mac_header_len(skb) < ETH_HLEN (network_header - mac_header), or
> skb_network_offset(skb) < ETH_HLEN with skb->data at the MAC header here?
>
> That would still reject the 1-byte header from the commit message (pop_eth
> followed by bpf_skb_change_head(skb, 1, 0)) without dropping egress and
> netkit traffic. Since the patch carries Cc: stable, this regression would
> also reach stable kernels.
>
>
> ---
> 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/40666397744
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
2026-10-28 11:40 ` Eric Dumazet
@ 2026-10-28 11:53 ` Jamal Hadi Salim
2026-10-28 20:10 ` Daniel Borkmann
0 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-10-28 11:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: bot+bpf-ci, netdev, bpf, stable, daniel, ast, dsahern, kafai,
martin.lau, john.fastabend, sdf, andrii, eddyz87, memxor, song,
yonghong.song, jolsa, emil, ihor.solodrai, davem, kuba, pabeni,
horms, jiri, victor, martin.lau, mason
On Wed, Oct 28, 2026 at 7:40 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le mer. 28 oct. 2026 à 11:19, <bot+bpf-ci@kernel.org> a écrit :
> >
> > > bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
> >
> > TL;DR: The new skb->mac_len < ETH_HLEN check in __bpf_redirect_neigh() may
> > wrongly drop valid packets on tc egress and netkit, where mac_len is often 0;
> > a regression that would reach stable.
> >
> > > diff --git a/net/core/filter.c b/net/core/filter.c
> > > index b7ac145b68b1..95a8a728a71d 100644
> > > --- a/net/core/filter.c
> > > +++ b/net/core/filter.c
> >
> > [ ... ]
> >
> > > @@ -2451,10 +2451,14 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
> > > static int __bpf_redirect_neigh(struct sk_buff *skb, struct net_device *dev,
> > > struct bpf_nh_params *nh)
> > > {
> > > - struct ethhdr *ethh = eth_hdr(skb);
> > > + struct ethhdr *ethh;
> > >
> > > if (unlikely(skb->mac_header >= skb->network_header))
> > > goto out;
> > > + if (unlikely(skb->mac_len < ETH_HLEN ||
> > > + !pskb_may_pull(skb, ETH_HLEN)))
> > > + goto out;
> >
> > Does the new skb->mac_len < ETH_HLEN test drop valid packets on tc egress
> > and netkit?
> >
>
>
> The bot is right, sorry Jamal, I missed this when looking at your
> first version.
>
agreed - i will send a v2. I will also test egress/netkit path.
That was fast! (in 7 days on the sashiko nipa i dont remember the
context given the number of fixes)
cheers,
jamal
> skb->mac_len is only set on the receive path. On tc egress (and netkit),
> it is usually 0 for locally generated packets, so this would drop all
> bpf_redirect_neigh() traffic from egress programs.
>
> In both cases skb->data is at the MAC header when __bpf_redirect_neigh()
> runs (sch_handle_ingress() pushes skb->mac_len before skb_do_redirect()),
> so you can use skb_network_offset() instead:
>
> if (unlikely(skb_network_offset(skb) < ETH_HLEN ||
> !pskb_may_pull(skb, ETH_HLEN)))
> goto out;
>
> This still rejects the 1-byte header from bpf_skb_change_head(skb, 1, 0),
> and also covers the existing mac_header >= network_header test.
>
>
> > skb->mac_len is only set on the receive path, where
> > __netif_receive_skb_core() calls skb_reset_mac_len(). The transmit path
> > only calls skb_reset_mac_header() in __dev_queue_xmit(), and nothing in
> > ip_finish_output2(), neigh_output(), eth_header(), sch_handle_egress(),
> > tcx_run(), cls_bpf_classify() (egress branch), netkit_xmit() or
> > netkit_prep_forward() writes mac_len.
> >
> > A packet built by a local socket starts with mac_len == 0, because
> > skbuff_clear() zeroes everything before 'tail', and skb_clone() (used by TCP
> > transmit) copies that 0.
> >
> > For example, with tc egress and a locally generated IPv4/IPv6 packet:
> >
> > ip_finish_output2()
> > neigh_output()
> > dev_queue_xmit()
> > __dev_queue_xmit() <- skb_reset_mac_header() only
> > sch_handle_egress()
> > tc_run() / tcx_run()
> > prog returns bpf_redirect_neigh() = TC_ACT_REDIRECT
> > skb_do_redirect()
> > __bpf_redirect_neigh()
> >
> > Here mac_header < network_header passes, because an Ethernet header was
> > pushed, but mac_len == 0 < ETH_HLEN, so the skb goes to 'out', gets
> > kfree_skb() and -ENOTSUPP is returned.
> >
> > The same happens with netkit:
> >
> > netkit_xmit()
> > netkit_run()
> > NETKIT_REDIRECT
> > skb_do_redirect()
> > __bpf_redirect_neigh()
> >
> > with mac_len still 0. Netkit programs are BPF_PROG_TYPE_SCHED_CLS, and
> > tc_cls_act_func_proto() allows BPF_FUNC_redirect_neigh. Forwarded traffic
> > that arrived on a layer 3 device (mac_len 0 on receive) and leaves through
> > an Ethernet device's tc egress would be dropped as well.
> >
> > Before this patch these paths worked, since the Ethernet header is linear
> > (dev_hard_header() pushes it into the head) and skb_pull(skb, ETH_HLEN) was
> > fine. bpf_out_neigh_v4/v6() calling sock_confirm_neigh(skb, neigh) also
> > suggests the helper is meant to handle locally generated skbs.
> >
> > Elsewhere the kernel already treats mac_len as unreliable at egress.
> > act_mirred uses:
> >
> > mac_len = at_ingress ? skb->mac_len : skb_network_offset(skb);
> >
> > and act_mpls uses skb_network_offset(skb) when not at ingress.
> >
> > The bpf_redirect_neigh selftests (test_tc_neigh, test_tc_neigh_fib,
> > test_tc_dtime) all attach at tc ingress, where mac_len is valid, so they
> > would not catch this.
> >
> > Would testing the real MAC header length work instead, for example
> > skb_mac_header_len(skb) < ETH_HLEN (network_header - mac_header), or
> > skb_network_offset(skb) < ETH_HLEN with skb->data at the MAC header here?
> >
> > That would still reject the 1-byte header from the commit message (pop_eth
> > followed by bpf_skb_change_head(skb, 1, 0)) without dropping egress and
> > netkit traffic. Since the patch carries Cc: stable, this regression would
> > also reach stable kernels.
> >
> >
> > ---
> > 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/40666397744
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
2026-10-28 9:19 [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh() Jamal Hadi Salim
2026-10-28 10:19 ` bot+bpf-ci
@ 2026-10-28 11:54 ` Jamal Hadi Salim
2026-10-29 9:19 ` sashiko-bot
2 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-10-28 11:54 UTC (permalink / raw)
To: netdev
Cc: Eric Dumazet, bpf, stable, Daniel Borkmann, Alexei Starovoitov,
David Ahern, Martin KaFai Lau, Martin KaFai Lau, John Fastabend,
Stanislav Fomichev, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song, Jiri Olsa,
Emil Tsalapatis, Ihor Solodrai, David S . Miller, Jakub Kicinski,
Paolo Abeni, Simon Horman, Jiri Pirko, Victor Nogueira
On Wed, Oct 28, 2026 at 5:19 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> The issue:
> This is a follow-up to the short link-layer header fixes[Link]. After
> vlan pop_eth, a privileged SCHED_ACT BPF program can call
> bpf_skb_change_head(skb, 1, 0) and then bpf_redirect_neigh().
>
> __bpf_redirect_neigh() calls skb_pull(skb, sizeof(*ethh)) after checking
> only that mac_header < network_header. It does not check that the MAC
> header is long enough or that it is linear.
>
> A SCHED_ACT BPF program can do:
>
> bpf_skb_change_head(skb, 1, 0);
> bpf_redirect_neigh(lo, NULL, 0, 0);
>
> after vlan pop_eth. The skb then has mac_len == 1 and only one byte of
> linear data; skb_pull(ETH_HLEN) trips the skb->len < skb->data_len BUG in
> __skb_pull():
>
> kernel BUG at include/linux/skbuff.h:2839
> RIP: skb_pull+0xc5/0x100
> Call Trace: skb_do_redirect -> ...
>
> The Fix:
> Drop skbs whose MAC header is shorter than ETH_HLEN, and use
> pskb_may_pull() to ensure the Ethernet header is linear before reading or
> pulling it. Re-read ethh after pskb_may_pull(), which may reallocate
> skb->head.
>
pw-bot: cr
cheers,
jamal
> Fixes: b4ab31414970 ("bpf: Add redirect_neigh helper as redirect drop-in")
> Link: https://lore.kernel.org/netdev/20261027082604.1485903-1-edumazet@kernel.org/
> Cc: stable@vger.kernel.org
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> net/core/filter.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 70dc621672f2..4d344ec09736 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2432,10 +2432,14 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
> static int __bpf_redirect_neigh(struct sk_buff *skb, struct net_device *dev,
> struct bpf_nh_params *nh)
> {
> - struct ethhdr *ethh = eth_hdr(skb);
> + struct ethhdr *ethh;
>
> if (unlikely(skb->mac_header >= skb->network_header))
> goto out;
> + if (unlikely(skb->mac_len < ETH_HLEN ||
> + !pskb_may_pull(skb, ETH_HLEN)))
> + goto out;
> + ethh = eth_hdr(skb);
> bpf_push_mac_rcsum(skb);
> if (is_multicast_ether_addr(ethh->h_dest))
> goto out;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
2026-10-28 11:53 ` Jamal Hadi Salim
@ 2026-10-28 20:10 ` Daniel Borkmann
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Borkmann @ 2026-10-28 20:10 UTC (permalink / raw)
To: Jamal Hadi Salim, Eric Dumazet
Cc: bot+bpf-ci, netdev, bpf, stable, ast, dsahern, kafai, martin.lau,
john.fastabend, sdf, andrii, eddyz87, memxor, song, yonghong.song,
jolsa, emil, ihor.solodrai, davem, kuba, pabeni, horms, jiri,
victor, martin.lau, mason
On 10/28/26 12:53 PM, Jamal Hadi Salim wrote:
> On Wed, Oct 28, 2026 at 7:40 AM Eric Dumazet <edumazet@kernel.org> wrote:
>> Le mer. 28 oct. 2026 à 11:19, <bot+bpf-ci@kernel.org> a écrit :
>>>
>>>> bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
>>>
>>> TL;DR: The new skb->mac_len < ETH_HLEN check in __bpf_redirect_neigh() may
>>> wrongly drop valid packets on tc egress and netkit, where mac_len is often 0;
>>> a regression that would reach stable.
>>>
>>>> diff --git a/net/core/filter.c b/net/core/filter.c
>>>> index b7ac145b68b1..95a8a728a71d 100644
>>>> --- a/net/core/filter.c
>>>> +++ b/net/core/filter.c
>>>
>>> [ ... ]
>>>
>>>> @@ -2451,10 +2451,14 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
>>>> static int __bpf_redirect_neigh(struct sk_buff *skb, struct net_device *dev,
>>>> struct bpf_nh_params *nh)
>>>> {
>>>> - struct ethhdr *ethh = eth_hdr(skb);
>>>> + struct ethhdr *ethh;
>>>>
>>>> if (unlikely(skb->mac_header >= skb->network_header))
>>>> goto out;
>>>> + if (unlikely(skb->mac_len < ETH_HLEN ||
>>>> + !pskb_may_pull(skb, ETH_HLEN)))
>>>> + goto out;
>>>
>>> Does the new skb->mac_len < ETH_HLEN test drop valid packets on tc egress
>>> and netkit?
>>
>> The bot is right, sorry Jamal, I missed this when looking at your
>> first version.
>
> agreed - i will send a v2. I will also test egress/netkit path.
> That was fast! (in 7 days on the sashiko nipa i dont remember the
> context given the number of fixes)
Lets also add a BPF selftest for all corner cases to make sure things don't break.
If you need help, I can also take your fix, add a netkit selftest on top and then
we ship via bpf.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
2026-10-28 9:19 [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh() Jamal Hadi Salim
2026-10-28 10:19 ` bot+bpf-ci
2026-10-28 11:54 ` Jamal Hadi Salim
@ 2026-10-29 9:19 ` sashiko-bot
2 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-29 9:19 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: packet drop on egress in __bpf_redirect_neigh() due to mac_len check
--
commit cc6241a04af2e98e3d6758d1d73c0ca8995faa4a
Author: Jamal Hadi Salim <jhs@mojatatu.com>
bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
This commit adds a check on skb->mac_len and a pskb_may_pull() call in
__bpf_redirect_neigh() before reading and pulling the Ethernet header to
prevent a BUG in __skb_pull() on short or non-linear link-layer headers.
Link: https://lore.kernel.org/netdev/20261027082604.1485903-1-edumazet@kernel.org/
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 70dc621672f2e..4d344ec097361 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2432,10 +2432,14 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
> static int __bpf_redirect_neigh(struct sk_buff *skb, struct net_device *dev,
> struct bpf_nh_params *nh)
> {
> - struct ethhdr *ethh = eth_hdr(skb);
> + struct ethhdr *ethh;
>
> if (unlikely(skb->mac_header >= skb->network_header))
> goto out;
> + if (unlikely(skb->mac_len < ETH_HLEN ||
> + !pskb_may_pull(skb, ETH_HLEN)))
> + goto out;
[Severity: High]
Does checking skb->mac_len < ETH_HLEN here cause __bpf_redirect_neigh() to
drop valid packets on egress and netkit transmit paths?
bpf_redirect_neigh() is available to BPF_PROG_TYPE_SCHED_CLS and
BPF_PROG_TYPE_SCHED_ACT programs via tc_cls_act_func_proto(), which run on
TC egress in sch_handle_egress() and in netkit_xmit() in addition to TC
ingress.
While the receive path sets skb->mac_len in __netif_receive_skb_core() via
skb_reset_mac_len(skb), the transmit path for locally-originated or
L3-forwarded packets leaves skb->mac_len as 0 when pushing the Ethernet
header and only resets skb->mac_header:
net/core/dev.c:__dev_queue_xmit() {
...
skb_reset_mac_header(skb);
skb_assert_len(skb);
...
skb = sch_handle_egress(skb, &rc, dev);
}
and in drivers/net/netkit.c:
drivers/net/netkit.c:netkit_prep_forward() {
skb_scrub_packet(skb, false);
nf_skip_egress(skb, true);
skb_reset_mac_header(skb);
...
}
drivers/net/netkit.c:netkit_xmit() {
...
netkit_prep_forward(skb, ! net_eq(net, peer_net), xnet_scrub);
...
ret = netkit_run(prog, skb);
switch (ret) {
...
case NETKIT_REDIRECT:
dev_sw_netstats_tx_add(dev, 1, len);
skb_do_redirect(skb);
break;
}
When sch_handle_egress() or netkit_xmit() calls skb_do_redirect(skb) ->
__bpf_redirect_neigh(), skb->network_header - skb->mac_header is ETH_HLEN,
but skb->mac_len is 0. This causes skb->mac_len < ETH_HLEN to evaluate to
true, freeing the skb via kfree_skb(skb) and returning -ENOTSUPP.
Conversely, if a packet received on an Ethernet device (where skb->mac_len
was set to ETH_HLEN) is forwarded via __dev_queue_xmit() out a device with a
link-layer header shorter than ETH_HLEN, __dev_queue_xmit() updates
skb->mac_header without updating skb->mac_len. The stale skb->mac_len passes
the skb->mac_len < ETH_HLEN check and skb_pull(skb, sizeof(*ethh)) pulls
past skb->network_header into the network header.
Should __bpf_redirect_neigh() instead check that skb->mac_header + ETH_HLEN
<= skb->network_header (or skb_network_offset(skb) >= ETH_HLEN), similar to
__bpf_redirect_no_mac() and skb_eth_pop()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/bpf_redirect_neigh_short_linear.v1.r0.20261028033717-1@mojatatu.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-29 9:19 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 9:19 [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh() Jamal Hadi Salim
2026-10-28 10:19 ` bot+bpf-ci
2026-10-28 11:40 ` Eric Dumazet
2026-10-28 11:53 ` Jamal Hadi Salim
2026-10-28 20:10 ` Daniel Borkmann
2026-10-28 11:54 ` Jamal Hadi Salim
2026-10-29 9:19 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.