Re: [PATCH bpf v3 2/2] bpf: reject incompatible protocol changes
From: bot+bpf-ci@kernel.org
Date: 2026-09-30 17:26:20
Also in:
bpf, lkml
bpf: reject incompatible protocol changes
TL;DR: The family revalidation looks incomplete: LWT ingress encap (bpf_lwt_push_ip_encap, seg6) can still change L3 protocol and leave a mismatched assigned socket. Extend the check, or narrow the commit message's claim?
quoted hunk ↗ jump to hunk
diff --git a/net/core/filter.c b/net/core/filter.c index 173349bd2a3f..22e2ecb0cf2f 100644 --- a/net/core/filter.c +++ b/net/core/filter.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -3549,6 +3549,12 @@ static bool bpf_sk_assign_family_ok(const struct sk_buff *skb, return bpf_sk_assign_family_ok_proto(sk, skb_protocol(skb, true)); } +static bool bpf_skb_proto_change_sk_ok(struct sk_buff *skb, __be16 proto) +{ + return !skb_sk_is_prefetched(skb) || + bpf_sk_assign_family_ok_proto(skb->sk, proto); +} + BPF_CALL_3(bpf_skb_change_proto, struct sk_buff *, skb, __be16, proto, u64, flags) {
The commit message says "Without revalidation, this bypasses the
assignment-time family check. Reject incompatible protocol changes before
modifying the skb, including L3 encapsulation and decapsulation."
Is this revalidation complete? It is only wired into bpf_skb_change_proto(),
bpf_skb_net_grow() and bpf_skb_net_shrink(), but another path can still change
the L3 protocol of an skb that keeps its bpf_sk_assign()ed socket.
Here is one concrete sequence:
1. A tc ingress program calls bpf_sk_assign() on an IPv4 packet with an
AF_INET TCP socket. bpf_sk_assign_family_ok() passes and the program
returns TC_ACT_OK.
2. ip_rcv_core() keeps the prefetched socket:
/* Must drop socket now because of tproxy. */
if (!skb_sk_is_prefetched(skb))
skb_orphan(skb);
3. ip_rcv_finish_core() skips early demux because skb->sk is set, then calls
ip_route_input_noref(). A route with "encap bpf in" sets
dst->input = lwtunnel_input.
4. bpf_input() runs the LWT_IN program, which calls
bpf_lwt_in_push_encap(BPF_LWT_ENCAP_IP) with an IPv6 header. That reaches
bpf_lwt_push_ip_encap(..., ingress=true):
} else {
skb->protocol = htons(ETH_P_IPV6);
}
The skb->sk / sock_pfree assignment is left untouched.
BPF_LWT_ENCAP_SEG6 behaves the same way.
5. The program returns BPF_LWT_REROUTE. bpf_lwt_input_reroute() calls
ip6_route_input() and dst_input(), so the packet goes through ip6_input
and on to tcp_v6_rcv().
6. inet6_steal_sock() -> skb_steal_sock() returns the prefetched AF_INET
socket for the IPv6 packet. This is the family mismatch that
bpf_skb_proto_change_sk_ok() is meant to prevent.
In-kernel IP tunnel decapsulation in the same netns keeps the prefetched
socket across a family change in the same way. For example, with sit:
ipip6_rcv()
iptunnel_pull_header() /* sets skb->protocol = ETH_P_IPV6 */
skb_scrub_packet(skb, false) /* does not orphan */
netif_rx()
Should bpf_lwt_push_ip_encap() and bpf_push_seg6_encap() on ingress also call
bpf_skb_proto_change_sk_ok() (or orphan a prefetched socket)? Or should the
commit message narrow its claim to the two tc helpers?
This is not introduced by this patch, and every step above needs
CAP_NET_ADMIN (tc and LWT programs). But it limits the protection the patch
claims to provide.
---
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/36746681022