Re: [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()

Le mer. 28 oct. 2026 à 11:19, [off-list ref] a écrit :

quoted

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.

quoted

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

[ ... ]

quoted

@@ -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