Re: [PATCH bpf v3 3/3] selftests/bpf: add tests for percpu map flags combination

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: test falsely passes via libbpf size validation masking buffer under-allocation
--

commit 1b911e2bbe76c9b313409a279ec05f4a4e42cb74
Author: Masoud Aghasi <maghasi@disroot.org>

selftests/bpf: add tests for percpu map flags combination

This commit adds tests for all possible combinations of (BPF_EXIST,
BPF_NOEXIST) and (BPF_F_ALL_CPUS, BPF_F_CPU) flags across percpu map types.
It also merges testing of percpu cgroup storage into the existing
cpu_flag_percpu_cgroup_storage test.

> diff --git a/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c b/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c
> index 7b4a1e24363b5..f50f51c91164b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c
> @@ -449,6 +449,37 @@ static void test_lru_percpu_hash_cpu_flag_create(void)
>  	test_percpu_map_cpu_flag_create(BPF_MAP_TYPE_LRU_PERCPU_HASH, 0);
>  }
>  
> +static void test_percpu_cgroup_storage_flags_combination(struct bpf_map *map, int nr_cpus,
> +							  struct bpf_cgroup_storage_key *key)
> +{
> +	int err;
> +	u64 flags = 0;
> +	size_t value_sz = sizeof(u32);
> +	u32 *values = NULL;
> +
> +	values = calloc(nr_cpus, value_sz);

[Severity: Medium]
The values array is only allocated 4 * nr_cpus bytes, whereas a valid percpu
map update requires 8 * nr_cpus bytes. If libbpf's size check were bypassed
(e.g., by directly using the bpf() syscall like the other tests do), would
this buffer under-allocation result in an out-of-bounds read when the kernel
attempts to copy 8 * nr_cpus bytes?

> +	if (!ASSERT_OK_PTR(values, "calloc values"))
> +		return;
> +
> +	flags = BPF_NOEXIST | BPF_EXIST;
> +	err = bpf_map__update_elem(map, key, sizeof(*key), values, value_sz, flags);
> +	if (!ASSERT_EQ(err, -EINVAL, "bpf_map__update_elem noexist|exist"))
> +		goto out;

[Severity: Medium]
Does this test incorrectly pass because libbpf intercepts the invalid value_sz,
returning -EINVAL before reaching the kernel? 

validate_map_op() in libbpf expects value_sz to be nr_cpus * 8 because neither
BPF_F_CPU nor BPF_F_ALL_CPUS is set. This seems to mask the fact that the
kernel's map_update_elem syscall is never actually exercised for this specific
flag combination.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003160213.2641506-1-maghasi@disroot.org?part=3