Re: [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The BPF helper bpf_map_lookup_percpu_elem and its map-specific implementations do not verify that the requested CPU is a possible CPU, allowing out-of-bounds kernel memory reads. The patch exposes this preexisting kernel bug by changing the test to loop over `nr_cpu_ids` and call the helper on non-possible CPUs, leading to arbitrary memory reads and test failures.
--

commit c6b22dcf947502ea445cd52bb9052f6450168c31
Author: Suchit Karunakaran <suchitkarunakaran@gmail.com>

selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs

This commit updates the map_lookup_percpu_elem selftest to handle sparse CPU
topologies by reading the possible CPU mask and explicitly iterating up to
nr_cpu_ids instead of the total count of possible CPUs.

> diff --git a/tools/testing/selftests/bpf/progs/test_map_lookup_percpu_elem.c b/tools/testing/selftests/bpf/progs/test_map_lookup_percpu_elem.c
> index ca827b1092daa..d8da0696b97cf 100644
> --- a/tools/testing/selftests/bpf/progs/test_map_lookup_percpu_elem.c
> +++ b/tools/testing/selftests/bpf/progs/test_map_lookup_percpu_elem.c

[ ... ]

> @@ -57,17 +57,17 @@ int sysenter_getuid(const void *ctx)
>  
>  	map_ctx.map = &percpu_array_map;
>  	map_ctx.sum = 0;
> -	bpf_loop(nr_cpus, read_percpu_elem_callback, &map_ctx, 0);
> +	bpf_loop(nr_cpu_ids, read_percpu_elem_callback, &map_ctx, 0);

[Severity: High]
This is a pre-existing issue, but does changing this loop to iterate up to
nr_cpu_ids expose a kernel bug on sparse topologies?

When the BPF program loops over non-possible CPU IDs, it passes them into
bpf_map_lookup_percpu_elem(). The map-specific lookup implementations, such
as percpu_array_map_lookup_percpu_elem() in kernel/bpf/arraymap.c and the
hash map equivalents in kernel/bpf/hashtab.c, check if the CPU exceeds
nr_cpu_ids, but they do not verify if the CPU is actually possible:

kernel/bpf/arraymap.c:percpu_array_map_lookup_percpu_elem() {
    ...
    if (cpu >= nr_cpu_ids)
        return NULL;
    ...
}

Without a cpu_possible(cpu) check, passing a non-possible CPU ID causes the
kernel helper to retrieve an unmapped base pointer. 

When read_percpu_elem_callback() receives this pointer, it expects a NULL
return for non-possible CPUs. Instead, it gets a valid-looking pointer to
unmapped memory. Can dereferencing this pointer crash the kernel?

Should the kernel helpers be updated to validate cpu_possible() before the
selftest is modified to intentionally trigger this path?

>  	percpu_array_elem_sum = map_ctx.sum;
>  
>  	map_ctx.map = &percpu_hash_map;
>  	map_ctx.sum = 0;
> -	bpf_loop(nr_cpus, read_percpu_elem_callback, &map_ctx, 0);
> +	bpf_loop(nr_cpu_ids, read_percpu_elem_callback, &map_ctx, 0);
>  	percpu_hash_elem_sum = map_ctx.sum;
>  
>  	map_ctx.map = &percpu_lru_hash_map;
>  	map_ctx.sum = 0;
> -	bpf_loop(nr_cpus, read_percpu_elem_callback, &map_ctx, 0);
> +	bpf_loop(nr_cpu_ids, read_percpu_elem_callback, &map_ctx, 0);
>  	percpu_lru_hash_elem_sum = map_ctx.sum;
>  
>  	return 0;

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