All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: fix percpu map update indexing with sparse CPU IDs
@ 2026-08-13 12:02 Hui Su
  2026-08-13 12:26 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Hui Su @ 2026-08-13 12:02 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, linux-kernel,
	linux-kselftest, Hui Su, sashiko-bot

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")
Reported-by: sashiko-bot@kernel.org
Signed-off-by: Hui Su <sh_def@163.com>
---
 kernel/bpf/arraymap.c      | 5 +++--
 kernel/bpf/hashtab.c       | 4 +++-
 kernel/bpf/local_storage.c | 5 +++--
 3 files changed, 9 insertions(+), 5 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..298b16ac4cc0 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1026,6 +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 off = 0;
 		int cpu;
 
 		if (map_flags & BPF_F_CPU) {
@@ -1038,9 +1039,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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf] bpf: fix percpu map update indexing with sparse CPU IDs
  2026-08-13 12:02 [PATCH bpf] bpf: fix percpu map update indexing with sparse CPU IDs Hui Su
@ 2026-08-13 12:26 ` sashiko-bot
  2026-08-13 13:48 ` bot+bpf-ci
  2026-08-13 15:00 ` Leon Hwang
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-13 12:26 UTC (permalink / raw)
  To: Hui Su; +Cc: bpf

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

Pre-existing issues:
- [High] Out-of-bounds memory corruption due to flawed CPU ID validation for BPF_F_CPU flag in percpu map updates
- [Medium] Uninitialized kernel memory is leaked to userspace during per-CPU map lookups for maps containing BTF records
--

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

bpf: fix percpu map update indexing with sparse CPU IDs

This commit fixes an issue where per-CPU map update paths calculated source
offsets using logical CPU IDs rather than advancing the offset for each
possible CPU. This resolves out-of-bounds reads and incorrect values on
systems with sparse CPU topologies.

> 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
[ ... ]
> @@ -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 does the BPF_F_CPU flag path have a
bounds checking issue with sparse CPU topologies?

Looking at bpf_map_check_op_flags() in include/linux/bpf.h, it validates
the CPU ID against num_possible_cpus():

include/linux/bpf.h:bpf_map_check_op_flags() {
        ...
        cpu = flags >> 32;
        if ((flags & BPF_F_CPU) && cpu >= num_possible_cpus())
                return -ERANGE;
        ...
}

If a system has sparse CPU topologies, can a user pass a CPU ID that is
less than num_possible_cpus() but not in the cpu_possible_mask? If so,
this would allow an impossible CPU ID to be passed to per_cpu_ptr() just
above this loop in bpf_percpu_array_update(), returning an invalid
pointer that copy_map_value() then writes to.

>  	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);

[Severity: Medium]
This is a pre-existing issue, but can per-CPU map lookups leak uninitialized
kernel memory to userspace when the map contains BTF records?

When looking up a per-CPU map, the lookup buffer is allocated via kvmalloc()
without __GFP_ZERO:

kernel/bpf/syscall.c:map_lookup_elem() {
        ...
        err = -ENOMEM;
        value = kvmalloc(value_size, GFP_USER | __GFP_NOWARN);
        if (!value)
                goto free_key;
        ...
}

For each CPU, bpf_obj_memcpy() (which bypasses bpf_long_memcpy when BTF
records are present) copies exactly map->value_size bytes. If map->value_size
is not a multiple of 8, it leaves the alignment padding uninitialized. When
this entire buffer is subsequently copied to userspace via copy_to_user(),
will it leak the uninitialized padding bytes?

>  		bpf_obj_cancel_fields(map, ptr);
> +		off += size;
>  	}
>  unlock:
>  	rcu_read_unlock();

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf] bpf: fix percpu map update indexing with sparse CPU IDs
  2026-08-13 12:02 [PATCH bpf] bpf: fix percpu map update indexing with sparse CPU IDs Hui Su
  2026-08-13 12:26 ` sashiko-bot
@ 2026-08-13 13:48 ` bot+bpf-ci
  2026-08-13 15:00 ` Leon Hwang
  2 siblings, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-08-13 13:48 UTC (permalink / raw)
  To: sh_def, bpf
  Cc: ast, daniel, andrii, martin.lau, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, linux-kernel,
	linux-kselftest, sh_def, sashiko-bot, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

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

> commit 3563498ad027820b11bad3993a5c6a17d293ad23
> Author: Hui Su <sh_def@163.com>
>
>     bpf: fix percpu map update indexing with sparse CPU IDs
>
>     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")

The Fixes: tag points to 8eb76cb03f0f, which introduced the 'value +
size * cpu' pattern in bpf_percpu_array_update() (kernel/bpf/arraymap.c).
That attribution is correct for the arraymap.c fix.

However, at commit 8eb76cb03f0f, kernel/bpf/hashtab.c and
kernel/bpf/local_storage.c still used the correct 'off += size' pattern
and did not have the 'size * cpu' bug yet. The hashtab.c and
local_storage.c bugs were introduced later in the same BPF_F_CPU feature
series.

Should this commit include additional Fixes: tags for the commits that
introduced the bug in hashtab.c and local_storage.c? This would ensure
that backporting tools pick up all three fixes even when the percpu_array
commit is not present in a given stable tree.


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf] bpf: fix percpu map update indexing with sparse CPU IDs
  2026-08-13 12:02 [PATCH bpf] bpf: fix percpu map update indexing with sparse CPU IDs Hui Su
  2026-08-13 12:26 ` sashiko-bot
  2026-08-13 13:48 ` bot+bpf-ci
@ 2026-08-13 15:00 ` Leon Hwang
  2 siblings, 0 replies; 4+ messages in thread
From: Leon Hwang @ 2026-08-13 15:00 UTC (permalink / raw)
  To: Hui Su, bpf
  Cc: ast, daniel, andrii, martin.lau, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, linux-kernel,
	linux-kselftest, sashiko-bot

Hi Hui,

Thanks for the further fixes.

On 2026/8/13 20:02, Hui Su wrote:
> 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")

The ci bot got the point about the missing Fixes:

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")

> Reported-by: sashiko-bot@kernel.org

This Reported-by is unnecessary, because Sashiko would review most LKML
patches.

> Signed-off-by: Hui Su <sh_def@163.com>

Acked-by: Leon Hwang <leon.hwang@linux.dev>

Thanks,
Leon

> ---

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-13 15:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 12:02 [PATCH bpf] bpf: fix percpu map update indexing with sparse CPU IDs Hui Su
2026-08-13 12:26 ` sashiko-bot
2026-08-13 13:48 ` bot+bpf-ci
2026-08-13 15:00 ` Leon Hwang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.