Re: [PATCH bpf-next v3 02/15] selftests/bpf: Bind an exclusive map through a continuous fd_array

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/bpf: missing NULL check for skeleton pointer in test_map_excl_fd_array()
--

commit 1bd492fc1fe89d69931d1485e93eef135d1cd6e5
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

selftests/bpf: Bind an exclusive map through a continuous fd_array

Load the exclusive program of map_excl with the map in a continuous
fd_array rather than through its insns alone, which has to work, and
the other program the same way, which has to be refused. The two
programs used to be identical and so shared a digest; give the second
one a value of its own so that the kernel can tell them apart.

> diff --git a/tools/testing/selftests/bpf/prog_tests/map_excl.c b/tools/testing/selftests/bpf/prog_tests/map_excl.c
> index 3f4422b9ffa69..3592f3ec49490 100644
> --- a/tools/testing/selftests/bpf/prog_tests/map_excl.c
> +++ b/tools/testing/selftests/bpf/prog_tests/map_excl.c
> @@ -161,12 +161,47 @@ static void test_map_excl_create_validation(void)
>  	ASSERT_EQ(fd, -EINVAL, "reject size with NULL hash pointer");
>  }
>  
> +/*
> + * The exclusive map bound through a continuous fd_array rather than by an
> + * insn. The kernel resolves the array before it verifies the program, so it
> + * has to have the digest of the program at hand by then.
> + */
> +static void test_map_excl_fd_array(void)
> +{
> +	struct map_excl *skel = map_excl__open();
> +	LIBBPF_OPTS(bpf_prog_load_opts, opts);
> +	int err, fd, map_fd;
> +
> +	err = bpf_map__set_exclusive_program(skel->maps.excl_map, skel->progs.should_have_access);

[Severity: Medium]
Does this code need to check if skel is NULL before dereferencing it?

If map_excl__open() fails to open the BPF object (for instance, due to
ENOMEM or a missing object file), it returns a NULL pointer. By passing
skel->maps.excl_map and skel->progs.should_have_access directly to
bpf_map__set_exclusive_program(), this will cause a segmentation fault in
the test runner instead of gracefully reporting a test failure.

Should there be an ASSERT_OK_PTR(skel, "open") check here before proceeding?

> +	if (!ASSERT_OK(err, "bpf_map__set_exclusive_program"))
> +		goto out;
> +
> +	bpf_program__set_autoload(skel->progs.should_have_access, true);
> +	bpf_program__set_autoload(skel->progs.should_not_have_access, true);
> +	err = bpf_object__prepare(skel->obj);
> +	if (!ASSERT_OK(err, "map_excl prepare"))
> +		goto out;

[ ... ]

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