[PATCH bpf v2] bpf: fix percpu map update indexing with sparse CPU IDs

[PATCH bpf v2] bpf: fix percpu map update indexing with sparse CPU IDs

[Hui Su]

Per-CPU array, hash, and cgroup storage map updates without BPF_F_CPU
or BPF_F_ALL_CPUS use a value buffer whose per-CPU slots are packed in
possible-CPU order. The buffer is sized as:

  round_up(value_size, 8) * num_possible_cpus()

The update paths iterate over possible CPUs, but use the logical CPU ID
to calculate the source offset:

  value + size * cpu

This only works when possible CPU IDs are contiguous starting at zero.

For example, with a possible CPU mask of 0,2-3, the buffer contains
three slots corresponding to CPUs 0, 2, and 3. CPU2 is therefore
expected to use slot 1 and CPU3 slot 2. Instead, the current code uses
slots 2 and 3 respectively, causing incorrect per-CPU values and an
out-of-bounds read from the update buffer for CPU3.

The corresponding lookup paths already use a dense offset while
iterating over possible CPUs. Do the same for the array, hash, and
cgroup storage update paths, advancing the source offset once for each
possible CPU. BPF_F_ALL_CPUS continues to use the same value for every
CPU.

Fixes: 8eb76cb03f0f ("bpf: Add BPF_F_CPU and BPF_F_ALL_CPUS flags support for percpu_array maps")
Fixes: c6936161fd55 ("bpf: Add BPF_F_CPU and BPF_F_ALL_CPUS flags support for percpu_hash and lru_percpu_hash maps")
Fixes: 47c79f05aa0d ("bpf: Add BPF_F_CPU and BPF_F_ALL_CPUS flags support for percpu_cgroup_storage maps")
Acked-by: Leon Hwang <leon.hwang@linux.dev>
Signed-off-by: Hui Su <sh_def@163.com>
---
Changes in v2:
- Add the missing Fixes tags for percpu hash and percpu cgroup storage.
- Drop the unnecessary Sashiko Reported-by tag.
v1 link: https://lore.kernel.org/lkml/20260813120250.796934-4-sh_def@163.com/

 kernel/bpf/arraymap.c      | 5 +++--
 kernel/bpf/hashtab.c       | 5 +++--
 kernel/bpf/local_storage.c | 5 +++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 248b4818178c..cc3f8c25a28b 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -405,7 +405,7 @@ int bpf_percpu_array_update(struct bpf_map *map, void *key, void *value,
 	void __percpu *pptr;
 	void *ptr, *val;
 	u32 size;
-	int cpu;
+	int cpu, off = 0;
 
 	if (unlikely((map_flags & BPF_F_LOCK) || (u32)map_flags > BPF_F_ALL_CPUS))
 		/* unknown flags */
@@ -437,9 +437,10 @@ int bpf_percpu_array_update(struct bpf_map *map, void *key, void *value,
 	}
 	for_each_possible_cpu(cpu) {
 		ptr = per_cpu_ptr(pptr, cpu);
-		val = (map_flags & BPF_F_ALL_CPUS) ? value : value + size * cpu;
+		val = (map_flags & BPF_F_ALL_CPUS) ? value : value + off;
 		copy_map_value(map, ptr, val);
 		bpf_obj_cancel_fields(map, ptr);
+		off += size;
 	}
 unlock:
 	rcu_read_unlock();
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 9f394e1aa2e8..dd4da18312ac 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1026,7 +1026,7 @@ static void pcpu_copy_value(struct bpf_htab *htab, void __percpu *pptr,
 	} else {
 		u32 size = round_up(htab->map.value_size, 8);
 		void *val;
-		int cpu;
+		int cpu, off = 0;
 
 		if (map_flags & BPF_F_CPU) {
 			cpu = map_flags >> 32;
@@ -1038,9 +1038,10 @@ static void pcpu_copy_value(struct bpf_htab *htab, void __percpu *pptr,
 
 		for_each_possible_cpu(cpu) {
 			ptr = per_cpu_ptr(pptr, cpu);
-			val = (map_flags & BPF_F_ALL_CPUS) ? value : value + size * cpu;
+			val = (map_flags & BPF_F_ALL_CPUS) ? value : value + off;
 			copy_map_value(&htab->map, ptr, val);
 			bpf_obj_cancel_fields(&htab->map, ptr);
+			off += size;
 		}
 	}
 }
diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 23267213a17f..83cd527a2542 100644
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -220,7 +220,7 @@ int bpf_percpu_cgroup_storage_update(struct bpf_map *_map, void *key,
 	struct bpf_cgroup_storage *storage;
 	void *val;
 	u32 size;
-	int cpu;
+	int cpu, off = 0;
 
 	if ((u32)map_flags & ~(BPF_ANY | BPF_EXIST | BPF_F_CPU | BPF_F_ALL_CPUS))
 		return -EINVAL;
@@ -245,8 +245,9 @@ int bpf_percpu_cgroup_storage_update(struct bpf_map *_map, void *key,
 	}
 	size = round_up(_map->value_size, 8);
 	for_each_possible_cpu(cpu) {
-		val = (map_flags & BPF_F_ALL_CPUS) ? value : value + size * cpu;
+		val = (map_flags & BPF_F_ALL_CPUS) ? value : value + off;
 		copy_map_value(_map, per_cpu_ptr(storage->percpu_buf, cpu), val);
+		off += size;
 	}
 unlock:
 	rcu_read_unlock();
-- 
2.54.0

Re: [PATCH bpf v2] bpf: fix percpu map update indexing with sparse CPU IDs

[sashiko-bot]

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

Pre-existing issues:
- [High] The BPF_F_CPU map flag validation is vulnerable to out-of-bounds memory access (read/write) when the `cpu_possible_mask` is sparse. The validation incorrectly checks against `num_possible_cpus()` instead of verifying if the CPU ID is actually a valid possible CPU.
--

--- Patch [1]: [PATCH bpf v2] bpf: fix percpu map update indexing with sparse CPU IDs ---
commit 5d6a274a595d67867bb26dbd708c1d7e75d30170
Author: Hui Su <sh_def@163.com>

bpf: fix percpu map update indexing with sparse CPU IDs

This patch fixes an issue where percpu map updates incorrectly advance
the user value pointer using the physical CPU ID instead of a dense index.
This resolves out-of-bounds reads from the user buffer on systems with
sparse possible CPU masks.

Link: https://lore.kernel.org/lkml/20260813120250.796934-4-sh_def@163.com/

> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index 248b4818178cd..cc3f8c25a28b5 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -405,7 +405,7 @@ int bpf_percpu_array_update(struct bpf_map *map, void *key, void *value,

[ ... ]

> @@ -437,9 +437,10 @@ int bpf_percpu_array_update(struct bpf_map *map, void *key, void *value,
>  	}

[Severity: High]
This is a pre-existing issue, but since this patch focuses on sparse CPU
topologies, is the BPF_F_CPU flag validation also vulnerable to sparse
CPU masks?

Looking at bpf_map_check_op_flags() in include/linux/bpf.h, the validation
checks:

    if ((flags & BPF_F_CPU) && cpu >= num_possible_cpus())
        return -ERANGE;

Because num_possible_cpus() is a population count rather than a check
against cpu_possible_mask or nr_cpu_ids, could an invalid or impossible
physical CPU ID that is strictly less than num_possible_cpus() pass this
check?

If such a CPU ID passes validation, it appears it would be used immediately
above the modified code in bpf_percpu_array_update():

    if (map_flags & BPF_F_CPU) {
        cpu = map_flags >> 32;
        ptr = per_cpu_ptr(pptr, cpu);
        copy_map_value(map, ptr, value);

Can this cause per_cpu_ptr() to return an unmapped raw percpu offset
since __per_cpu_offset[cpu] would be 0 for an impossible CPU, resulting
in a page fault and kernel panic when dereferenced by copy_map_value()?

Should the validation in bpf_map_check_op_flags() be updated to use
cpu_possible(cpu) or check against nr_cpu_ids instead?

>  	for_each_possible_cpu(cpu) {
>  		ptr = per_cpu_ptr(pptr, cpu);
> -		val = (map_flags & BPF_F_ALL_CPUS) ? value : value + size * cpu;
> +		val = (map_flags & BPF_F_ALL_CPUS) ? value : value + off;
>  		copy_map_value(map, ptr, val);
>  		bpf_obj_cancel_fields(map, ptr);
> +		off += size;
>  	}
>  unlock:
>  	rcu_read_unlock();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813155131.1022745-3-sh_def@163.com?part=1