* Re: [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs
2026-09-01 14:19 [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs Suchit Karunakaran
@ 2026-09-01 14:36 ` sashiko-bot
2026-09-01 15:34 ` bot+bpf-ci
2026-09-03 0:32 ` Andrii Nakryiko
2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-01 14:36 UTC (permalink / raw)
To: Suchit Karunakaran; +Cc: bpf
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs
2026-09-01 14:19 [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs Suchit Karunakaran
2026-09-01 14:36 ` sashiko-bot
@ 2026-09-01 15:34 ` bot+bpf-ci
2026-09-01 16:42 ` Suchit Karunakaran
2026-09-03 0:32 ` Andrii Nakryiko
2 siblings, 1 reply; 8+ messages in thread
From: bot+bpf-ci @ 2026-09-01 15:34 UTC (permalink / raw)
To: suchitkarunakaran, andrii, ast, daniel, eddyz87, memxor, shuah
Cc: emil, ihor.solodrai, jolsa, martin.lau, song, yonghong.song,
zhoufeng.zf, bpf, linux-kselftest, linux-kernel,
suchitkarunakaran, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3689 bytes --]
> diff --git a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> index bfb1bf3fd427b..849f33259c003 100644
> --- a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> +++ b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> @@ -2,29 +2,45 @@
> /* Copyright (c) 2022 Bytedance */
>
> #include <test_progs.h>
> +#include "bpf/libbpf_internal.h"
> #include "test_map_lookup_percpu_elem.skel.h"
>
> void test_map_lookup_percpu_elem(void)
> {
> struct test_map_lookup_percpu_elem *skel;
> - __u64 key = 0, sum;
> - int ret, i, nr_cpus = libbpf_num_possible_cpus();
> + bool *possible = NULL;
> + __u64 key = 0, sum = 0;
> + int cpu, nr_cpu_ids, nr_cpus, ret, slot = 0;
> __u64 *buf;
>
> - buf = malloc(nr_cpus*sizeof(__u64));
> - if (!ASSERT_OK_PTR(buf, "malloc"))
> + ret = parse_cpu_mask_file("/sys/devices/system/cpu/possible", &possible,
> + &nr_cpu_ids);
> + if (!ASSERT_OK(ret, "parse possible CPU mask"))
> return;
>
> - for (i = 0; i < nr_cpus; i++)
> - buf[i] = i;
> - sum = (nr_cpus - 1) * nr_cpus / 2;
> + nr_cpus = libbpf_num_possible_cpus();
> + if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
> + goto free_mask;
> +
> + buf = malloc(nr_cpus * sizeof(*buf));
> + if (!ASSERT_OK_PTR(buf, "malloc"))
> + goto free_mask;
> +
> + for (cpu = 0; cpu < nr_cpu_ids; cpu++) {
> + if (!possible[cpu])
> + continue;
> + buf[slot++] = cpu;
> + sum += cpu;
> + }
Does this buffer layout match how the kernel expects it?
The userspace side fills buf[] with one entry per possible CPU, storing
the logical CPU ID at packed index 'slot' (0, 1, 2, ...). For a mask
like 0,2-3, this creates buf = {0, 2, 3} with 3 entries total.
Looking at the kernel's update path, both percpu array and hash maps
index the user buffer by logical CPU ID:
kernel/bpf/arraymap.c:469 bpf_percpu_array_update()
for_each_possible_cpu(cpu) {
ptr = per_cpu_ptr(pptr, cpu);
val = (map_flags & BPF_F_ALL_CPUS) ? value : value + size * cpu;
kernel/bpf/hashtab.c:1038 pcpu_copy_value()
for_each_possible_cpu(cpu) {
...
copy_map_value_long(&htab->map, per_cpu_ptr(pptrs, cpu),
value + off + size * cpu);
With the mask 0,2-3 (num_possible_cpus() = 3, nr_cpu_ids = 4), the
update walks possible CPUs {0, 2, 3} and indexes the buffer as:
cpu 0: *(value + 0) = buf[0] = 0 (intended)
cpu 2: *(value + 16) = buf[2] = 3 (expects 2, gets 3)
cpu 3: *(value + 24) (past the 24-byte buffer end)
The userspace layout is packed (3 entries), but the kernel accesses it
as sparse (indexed by CPU ID). The test sized the buffer for
num_possible_cpus() bytes, but the kernel's 'size * cpu' indexing can
exceed that when the possible mask has holes.
Should the userspace buffer either be laid out by CPU ID (sized for
nr_cpu_ids, with holes at non-possible indices), or should the kernel's
update path use packed indexing like the lookup direction does?
> + if (!ASSERT_EQ(slot, nr_cpus, "possible CPU mask weight"))
> + goto exit;
>
> skel = test_map_lookup_percpu_elem__open();
> if (!ASSERT_OK_PTR(skel, "test_map_lookup_percpu_elem__open"))
> goto exit;
>
> skel->rodata->my_pid = getpid();
> - skel->rodata->nr_cpus = nr_cpus;
> + skel->rodata->nr_cpu_ids = nr_cpu_ids;
>
> ret = test_map_lookup_percpu_elem__load(skel);
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33521264909
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs
2026-09-01 15:34 ` bot+bpf-ci
@ 2026-09-01 16:42 ` Suchit Karunakaran
0 siblings, 0 replies; 8+ messages in thread
From: Suchit Karunakaran @ 2026-09-01 16:42 UTC (permalink / raw)
To: bot+bpf-ci
Cc: andrii, ast, daniel, eddyz87, memxor, shuah, emil, ihor.solodrai,
jolsa, martin.lau, song, yonghong.song, zhoufeng.zf, bpf,
linux-kselftest, linux-kernel, martin.lau, mason
On Tue, 1 Sept 2026 at 21:04, <bot+bpf-ci@kernel.org> wrote:
>
> > diff --git a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > index bfb1bf3fd427b..849f33259c003 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > @@ -2,29 +2,45 @@
> > /* Copyright (c) 2022 Bytedance */
> >
> > #include <test_progs.h>
> > +#include "bpf/libbpf_internal.h"
> > #include "test_map_lookup_percpu_elem.skel.h"
> >
> > void test_map_lookup_percpu_elem(void)
> > {
> > struct test_map_lookup_percpu_elem *skel;
> > - __u64 key = 0, sum;
> > - int ret, i, nr_cpus = libbpf_num_possible_cpus();
> > + bool *possible = NULL;
> > + __u64 key = 0, sum = 0;
> > + int cpu, nr_cpu_ids, nr_cpus, ret, slot = 0;
> > __u64 *buf;
> >
> > - buf = malloc(nr_cpus*sizeof(__u64));
> > - if (!ASSERT_OK_PTR(buf, "malloc"))
> > + ret = parse_cpu_mask_file("/sys/devices/system/cpu/possible", &possible,
> > + &nr_cpu_ids);
> > + if (!ASSERT_OK(ret, "parse possible CPU mask"))
> > return;
> >
> > - for (i = 0; i < nr_cpus; i++)
> > - buf[i] = i;
> > - sum = (nr_cpus - 1) * nr_cpus / 2;
> > + nr_cpus = libbpf_num_possible_cpus();
> > + if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
> > + goto free_mask;
> > +
> > + buf = malloc(nr_cpus * sizeof(*buf));
> > + if (!ASSERT_OK_PTR(buf, "malloc"))
> > + goto free_mask;
> > +
> > + for (cpu = 0; cpu < nr_cpu_ids; cpu++) {
> > + if (!possible[cpu])
> > + continue;
> > + buf[slot++] = cpu;
> > + sum += cpu;
> > + }
>
> Does this buffer layout match how the kernel expects it?
>
> The userspace side fills buf[] with one entry per possible CPU, storing
> the logical CPU ID at packed index 'slot' (0, 1, 2, ...). For a mask
> like 0,2-3, this creates buf = {0, 2, 3} with 3 entries total.
>
> Looking at the kernel's update path, both percpu array and hash maps
> index the user buffer by logical CPU ID:
>
> kernel/bpf/arraymap.c:469 bpf_percpu_array_update()
> for_each_possible_cpu(cpu) {
> ptr = per_cpu_ptr(pptr, cpu);
> val = (map_flags & BPF_F_ALL_CPUS) ? value : value + size * cpu;
>
> kernel/bpf/hashtab.c:1038 pcpu_copy_value()
> for_each_possible_cpu(cpu) {
> ...
> copy_map_value_long(&htab->map, per_cpu_ptr(pptrs, cpu),
> value + off + size * cpu);
>
> With the mask 0,2-3 (num_possible_cpus() = 3, nr_cpu_ids = 4), the
> update walks possible CPUs {0, 2, 3} and indexes the buffer as:
>
> cpu 0: *(value + 0) = buf[0] = 0 (intended)
> cpu 2: *(value + 16) = buf[2] = 3 (expects 2, gets 3)
> cpu 3: *(value + 24) (past the 24-byte buffer end)
>
> The userspace layout is packed (3 entries), but the kernel accesses it
> as sparse (indexed by CPU ID). The test sized the buffer for
> num_possible_cpus() bytes, but the kernel's 'size * cpu' indexing can
> exceed that when the possible mask has holes.
>
> Should the userspace buffer either be laid out by CPU ID (sized for
> nr_cpu_ids, with holes at non-possible indices), or should the kernel's
> update path use packed indexing like the lookup direction does?
>
> > + if (!ASSERT_EQ(slot, nr_cpus, "possible CPU mask weight"))
> > + goto exit;
> >
> > skel = test_map_lookup_percpu_elem__open();
> > if (!ASSERT_OK_PTR(skel, "test_map_lookup_percpu_elem__open"))
> > goto exit;
> >
> > skel->rodata->my_pid = getpid();
> > - skel->rodata->nr_cpus = nr_cpus;
> > + skel->rodata->nr_cpu_ids = nr_cpu_ids;
> >
> > ret = test_map_lookup_percpu_elem__load(skel);
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33521264909
For context on the review, there's a commit that's directly relevant
here but isn't in the mainline or bpf-next tree yet, so flagging it
explicitly:
Commit: 75b0a6db4300e4c2c9e97a0848deaa7acfb42fb7 ("bpf: Fix percpu map
update indexing with sparse CPU IDs")
That commit changes the array, hash, and cgroup-storage update paths
from value + size * cpu to a dense running offset, matching the lookup
side. With it, a possible mask of 0,2-3 maps the packed buffer as:
slot 0 -> CPU 0
slot 1 -> CPU 2
slot 2 -> CPU 3
I developed this selftest change on top of that commit but failed to
state the dependency.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs
2026-09-01 14:19 [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs Suchit Karunakaran
2026-09-01 14:36 ` sashiko-bot
2026-09-01 15:34 ` bot+bpf-ci
@ 2026-09-03 0:32 ` Andrii Nakryiko
2026-09-03 5:29 ` Suchit Karunakaran
2 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2026-09-03 0:32 UTC (permalink / raw)
To: Suchit Karunakaran
Cc: andrii, ast, daniel, eddyz87, memxor, shuah, emil, ihor.solodrai,
jolsa, martin.lau, song, yonghong.song, zhoufeng.zf, bpf,
linux-kselftest, linux-kernel
On Tue, Sep 1, 2026 at 7:20 AM Suchit Karunakaran
<suchitkarunakaran@gmail.com> wrote:
>
> libbpf_num_possible_cpus() returns the number of possible CPUs, which is
> appropriate for sizing packed per-CPU map value buffers. It is not the
> upper bound for logical CPU IDs.
>
> For a possible CPU mask such as 0,2-3, the test loops over CPU IDs 0
> through 2. This incorrectly visits CPU 1 and misses CPU 3. It also
> initializes packed per-CPU slots using their slot indexes rather than
> the corresponding logical CPU IDs.
>
> Parse the possible CPU mask and keep the packed slot count separate
> from the logical CPU ID range. Populate each dense per-CPU slot with
> its logical CPU ID, calculate the corresponding expected sum, and make
> the BPF program iterate over the full CPU ID range.
>
Is this the issue you ran into in practice or it's just another of
those found by AI reading code?
> Fixes: 7aa424e02a04bba5ecc84afe9b58b16e9e0b34f8 ("selftests/bpf: Fix some bugs in map_lookup_percpu_elem testcase")
>
> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> ---
> .../bpf/prog_tests/map_lookup_percpu_elem.c | 34 ++++++++++++++-----
> .../bpf/progs/test_map_lookup_percpu_elem.c | 8 ++---
> 2 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> index bfb1bf3fd427..849f33259c00 100644
> --- a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> +++ b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> @@ -2,29 +2,45 @@
> /* Copyright (c) 2022 Bytedance */
>
> #include <test_progs.h>
> +#include "bpf/libbpf_internal.h"
> #include "test_map_lookup_percpu_elem.skel.h"
>
> void test_map_lookup_percpu_elem(void)
> {
> struct test_map_lookup_percpu_elem *skel;
> - __u64 key = 0, sum;
> - int ret, i, nr_cpus = libbpf_num_possible_cpus();
> + bool *possible = NULL;
> + __u64 key = 0, sum = 0;
> + int cpu, nr_cpu_ids, nr_cpus, ret, slot = 0;
> __u64 *buf;
>
> - buf = malloc(nr_cpus*sizeof(__u64));
> - if (!ASSERT_OK_PTR(buf, "malloc"))
> + ret = parse_cpu_mask_file("/sys/devices/system/cpu/possible", &possible,
> + &nr_cpu_ids);
> + if (!ASSERT_OK(ret, "parse possible CPU mask"))
> return;
>
> - for (i = 0; i < nr_cpus; i++)
> - buf[i] = i;
> - sum = (nr_cpus - 1) * nr_cpus / 2;
> + nr_cpus = libbpf_num_possible_cpus();
> + if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
> + goto free_mask;
> +
> + buf = malloc(nr_cpus * sizeof(*buf));
> + if (!ASSERT_OK_PTR(buf, "malloc"))
> + goto free_mask;
> +
> + for (cpu = 0; cpu < nr_cpu_ids; cpu++) {
> + if (!possible[cpu])
> + continue;
> + buf[slot++] = cpu;
> + sum += cpu;
> + }
> + if (!ASSERT_EQ(slot, nr_cpus, "possible CPU mask weight"))
> + goto exit;
>
> skel = test_map_lookup_percpu_elem__open();
> if (!ASSERT_OK_PTR(skel, "test_map_lookup_percpu_elem__open"))
> goto exit;
>
> skel->rodata->my_pid = getpid();
> - skel->rodata->nr_cpus = nr_cpus;
> + skel->rodata->nr_cpu_ids = nr_cpu_ids;
>
> ret = test_map_lookup_percpu_elem__load(skel);
> if (!ASSERT_OK(ret, "test_map_lookup_percpu_elem__load"))
> @@ -55,4 +71,6 @@ void test_map_lookup_percpu_elem(void)
> test_map_lookup_percpu_elem__destroy(skel);
> exit:
> free(buf);
> +free_mask:
> + free(possible);
> }
> 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 ca827b1092da..d8da0696b97c 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
> @@ -7,7 +7,7 @@
> __u64 percpu_array_elem_sum = 0;
> __u64 percpu_hash_elem_sum = 0;
> __u64 percpu_lru_hash_elem_sum = 0;
> -const volatile int nr_cpus;
> +const volatile int nr_cpu_ids;
> const volatile int my_pid;
>
> struct {
> @@ -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);
> 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;
> --
> 2.55.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs
2026-09-03 0:32 ` Andrii Nakryiko
@ 2026-09-03 5:29 ` Suchit Karunakaran
2026-09-03 16:01 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Suchit Karunakaran @ 2026-09-03 5:29 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: andrii, ast, daniel, eddyz87, memxor, shuah, emil, ihor.solodrai,
jolsa, martin.lau, song, yonghong.song, zhoufeng.zf, bpf,
linux-kselftest, linux-kernel
On Thu, 3 Sept 2026 at 06:02, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Sep 1, 2026 at 7:20 AM Suchit Karunakaran
> <suchitkarunakaran@gmail.com> wrote:
> >
> > libbpf_num_possible_cpus() returns the number of possible CPUs, which is
> > appropriate for sizing packed per-CPU map value buffers. It is not the
> > upper bound for logical CPU IDs.
> >
> > For a possible CPU mask such as 0,2-3, the test loops over CPU IDs 0
> > through 2. This incorrectly visits CPU 1 and misses CPU 3. It also
> > initializes packed per-CPU slots using their slot indexes rather than
> > the corresponding logical CPU IDs.
> >
> > Parse the possible CPU mask and keep the packed slot count separate
> > from the logical CPU ID range. Populate each dense per-CPU slot with
> > its logical CPU ID, calculate the corresponding expected sum, and make
> > the BPF program iterate over the full CPU ID range.
> >
>
> Is this the issue you ran into in practice or it's just another of
> those found by AI reading code?
>
>
> > Fixes: 7aa424e02a04bba5ecc84afe9b58b16e9e0b34f8 ("selftests/bpf: Fix some bugs in map_lookup_percpu_elem testcase")
> >
> > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > ---
> > .../bpf/prog_tests/map_lookup_percpu_elem.c | 34 ++++++++++++++-----
> > .../bpf/progs/test_map_lookup_percpu_elem.c | 8 ++---
> > 2 files changed, 30 insertions(+), 12 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > index bfb1bf3fd427..849f33259c00 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > @@ -2,29 +2,45 @@
> > /* Copyright (c) 2022 Bytedance */
> >
> > #include <test_progs.h>
> > +#include "bpf/libbpf_internal.h"
> > #include "test_map_lookup_percpu_elem.skel.h"
> >
> > void test_map_lookup_percpu_elem(void)
> > {
> > struct test_map_lookup_percpu_elem *skel;
> > - __u64 key = 0, sum;
> > - int ret, i, nr_cpus = libbpf_num_possible_cpus();
> > + bool *possible = NULL;
> > + __u64 key = 0, sum = 0;
> > + int cpu, nr_cpu_ids, nr_cpus, ret, slot = 0;
> > __u64 *buf;
> >
> > - buf = malloc(nr_cpus*sizeof(__u64));
> > - if (!ASSERT_OK_PTR(buf, "malloc"))
> > + ret = parse_cpu_mask_file("/sys/devices/system/cpu/possible", &possible,
> > + &nr_cpu_ids);
> > + if (!ASSERT_OK(ret, "parse possible CPU mask"))
> > return;
> >
> > - for (i = 0; i < nr_cpus; i++)
> > - buf[i] = i;
> > - sum = (nr_cpus - 1) * nr_cpus / 2;
> > + nr_cpus = libbpf_num_possible_cpus();
> > + if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
> > + goto free_mask;
> > +
> > + buf = malloc(nr_cpus * sizeof(*buf));
> > + if (!ASSERT_OK_PTR(buf, "malloc"))
> > + goto free_mask;
> > +
> > + for (cpu = 0; cpu < nr_cpu_ids; cpu++) {
> > + if (!possible[cpu])
> > + continue;
> > + buf[slot++] = cpu;
> > + sum += cpu;
> > + }
> > + if (!ASSERT_EQ(slot, nr_cpus, "possible CPU mask weight"))
> > + goto exit;
> >
> > skel = test_map_lookup_percpu_elem__open();
> > if (!ASSERT_OK_PTR(skel, "test_map_lookup_percpu_elem__open"))
> > goto exit;
> >
> > skel->rodata->my_pid = getpid();
> > - skel->rodata->nr_cpus = nr_cpus;
> > + skel->rodata->nr_cpu_ids = nr_cpu_ids;
> >
> > ret = test_map_lookup_percpu_elem__load(skel);
> > if (!ASSERT_OK(ret, "test_map_lookup_percpu_elem__load"))
> > @@ -55,4 +71,6 @@ void test_map_lookup_percpu_elem(void)
> > test_map_lookup_percpu_elem__destroy(skel);
> > exit:
> > free(buf);
> > +free_mask:
> > + free(possible);
> > }
> > 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 ca827b1092da..d8da0696b97c 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
> > @@ -7,7 +7,7 @@
> > __u64 percpu_array_elem_sum = 0;
> > __u64 percpu_hash_elem_sum = 0;
> > __u64 percpu_lru_hash_elem_sum = 0;
> > -const volatile int nr_cpus;
> > +const volatile int nr_cpu_ids;
> > const volatile int my_pid;
> >
> > struct {
> > @@ -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);
> > 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;
> > --
> > 2.55.0
> >
Hi Andrii, I did use AI to find this. I often go through recent
commits and then ask AI to check whether the related selftests have
been updated and that's how I found this.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs
2026-09-03 5:29 ` Suchit Karunakaran
@ 2026-09-03 16:01 ` Andrii Nakryiko
2026-09-03 16:57 ` Suchit Karunakaran
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2026-09-03 16:01 UTC (permalink / raw)
To: Suchit Karunakaran
Cc: andrii, ast, daniel, eddyz87, memxor, shuah, emil, ihor.solodrai,
jolsa, martin.lau, song, yonghong.song, zhoufeng.zf, bpf,
linux-kselftest, linux-kernel
On Wed, Sep 2, 2026 at 10:29 PM Suchit Karunakaran
<suchitkarunakaran@gmail.com> wrote:
>
> On Thu, 3 Sept 2026 at 06:02, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Tue, Sep 1, 2026 at 7:20 AM Suchit Karunakaran
> > <suchitkarunakaran@gmail.com> wrote:
> > >
> > > libbpf_num_possible_cpus() returns the number of possible CPUs, which is
> > > appropriate for sizing packed per-CPU map value buffers. It is not the
> > > upper bound for logical CPU IDs.
> > >
> > > For a possible CPU mask such as 0,2-3, the test loops over CPU IDs 0
> > > through 2. This incorrectly visits CPU 1 and misses CPU 3. It also
> > > initializes packed per-CPU slots using their slot indexes rather than
> > > the corresponding logical CPU IDs.
> > >
> > > Parse the possible CPU mask and keep the packed slot count separate
> > > from the logical CPU ID range. Populate each dense per-CPU slot with
> > > its logical CPU ID, calculate the corresponding expected sum, and make
> > > the BPF program iterate over the full CPU ID range.
> > >
> >
> > Is this the issue you ran into in practice or it's just another of
> > those found by AI reading code?
> >
> >
> > > Fixes: 7aa424e02a04bba5ecc84afe9b58b16e9e0b34f8 ("selftests/bpf: Fix some bugs in map_lookup_percpu_elem testcase")
> > >
> > > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > > ---
> > > .../bpf/prog_tests/map_lookup_percpu_elem.c | 34 ++++++++++++++-----
> > > .../bpf/progs/test_map_lookup_percpu_elem.c | 8 ++---
> > > 2 files changed, 30 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > > index bfb1bf3fd427..849f33259c00 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > > @@ -2,29 +2,45 @@
> > > /* Copyright (c) 2022 Bytedance */
> > >
> > > #include <test_progs.h>
> > > +#include "bpf/libbpf_internal.h"
> > > #include "test_map_lookup_percpu_elem.skel.h"
> > >
> > > void test_map_lookup_percpu_elem(void)
> > > {
> > > struct test_map_lookup_percpu_elem *skel;
> > > - __u64 key = 0, sum;
> > > - int ret, i, nr_cpus = libbpf_num_possible_cpus();
> > > + bool *possible = NULL;
> > > + __u64 key = 0, sum = 0;
> > > + int cpu, nr_cpu_ids, nr_cpus, ret, slot = 0;
> > > __u64 *buf;
> > >
> > > - buf = malloc(nr_cpus*sizeof(__u64));
> > > - if (!ASSERT_OK_PTR(buf, "malloc"))
> > > + ret = parse_cpu_mask_file("/sys/devices/system/cpu/possible", &possible,
> > > + &nr_cpu_ids);
> > > + if (!ASSERT_OK(ret, "parse possible CPU mask"))
> > > return;
> > >
> > > - for (i = 0; i < nr_cpus; i++)
> > > - buf[i] = i;
> > > - sum = (nr_cpus - 1) * nr_cpus / 2;
> > > + nr_cpus = libbpf_num_possible_cpus();
> > > + if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
> > > + goto free_mask;
> > > +
> > > + buf = malloc(nr_cpus * sizeof(*buf));
> > > + if (!ASSERT_OK_PTR(buf, "malloc"))
> > > + goto free_mask;
> > > +
> > > + for (cpu = 0; cpu < nr_cpu_ids; cpu++) {
> > > + if (!possible[cpu])
> > > + continue;
> > > + buf[slot++] = cpu;
> > > + sum += cpu;
> > > + }
> > > + if (!ASSERT_EQ(slot, nr_cpus, "possible CPU mask weight"))
> > > + goto exit;
> > >
> > > skel = test_map_lookup_percpu_elem__open();
> > > if (!ASSERT_OK_PTR(skel, "test_map_lookup_percpu_elem__open"))
> > > goto exit;
> > >
> > > skel->rodata->my_pid = getpid();
> > > - skel->rodata->nr_cpus = nr_cpus;
> > > + skel->rodata->nr_cpu_ids = nr_cpu_ids;
> > >
> > > ret = test_map_lookup_percpu_elem__load(skel);
> > > if (!ASSERT_OK(ret, "test_map_lookup_percpu_elem__load"))
> > > @@ -55,4 +71,6 @@ void test_map_lookup_percpu_elem(void)
> > > test_map_lookup_percpu_elem__destroy(skel);
> > > exit:
> > > free(buf);
> > > +free_mask:
> > > + free(possible);
> > > }
> > > 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 ca827b1092da..d8da0696b97c 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
> > > @@ -7,7 +7,7 @@
> > > __u64 percpu_array_elem_sum = 0;
> > > __u64 percpu_hash_elem_sum = 0;
> > > __u64 percpu_lru_hash_elem_sum = 0;
> > > -const volatile int nr_cpus;
> > > +const volatile int nr_cpu_ids;
> > > const volatile int my_pid;
> > >
> > > struct {
> > > @@ -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);
> > > 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;
> > > --
> > > 2.55.0
> > >
>
> Hi Andrii, I did use AI to find this. I often go through recent
> commits and then ask AI to check whether the related selftests have
> been updated and that's how I found this.
in that case I'm inclined to leave selftest as is. This set up where
possible cpu mask is not contiguous seems to be some fancy qemu-based
setups so far, and frankly a bunch of other code is probably broken in
such situations anyways. let's keep things as is for now
pw-bot: cr
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] selftests/bpf: Fix map_lookup_percpu_elem on sparse CPU IDs
2026-09-03 16:01 ` Andrii Nakryiko
@ 2026-09-03 16:57 ` Suchit Karunakaran
0 siblings, 0 replies; 8+ messages in thread
From: Suchit Karunakaran @ 2026-09-03 16:57 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: andrii, ast, daniel, eddyz87, memxor, shuah, emil, ihor.solodrai,
jolsa, martin.lau, song, yonghong.song, zhoufeng.zf, bpf,
linux-kselftest, linux-kernel
On Thu, 3 Sept 2026 at 21:31, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Sep 2, 2026 at 10:29 PM Suchit Karunakaran
> <suchitkarunakaran@gmail.com> wrote:
> >
> > On Thu, 3 Sept 2026 at 06:02, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Tue, Sep 1, 2026 at 7:20 AM Suchit Karunakaran
> > > <suchitkarunakaran@gmail.com> wrote:
> > > >
> > > > libbpf_num_possible_cpus() returns the number of possible CPUs, which is
> > > > appropriate for sizing packed per-CPU map value buffers. It is not the
> > > > upper bound for logical CPU IDs.
> > > >
> > > > For a possible CPU mask such as 0,2-3, the test loops over CPU IDs 0
> > > > through 2. This incorrectly visits CPU 1 and misses CPU 3. It also
> > > > initializes packed per-CPU slots using their slot indexes rather than
> > > > the corresponding logical CPU IDs.
> > > >
> > > > Parse the possible CPU mask and keep the packed slot count separate
> > > > from the logical CPU ID range. Populate each dense per-CPU slot with
> > > > its logical CPU ID, calculate the corresponding expected sum, and make
> > > > the BPF program iterate over the full CPU ID range.
> > > >
> > >
> > > Is this the issue you ran into in practice or it's just another of
> > > those found by AI reading code?
> > >
> > >
> > > > Fixes: 7aa424e02a04bba5ecc84afe9b58b16e9e0b34f8 ("selftests/bpf: Fix some bugs in map_lookup_percpu_elem testcase")
> > > >
> > > > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > > > ---
> > > > .../bpf/prog_tests/map_lookup_percpu_elem.c | 34 ++++++++++++++-----
> > > > .../bpf/progs/test_map_lookup_percpu_elem.c | 8 ++---
> > > > 2 files changed, 30 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > > > index bfb1bf3fd427..849f33259c00 100644
> > > > --- a/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > > > +++ b/tools/testing/selftests/bpf/prog_tests/map_lookup_percpu_elem.c
> > > > @@ -2,29 +2,45 @@
> > > > /* Copyright (c) 2022 Bytedance */
> > > >
> > > > #include <test_progs.h>
> > > > +#include "bpf/libbpf_internal.h"
> > > > #include "test_map_lookup_percpu_elem.skel.h"
> > > >
> > > > void test_map_lookup_percpu_elem(void)
> > > > {
> > > > struct test_map_lookup_percpu_elem *skel;
> > > > - __u64 key = 0, sum;
> > > > - int ret, i, nr_cpus = libbpf_num_possible_cpus();
> > > > + bool *possible = NULL;
> > > > + __u64 key = 0, sum = 0;
> > > > + int cpu, nr_cpu_ids, nr_cpus, ret, slot = 0;
> > > > __u64 *buf;
> > > >
> > > > - buf = malloc(nr_cpus*sizeof(__u64));
> > > > - if (!ASSERT_OK_PTR(buf, "malloc"))
> > > > + ret = parse_cpu_mask_file("/sys/devices/system/cpu/possible", &possible,
> > > > + &nr_cpu_ids);
> > > > + if (!ASSERT_OK(ret, "parse possible CPU mask"))
> > > > return;
> > > >
> > > > - for (i = 0; i < nr_cpus; i++)
> > > > - buf[i] = i;
> > > > - sum = (nr_cpus - 1) * nr_cpus / 2;
> > > > + nr_cpus = libbpf_num_possible_cpus();
> > > > + if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
> > > > + goto free_mask;
> > > > +
> > > > + buf = malloc(nr_cpus * sizeof(*buf));
> > > > + if (!ASSERT_OK_PTR(buf, "malloc"))
> > > > + goto free_mask;
> > > > +
> > > > + for (cpu = 0; cpu < nr_cpu_ids; cpu++) {
> > > > + if (!possible[cpu])
> > > > + continue;
> > > > + buf[slot++] = cpu;
> > > > + sum += cpu;
> > > > + }
> > > > + if (!ASSERT_EQ(slot, nr_cpus, "possible CPU mask weight"))
> > > > + goto exit;
> > > >
> > > > skel = test_map_lookup_percpu_elem__open();
> > > > if (!ASSERT_OK_PTR(skel, "test_map_lookup_percpu_elem__open"))
> > > > goto exit;
> > > >
> > > > skel->rodata->my_pid = getpid();
> > > > - skel->rodata->nr_cpus = nr_cpus;
> > > > + skel->rodata->nr_cpu_ids = nr_cpu_ids;
> > > >
> > > > ret = test_map_lookup_percpu_elem__load(skel);
> > > > if (!ASSERT_OK(ret, "test_map_lookup_percpu_elem__load"))
> > > > @@ -55,4 +71,6 @@ void test_map_lookup_percpu_elem(void)
> > > > test_map_lookup_percpu_elem__destroy(skel);
> > > > exit:
> > > > free(buf);
> > > > +free_mask:
> > > > + free(possible);
> > > > }
> > > > 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 ca827b1092da..d8da0696b97c 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
> > > > @@ -7,7 +7,7 @@
> > > > __u64 percpu_array_elem_sum = 0;
> > > > __u64 percpu_hash_elem_sum = 0;
> > > > __u64 percpu_lru_hash_elem_sum = 0;
> > > > -const volatile int nr_cpus;
> > > > +const volatile int nr_cpu_ids;
> > > > const volatile int my_pid;
> > > >
> > > > struct {
> > > > @@ -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);
> > > > 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;
> > > > --
> > > > 2.55.0
> > > >
> >
> > Hi Andrii, I did use AI to find this. I often go through recent
> > commits and then ask AI to check whether the related selftests have
> > been updated and that's how I found this.
>
> in that case I'm inclined to leave selftest as is. This set up where
> possible cpu mask is not contiguous seems to be some fancy qemu-based
> setups so far, and frankly a bunch of other code is probably broken in
> such situations anyways. let's keep things as is for now
>
> pw-bot: cr
Yup got it. Thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread