Re: [PATCH bpf-next v9 1/5] bpf: add bpf_icmp_send kfunc

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Wed Jun 24, 2026 at 12:19 PM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing `skb_valid_dst()` check for IPv6 allows type confusion with metadata dst entries, leading to an out-of-bounds read and kernel crash.
> --
>
> commit a59a53956c9b8f69a9dfa47bc54cc9fbcd9759ec
> Author: Mahe Tardy <mahe.tardy@gmail.com>
>
> bpf: add bpf_icmp_send kfunc
>
> This commit introduces a new bpf_icmp_send kfunc to allow BPF programs to
> generate ICMP error messages. It clones the original skb and invokes the
> underlying IPv4 or IPv6 ICMP send routines.
>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 2e96b4b847ce1..f3aa494ed1053 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>
> [ ... ]
>
>> +__bpf_kfunc int bpf_icmp_send(struct __sk_buff *skb_ctx, int type, int code)
>> +{
>> +	struct sk_buff *skb = (struct sk_buff *)skb_ctx;
>> +	struct sk_buff *nskb;
>> +	struct sock *sk;
>
> [ ... ]
>
>> +	switch (skb->protocol) {
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case htons(ETH_P_IP): {
>> +		if (type != ICMP_DEST_UNREACH)
>> +			return -EOPNOTSUPP;
>> +		if (code < 0 || code > NR_ICMP_UNREACH ||
>> +		    code == ICMP_FRAG_NEEDED) /* needs a valid next-hop MTU */
>> +			return -EINVAL;
>> +
>> +		/* icmp_send requires a rtable; test-run synthetic skbs lack one. */
>> +		if (!skb_valid_dst(skb))
>> +			return -ENETUNREACH;
>> +
>> +		nskb = skb_clone(skb, GFP_ATOMIC);
>
> [ ... ]
>
>> +#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;
>> +
>
> [Severity: High]
> Should there be a check for skb_valid_dst(skb) here before cloning and
> sending the IPv6 ICMP message?
>
> It looks like the IPv4 branch validates the dst to prevent routing table
> lookups on synthetic or metadata-dst packets, but the IPv6 branch does not.
>
> If an IPv6 packet with a metadata_dst (e.g., from LWT or a tunnel) is
> passed to bpf_icmp_send(), could icmpv6_send() process the invalid dst and
> cast it to a struct rt6_info in icmp6_dev()? Dereferencing rt6->rt6i_idev
> might then read out of bounds from the metadata_dst struct and cause a
> kernel crash.

Sounds like a real issue.

Mahe,
when you respin please cc netdev.
We need an ack from networking maintainers.

pw-bot: cr