[PATCH bpf v4] bpf: check cpu_possible() in per-cpu helpers and map callbacks

[PATCH bpf v4] bpf: check cpu_possible() in per-cpu helpers and map callbacks

[bo cao]

From: Bo Cao <treebacker@foxmail.com>

bpf_map_lookup_percpu_elem() and bpf_per_cpu_ptr() take the target
CPU id as an unconstrained u32 and only reject cpu >= nr_cpu_ids
before using it as a per_cpu_ptr() index.  Per-cpu memory is only
allocated for possible CPUs, so on a system with a sparse possible
mask (e.g. an arm64 guest with possible CPUs 0,2-3) a hole CPU id
passes the checks and per_cpu_ptr() computes the pointer from the
uninitialized __per_cpu_offset[hole].  For per-cpu maps this reads
and writes memory outside any valid per-cpu chunk, and for a percpu
obj allocated with bpf_percpu_obj_new() the result is writable, too.

The cpu_possible() check must go into the three per-cpu map
callbacks as well as the helper wrapper, because the verifier
rewrites bpf_map_lookup_percpu_elem() calls with a known map into
direct calls to the callbacks (kernel/bpf/fixups.c).

Also carry PTR_MAYBE_NULL into the register type on the
returns_cpu_specific_alloc_ptr path of check_helper_call():
bpf_per_cpu_ptr() can return NULL (for cpu >= nr_cpu_ids since
01cc55af9388), but the verifier currently drops the flag on that
path, so a program that skips the NULL test on a percpu obj result
passes verification and oopses on NULL + offset at run time.

Reproduced on an arm64 guest with possible mask 0,2-3, where a
socket filter program passing the hole-CPU lookup result of
bpf_map_lookup_percpu_elem() to bpf_map_update_elem() faults inside
array_map_update_elem().

Fixes: 07343110b293 ("bpf: add bpf_map_lookup_percpu_elem for percpu map")
Fixes: eaa6bcb71ef6 ("bpf: Introduce bpf_per_cpu_ptr()")
Fixes: 01cc55af9388 ("bpf: Add bpf_this_cpu_ptr/bpf_per_cpu_ptr support for allocated percpu obj")
Cc: stable@vger.kernel.org
Signed-off-by: Bo Cao <treebacker@foxmail.com>
---
Changes in v4:
- trim the commit message per review
- no code changes since v3

Link: https://lore.kernel.org/bpf/CAEf4BzYDmzNoN-5Rnfj4RrAU8wKnZe+tDFApng61L0mubXqung@mail.gmail.com/

Changes in v3:
- rename author and Signed-off-by to match the sender (bo cao).
- drop the incorrect "MEM_RDONLY, so reads only" claim for
  bpf_per_cpu_ptr(): it only holds for ksym arguments; percpu kptr
  results are writable.
- keep PTR_MAYBE_NULL on R0 for bpf_per_cpu_ptr() on percpu obj:
  check_helper_call() drops the flag on the
  returns_cpu_specific_alloc_ptr path, so the NULL return (for
  cpu >= nr_cpu_ids since 01cc55af9388, and now also for a hole
  CPU) let a non-checking program oops on NULL + offset.  With the
  flag kept, the verifier requires the NULL test; existing selftests
  already check.

Changes in v2: add the cpu_possible() check to the three per-cpu map
callbacks as well, because the verifier rewrites helper calls with a
known map into direct calls to the callbacks (kernel/bpf/fixups.c)
and thus bypasses the helpers.c wrapper.

Link: https://lore.kernel.org/r/tencent_3A8413B414528B9802AB092A67AE015FFE09@qq.com
---
 kernel/bpf/arraymap.c | 2 +-
 kernel/bpf/hashtab.c  | 4 ++--
 kernel/bpf/helpers.c  | 6 +++++-
 kernel/bpf/verifier.c | 3 ++-
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 133f189bf6d2..eab03f985280 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -327,7 +327,7 @@ static void *percpu_array_map_lookup_percpu_elem(struct bpf_map *map, void *key,
 	struct bpf_array *array = container_of(map, struct bpf_array, map);
 	u32 index = *(u32 *)key;
 
-	if (cpu >= nr_cpu_ids)
+	if (cpu >= nr_cpu_ids || !cpu_possible(cpu))
 		return NULL;
 
 	if (unlikely(index >= array->map.max_entries))
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index bf78e09f4d51..007279c16b77 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -2670,7 +2670,7 @@ static void *htab_percpu_map_lookup_percpu_elem(struct bpf_map *map, void *key,
 {
 	struct htab_elem *l;
 
-	if (cpu >= nr_cpu_ids)
+	if (cpu >= nr_cpu_ids || !cpu_possible(cpu))
 		return NULL;
 
 	l = __htab_map_lookup_elem(map, key);
@@ -2696,7 +2696,7 @@ static void *htab_lru_percpu_map_lookup_percpu_elem(struct bpf_map *map, void *k
 {
 	struct htab_elem *l;
 
-	if (cpu >= nr_cpu_ids)
+	if (cpu >= nr_cpu_ids || !cpu_possible(cpu))
 		return NULL;
 
 	l = __htab_map_lookup_elem(map, key);
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 648abe1ba63a..f9f2bd665038 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -133,6 +133,10 @@ const struct bpf_func_proto bpf_map_peek_elem_proto = {
 BPF_CALL_3(bpf_map_lookup_percpu_elem, struct bpf_map *, map, void *, key, u32, cpu)
 {
 	WARN_ON_ONCE(!bpf_rcu_lock_held());
+
+	if (cpu >= nr_cpu_ids || !cpu_possible(cpu))
+		return 0;
+
 	return (unsigned long) map->ops->map_lookup_percpu_elem(map, key, cpu);
 }
 
@@ -753,7 +757,7 @@ const struct bpf_func_proto bpf_copy_from_user_task_proto = {
 
 BPF_CALL_2(bpf_per_cpu_ptr, const void *, ptr, u32, cpu)
 {
-	if (cpu >= nr_cpu_ids)
+	if (cpu >= nr_cpu_ids || !cpu_possible(cpu))
 		return (unsigned long)NULL;
 
 	return (unsigned long)per_cpu_ptr((const void __percpu *)(const uintptr_t)ptr, cpu);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5601d2056218..0cedf39ee226 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12957,7 +12957,8 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 			regs[BPF_REG_0].mem_size = tsize;
 		} else {
 			if (returns_cpu_specific_alloc_ptr) {
-				regs[BPF_REG_0].type = PTR_TO_BTF_ID | MEM_ALLOC | MEM_RCU;
+				regs[BPF_REG_0].type = PTR_TO_BTF_ID | MEM_ALLOC | MEM_RCU |
+						       (ret_flag & PTR_MAYBE_NULL);
 			} else {
 				/* MEM_RDONLY may be carried from ret_flag, but it
 				 * doesn't apply on PTR_TO_BTF_ID. Fold it, otherwise
-- 
2.39.5 (Apple Git-154)

Re: [PATCH bpf v4] bpf: check cpu_possible() in per-cpu helpers and map callbacks

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1666 bytes --]

> bpf: check cpu_possible() in per-cpu helpers and map callbacks

TL;DR: Not a bug, a design question: the new cpu_possible() check in the
bpf_map_lookup_percpu_elem() helper looks redundant, since every per-cpu map
callback now does the same check. Could it be dropped?

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c

[ ... ]

> @@ -133,6 +133,10 @@ const struct bpf_func_proto bpf_map_peek_elem_proto = {
>  BPF_CALL_3(bpf_map_lookup_percpu_elem, struct bpf_map *, map, void *, key, u32, cpu)
>  {
>  	WARN_ON_ONCE(!bpf_rcu_lock_held());
> +
> +	if (cpu >= nr_cpu_ids || !cpu_possible(cpu))
> +		return 0;
> +
>  	return (unsigned long) map->ops->map_lookup_percpu_elem(map, key, cpu);
>  }

This isn't a bug, but is the extra check in the
bpf_map_lookup_percpu_elem() helper needed?

Every map_lookup_percpu_elem callback (percpu_array_map_lookup_percpu_elem(),
htab_percpu_map_lookup_percpu_elem() and
htab_lru_percpu_map_lookup_percpu_elem()) now rejects impossible CPUs
itself, and check_map_func_compatibility() only admits those three map
types for this helper.

When bpf_do_misc_fixups() patches the call into a direct call to the
callback, the helper body never runs.  So the helper-level check never
decides anything on either path.

Could it be dropped, to match bpf_map_lookup_elem() and friends, which
leave argument checks to the map ops?


---
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/42553704194

Re: [PATCH bpf v4] bpf: check cpu_possible() in per-cpu helpers and map callbacks

[Alexei Starovoitov]

On Sun, Nov 08, 2026 at 06:43 PM bo cao <treebacker@foxmail.com> wrote:
> @@ -133,6 +133,10 @@ const struct bpf_func_proto bpf_map_peek_elem_proto = {
>  BPF_CALL_3(bpf_map_lookup_percpu_elem, struct bpf_map *, map, void *, key, u32, cpu)
>  {
>  	WARN_ON_ONCE(!bpf_rcu_lock_held());
> +
> +	if (cpu >= nr_cpu_ids || !cpu_possible(cpu))
> +		return 0;
> +
>  	return (unsigned long) map->ops->map_lookup_percpu_elem(map, key, cpu);

bpf-ci is right. All three callbacks have the check.
Drop this hunk. It's dead code.

>  			if (returns_cpu_specific_alloc_ptr) {
> -				regs[BPF_REG_0].type = PTR_TO_BTF_ID | MEM_ALLOC | MEM_RCU;
> +				regs[BPF_REG_0].type = PTR_TO_BTF_ID | MEM_ALLOC | MEM_RCU |
> +						       (ret_flag & PTR_MAYBE_NULL);

This is a fix for a different bug. It doesn't need a sparse mask.
bpf_per_cpu_ptr(p, -1) returns NULL on every machine.
It may break existing progs and needs a separate discussion.

pw-bot: cr