Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for map-stored struct file kptrs

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Wrapping `RUN_TESTS()` inside `test__start_subtest()` breaks subtest filtering and creates dummy subtests.
- [Low] Zombie process leak if an error occurs before `waitpid()`.
--

commit a0fbd925d86bff9e8b40a48dc8cff998dbbb50fc
Author: Justin Suess <utilityemal77@gmail.com>

selftests/bpf: Add test for map-stored struct file kptrs

This commit adds tests for map-stored referenced struct file pointers. It
verifies that the file reference remains valid after the original file
descriptor is closed, and checks that unreferenced struct file pointers are
rejected by the verifier.

> diff --git a/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
> new file mode 100644
> index 0000000000000..173121377040c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/refcounted_kptr_file.c
[ ... ]
> +	skel->bss->file_kptr_insert_pid = child_pid;
> +	err = write_full(pipefd[1], "1", 1);
> +	if (!ASSERT_OK(err, "start_child"))
> +		goto out;

If an error occurs here and the code jumps to the out label, does it leave
the child process as a zombie?

It looks like the cleanup path closes the pipe file descriptors which causes
the child to read EOF and exit, but waitpid() isn't called in the cleanup
path.

[ ... ]
> +void test_refcounted_kptr_file(void)
> +{
> +	if (test__start_subtest("holds_ref_after_close"))
> +		run_refcounted_file_kptr_success();
> +
> +	if (test__start_subtest("reject_unref_ctx_file"))
> +		RUN_TESTS(refcounted_kptr_file_fail);
> +}

Can wrapping RUN_TESTS() inside test__start_subtest() break test filtering?

RUN_TESTS() internally parses the skeleton and creates its own subtests for
each BPF program by calling test__start_subtest_with_desc().

Since the selftests framework doesn't support nested subtests, if a user
filters for the inner program name (like -t refcounted_kptr_file/stash_unref_ctx_file),
the outer test__start_subtest() will not match the filter and will skip
RUN_TESTS() completely.

Should RUN_TESTS() be called unconditionally at the top level of the test
function instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260420203306.3107246-1-utilityemal77@gmail.com?part=2