Re: [PATCH v2 14/15] selftests/bpf: Test the LSM policy object kfuncs with Landlock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] File descriptors leak on the error path in test_restrict_binprm_discard().
--

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

selftests/bpf: Test the LSM policy object kfuncs with Landlock

This commit adds tests for LSM policy object kfuncs interacting with Landlock.
It tests execution confinement, audit log flags, and concurrent application
using a syscall program and BPF RCU lock management.

> diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_policy_landlock.c b/tools/testing/selftests/bpf/prog_tests/lsm_policy_landlock.c
> new file mode 100644
> index 0000000000000..9270b39f5e3a7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/lsm_policy_landlock.c
[ ... ]
> +static void test_restrict_binprm_discard(void)
> +{
> +	struct policy_test_env env;
> +	char garbage_path[] = "/tmp/lsm_policy_garbage_XXXXXX";
> +	int garbage_fd, pipe_fds[2];
> +	char buf = 0;
> +	pid_t pid;
> +
> +	if (setup_env(&env))
> +		goto out;
[ ... ]
> +	if (!ASSERT_OK(pipe(pipe_fds), "pipe"))
> +		goto out_unlink;
> +
> +	/*
> +	 * Cannot use spawn_exec_child(): the same process must test its
> +	 * write access after the failed exec.
> +	 */
> +	pid = fork();
> +	if (!ASSERT_GE(pid, 0, "fork"))
> +		goto out_unlink;

[Severity: Low]
Does this code leak the file descriptors created by pipe() if fork() fails? 

If fork() fails here, the code jumps directly to the out_unlink label, and it 
appears neither pipe_fds[0] nor pipe_fds[1] are closed before exiting the
function. 

The spawn_exec_child() helper function introduced in this same commit properly 
closes both descriptors on fork failure. Should similar cleanup be added here?

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