Re: [PATCH bpf-next v6 6/6] selftests/bpf: add bpf_icmp_send recursion test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The BPF program `recursion` acts as a global drop-all firewall for host IPv4 traffic, causing severe network disruption and test flakiness.
--

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

selftests/bpf: add bpf_icmp_send recursion test

This commit adds a test to verify that the bpf_icmp_send kfunc stops early
and returns -EBUSY when triggered recursively. The test attaches a BPF
program to the root cgroup to intercept the kernel-generated ICMP packet.

> diff --git a/tools/testing/selftests/bpf/progs/icmp_send.c b/tools/testing/selftests/bpf/progs/icmp_send.c
> index 6e1ba539eeb0f..7830334b747ac 100644
> --- a/tools/testing/selftests/bpf/progs/icmp_send.c
> +++ b/tools/testing/selftests/bpf/progs/icmp_send.c

[ ... ]

> @@ -65,4 +68,32 @@ int egress(struct __sk_buff *skb)
>  	return SK_DROP;
>  }
>  
> +SEC("cgroup_skb/egress")
> +int recursion(struct __sk_buff *skb)
> +{
> +	void *data = (void *)(long)skb->data;
> +	void *data_end = (void *)(long)skb->data_end;
> +	struct iphdr *iph;
> +	int ret;
> +
> +	iph = data;
> +	if ((void *)(iph + 1) > data_end || iph->version != 4)
> +		return SK_PASS;
> +
> +	/* This call will provoke a recursion: the ICMP packet generated by the
> +	 * kfunc will re-trigger this program since we are in the root cgroup in
> +	 * which the kernel ICMP socket belongs. However when re-entering the
> +	 * kfunc, it should return EBUSY.
> +	 */
> +	ret = bpf_icmp_send(skb, unreach_type, unreach_code);
> +	rec_kfunc_rets[rec_count & 1] = ret;
> +	__sync_fetch_and_add(&rec_count, 1);
> +
> +	/* Let the first ICMP error message pass */
> +	if (iph->protocol == IPPROTO_ICMP)
> +		return SK_PASS;
> +
> +	return SK_DROP;
> +}

Since this program is attached to the global root cgroup, does it act as a
global drop-all firewall for host IPv4 traffic? 

If any background traffic occurs while this test runs, it appears it will be
unconditionally dropped here.

Could this also cause test flakiness? If concurrent background traffic
triggers this program, it would race with the test traffic and overwrite the
rec_kfunc_rets array, leading to unexpected values in the test assertions.

Should the program explicitly filter packets, for example by checking
iph->daddr or iph->protocol, to ensure it only processes the test's intended
traffic?

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