Re: [PATCH net v2] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
flat view
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-10-31 13:32:18
Also in:
bpf, linux-kselftest, stable
On Fri, Oct 30, 2026 at 5:47 PM Alexei Starovoitov
[off-list ref] wrote:On Thu, Oct 29, 2026 at 02:36 AM Jamal Hadi Salim [off-list ref] wrote:quoted
Add egress coverage to tc_redirect_neigh: the existing test only attaches redirect_neigh at ingress, where mac_len is populated.This test cannot hit the BUG.
It was reachable _before_ the fix, normally useful to include such tests.
I will drop it, but if you think it is still useful and you catch me
before i send v3 i can include it (or can be sent afterwards).
Just drop it and respin as [PATCH bpf v3].
Will do a v3 sometimes today for below... cheers, jamal
quoted hunk ↗ jump to hunk
quoted
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_header_len(skb) < ETH_HLEN || + !pskb_may_pull(skb, ETH_HLEN))) + goto out; + ethh = eth_hdr(skb);ETH_HLEN is not enough. __bpf_redirect_neigh_v4() and __bpf_redirect_neigh_v6() read ip_hdr() and ipv6_hdr() right after the pull. Nothing makes these headers linear. Take the AF_PACKET frame from the commit log without pop_eth and bpf_skb_change_head(): packet_snd() headlen == 14, IP header in frags skb_pull(skb, ETH_HLEN) headlen == 0 skb_reset_network_header(skb) network header == skb->tail ip4h->daddr, ip4h->saddr read past skb->tail The route lookup runs on garbage. Eric's skb_network_offset() check also covers mac_header >= network_header. No need for two checks. Something like this:diff --git a/net/core/filter.c b/net/core/filter.c --- a/net/core/filter.c +++ b/net/core/filter.c@@ -2290,10 +2290,14 @@ static int bpf_out_neigh_v6(struct net *net, struct sk_buff *skb, static int __bpf_redirect_neigh_v6(struct sk_buff *skb, struct net_device *dev, struct bpf_nh_params *nh) { - const struct ipv6hdr *ip6h = ipv6_hdr(skb); struct net *net = dev_net(dev); int err, ret = NET_XMIT_DROP; + const struct ipv6hdr *ip6h; + + if (unlikely(!pskb_may_pull(skb, sizeof(*ip6h)))) + goto out_drop; + ip6h = ipv6_hdr(skb); if (!nh) { struct dst_entry *dst; struct flowi6 fl6 = {@@ -2399,10 +2403,14 @@ static int bpf_out_neigh_v4(struct net *net, struct sk_buff *skb, static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev, struct bpf_nh_params *nh) { - const struct iphdr *ip4h = ip_hdr(skb); struct net *net = dev_net(dev); int err, ret = NET_XMIT_DROP; + const struct iphdr *ip4h; + + if (unlikely(!pskb_may_pull(skb, sizeof(*ip4h)))) + goto out_drop; + ip4h = ip_hdr(skb); if (!nh) { struct flowi4 fl4 = { .flowi4_flags = FLOWI_FLAG_ANYSRC,@@ -2451,15 +2459,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); - - if (unlikely(skb->mac_header >= skb->network_header)) + if (unlikely(skb_network_offset(skb) < ETH_HLEN || + !pskb_may_pull(skb, ETH_HLEN))) goto out; bpf_push_mac_rcsum(skb); - if (is_multicast_ether_addr(ethh->h_dest)) + if (is_multicast_ether_addr(eth_hdr(skb)->h_dest)) goto out; - skb_pull(skb, sizeof(*ethh)); + skb_pull(skb, ETH_HLEN); skb_unset_mac_header(skb); skb_reset_network_header(skb);pw-bot: cr