-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpf: policy: fix handling of ICMPv6 packet with extension headers #24797
bpf: policy: fix handling of ICMPv6 packet with extension headers #24797
Conversation
After walking the IPv6 extension headers, ipv6_hdrlen() returns the L4 proto type in `nexthdr` parameter. If we pass in a pointer to the IPv6 header's nexthdr field, then the actual packet content is changed and subsequent processing of the packet is broken (because we treat the first IPv6 extension header as an ICMPv6 header). So even if we don't care about the L4 proto type (because we already know that it's ICMPv6), we still need to provide some stack space to store the `nexthdr`. This only becomes relevant when ENABLE_ICMP_RULE is set, which is currently controlled by a hidden agent flag. Fixes: d49311c ("policy: Add bpf ICMP policy support with the "ENABLE_ICMP_POLICY" flag") Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
/test |
Note that flag is enabled by default. |
ha, completely missed that (and doesn't make sense at all to me 😺). Thanks! |
It's a new beta feature, so we want to be able to disable it in user environments if there's a regression affecting more than just the ICMP policies. Long term, we want it to be always enabled, like other network policy features (e.g., deny policies). Also worth noting that this feature wasn't enabled until recently (unsure which exact version) because it was affected by a complexity issue before then. |
Removing from v1.11 backports given it conflicts and ICMP policies were disabled there. |
After walking the IPv6 extension headers, ipv6_hdrlen() returns the L4 proto type in
nexthdr
parameter. If we pass in a pointer to the IPv6 header's nexthdr field, then the actual packet content is changed and subsequent processing of the packet is broken (because we treat the first IPv6 extension header as an ICMPv6 header).So even if we don't care about the L4 proto type (because we already know that it's ICMPv6), we still need to provide some stack space to store the
nexthdr
.This only becomes relevant when ENABLE_ICMP_RULE is set, which is currently controlled by a hidden agent flag.
Fixes: d49311c ("policy: Add bpf ICMP policy support with the "ENABLE_ICMP_POLICY" flag")