Re: [PATCH bpf-next v6 3/6] bpf: add bpf_icmp_send kfunc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The BPF kfunc `bpf_icmp_send()` originates ICMPv6 DEST_UNREACH messages for IPv6 fragments other than the first fragment, violating RFC 4443.
--

commit 7a9ef91672faeb43ff51c256f81255a263f374aa
Author: Mahe Tardy <mahe.tardy@gmail.com>

bpf: add bpf_icmp_send kfunc

Adds a bpf_icmp_send() kfunc for cgroup_skb and tc BPF programs to reply
with ICMP/ICMPv6 destination unreachable messages. It clones the packet
to safely generate the ICMP response without altering the original skb,
and includes protection against recursion.

> diff --git a/net/core/filter.c b/net/core/filter.c
> --- a/net/core/filter.c
> +++ b/net/core/filter.c

[ ... ]

> @@ -12464,6 +12466,110 @@ __bpf_kfunc int bpf_xdp_pull_data(struct xdp_md *x, u32 len)
>  	return 0;
>  }
>  
> +static DEFINE_PER_CPU(bool, bpf_icmp_send_in_progress);
> +
> +/**
> + * bpf_icmp_send - Send an ICMP control message

[ ... ]

> +__bpf_kfunc int bpf_icmp_send(struct __sk_buff *skb_ctx, int type, int code)
> +{

[ ... ]

> +#if IS_ENABLED(CONFIG_IPV6)
> +	case htons(ETH_P_IPV6):
> +		if (type != ICMPV6_DEST_UNREACH)
> +			return -EOPNOTSUPP;
> +		if (code < 0 || code > ICMPV6_REJECT_ROUTE)
> +			return -EINVAL;
> +
> +		nskb = skb_clone(skb, GFP_ATOMIC);
> +		if (!nskb)
> +			return -ENOMEM;
> +
> +		if (!pskb_network_may_pull(nskb, sizeof(struct ipv6hdr))) {
> +			kfree_skb(nskb);
> +			return -EBADMSG;
> +		}
> +
> +		if (!skb_dst(nskb) && ip6_route_reply_fill_dst(nskb) < 0) {
> +			kfree_skb(nskb);
> +			return -EHOSTUNREACH;
> +		}
> +
> +		memset(IP6CB(nskb), 0, sizeof(struct inet6_skb_parm));
> +
> +		*in_progress = true;
> +		icmpv6_send(nskb, type, code, 0);

The commit message notes that "icmp_send will only reply to fragment 0".
While IPv4's __icmp_send() explicitly drops non-initial fragments, does
icmpv6_send() provide the same guarantee?

Looking at icmpv6_send(), the internal is_ineligible() check does not seem
to prevent replying to non-initial UDP/TCP fragments. Other kernel subsystems
that originate ICMPv6 errors, such as Netfilter's nf_send_unreach6(), appear
to manually parse extension headers via ipv6_skip_exthdr() and explicitly
drop non-initial fragments.

Will calling icmpv6_send() directly here originate ICMPv6 errors for every
fragment of a dropped fragmented IPv6 packet, violating RFC 4443 Section
2.4 (e.5)?

> +		*in_progress = false;
> +		kfree_skb(nskb);
> +		break;
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518122842.218522-1-mahe.tardy@gmail.com?part=3