Re: [PATCH bpf-next v8 7/7] 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:
- [Medium] Incorrect cleanup order causes cgroup umount failure and leaks the mount state.
--

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

selftests/bpf: add bpf_icmp_send recursion test

This commit adds a new BPF selftest for bpf_icmp_send recursion handling. It
attaches a BPF program to the root cgroup to ensure that generated ICMP
packets re-trigger the BPF program, and verifies the kfunc returns -EBUSY
early.

> diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
> index 66447681f72db..fd4b8fa78a018 100644
> --- a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c

[ ... ]

> +void test_icmp_send_unreach_recursion(void)
> +{
> +	struct icmp_send *skel;
> +	int cgroup_fd = -1;

[ ... ]

> +cleanup:
> +	cleanup_cgroup_environment();
> +	icmp_send__destroy(skel);
> +	if (cgroup_fd >= 0)
> +		close(cgroup_fd);
> +}

[Severity: Medium]
Does this cleanup order cause the cgroup environment unmount to fail?

It looks like cleanup_cgroup_environment() is called while cgroup_fd is still
open and the BPF link inside skel is still active. Because cgroup_fd holds an
open file descriptor to the cgroup mount directory, calling
cleanup_cgroup_environment() first will attempt to unmount the directory while
it is busy.

Would it be better to close cgroup_fd and destroy the skeleton before cleaning
up the cgroup environment to avoid leaking the mount state?

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