[PATCH bpf v2 0/2] bpf: fix per-cpu initialization of a BPF_F_CPU created hash element

[PATCH bpf v2 0/2] bpf: fix per-cpu initialization of a BPF_F_CPU created hash element

[Donggeun Yoo]

A BPF_F_CPU update that creates a [lru_]percpu_hash element writes the
named CPU's slot and leaves the others holding the recycled element's
values, so a lookup of the new key returns a deleted key's per-cpu
values.

Patch 1 zero-fills the other CPUs.  Patch 2 adds the selftest: the
existing cpu_flag subtests always prime a key with BPF_F_ALL_CPUS
first, so the create path is not covered today.

v1: https://lore.kernel.org/bpf/20260920093153.439743-1-donggeunyoo.kernel@gmail.com/

Changes in v2:
 - skip the new subtests instead of failing them on a uniprocessor
   machine (Sashiko AI review)
 - cover the BPF_F_CPU entry condition in the block comment above
   pcpu_init_value() (BPF CI AI review, Alexei Starovoitov)

test_progs -t percpu_alloc and fourteen neighboring map tests, x86_64
under QEMU/KVM, 4 vCPUs:

  without patch 1   31/112 PASSED, 1/3 FAILED
  with patch 1      32/115 PASSED, 0/0 FAILED

Donggeun Yoo (2):
  bpf: Zero-fill other CPUs when BPF_F_CPU creates a per-cpu hash
    element
  selftests/bpf: Test per-cpu initialization of a BPF_F_CPU created
    element

 kernel/bpf/hashtab.c                          | 10 ++-
 .../selftests/bpf/prog_tests/percpu_alloc.c   | 80 +++++++++++++++++++
 2 files changed, 86 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH bpf v2 1/2] bpf: Zero-fill other CPUs when BPF_F_CPU creates a per-cpu hash element

[Donggeun Yoo]

pcpu_init_value() initializes the per-cpu area of a newly created
[lru_]percpu_hash element.  The area is recycled, so when the value
comes from a BPF program (onallcpus == false) it writes the running
CPU's slot and zeroes the rest.

bpf_percpu_hash_update() passes onallcpus == true, which delegates to
pcpu_copy_value().  Since BPF_F_CPU was added that writes only the CPU
named in map_flags when the flag is set, so on the create path the other
slots keep the recycled element's values:

  update(k1, 0xdeadc0de, BPF_F_ALL_CPUS)  every CPU holds 0xdeadc0de
  delete(k1)                              element back on the freelist
  update(k2, 0xc0ffee, BPF_F_CPU | 0)     creates, writes CPU 0 only
  lookup(k2)                              CPU 0 0xc0ffee, rest 0xdeadc0de

Zero-fill the other CPUs on that arm too.

Fixes: c6936161fd55 ("bpf: Add BPF_F_CPU and BPF_F_ALL_CPUS flags support for percpu_hash and lru_percpu_hash maps")
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
The merged arm no longer calls bpf_obj_cancel_fields() on the named
CPU.  That call is inert here: it acts only on BPF_TIMER,
BPF_WORKQUEUE and BPF_TASK_WORK, which map_check_btf() rejects for
[lru_]percpu_hash.

v2: name the BPF_F_CPU entry condition in the block comment.

 kernel/bpf/hashtab.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 4f495dcbf670c..f7a9c432d9b76 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1054,14 +1054,16 @@ static void pcpu_init_value(struct bpf_htab *htab, void __percpu *pptr,
 	/* When not setting the initial value on all cpus, zero-fill element
 	 * values for other cpus. Otherwise, bpf program has no way to ensure
 	 * known initial values for cpus other than current one
-	 * (onallcpus=false always when coming from bpf prog).
+	 * (onallcpus=false always when coming from bpf prog,
+	 *  map_flags & BPF_F_CPU when coming from syscall but setting
+	 *  only one cpu).
 	 */
-	if (!onallcpus) {
-		int current_cpu = raw_smp_processor_id();
+	if (!onallcpus || (map_flags & BPF_F_CPU)) {
+		int init_cpu = onallcpus ? map_flags >> 32 : raw_smp_processor_id();
 		int cpu;
 
 		for_each_possible_cpu(cpu) {
-			if (cpu == current_cpu)
+			if (cpu == init_cpu)
 				copy_map_value(&htab->map, per_cpu_ptr(pptr, cpu), value);
 			else /* Since elem is preallocated, we cannot touch special fields */
 				zero_map_value(&htab->map, per_cpu_ptr(pptr, cpu));
-- 
2.53.0

Re: [PATCH bpf v2 1/2] bpf: Zero-fill other CPUs when BPF_F_CPU creates a per-cpu hash element

[bot+bpf-ci]

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

> bpf: Zero-fill other CPUs when BPF_F_CPU creates a per-cpu hash element
>
> pcpu_init_value() initializes the per-cpu area of a newly created
> [lru_]percpu_hash element.  The area is recycled, so when the value
> comes from a BPF program (onallcpus == false) it writes the running
> CPU's slot and zeroes the rest.
>
> bpf_percpu_hash_update() passes onallcpus == true, which delegates to
> pcpu_copy_value().  Since BPF_F_CPU was added that writes only the CPU
> named in map_flags when the flag is set, so on the create path the other
> slots keep the recycled element's values:
>
>   update(k1, 0xdeadc0de, BPF_F_ALL_CPUS)  every CPU holds 0xdeadc0de
>   delete(k1)                              element back on the freelist
>   update(k2, 0xc0ffee, BPF_F_CPU | 0)     creates, writes CPU 0 only
>   lookup(k2)                              CPU 0 0xc0ffee, rest 0xdeadc0de
>
> Zero-fill the other CPUs on that arm too.
>
> Fixes: c6936161fd55 ("bpf: Add BPF_F_CPU and BPF_F_ALL_CPUS flags support for percpu_hash and lru_percpu_hash maps")
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>

This isn't a bug, but could the sentence starting with "Since BPF_F_CPU
was added..." be split or re-flowed?  The construction mixes two
sentence frames ("Since ... was added that writes ... so on the create
path ..."), which makes it harder to parse on a first read.


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

Re: [PATCH bpf v2 1/2] bpf: Zero-fill other CPUs when BPF_F_CPU creates a per-cpu hash element

[Leon Hwang]

On 23/9/26 08:08, Donggeun Yoo wrote:
> pcpu_init_value() initializes the per-cpu area of a newly created
> [lru_]percpu_hash element.  The area is recycled, so when the value
> comes from a BPF program (onallcpus == false) it writes the running
> CPU's slot and zeroes the rest.
> 
> bpf_percpu_hash_update() passes onallcpus == true, which delegates to
> pcpu_copy_value().  Since BPF_F_CPU was added that writes only the CPU
> named in map_flags when the flag is set, so on the create path the other
> slots keep the recycled element's values:
> 
>   update(k1, 0xdeadc0de, BPF_F_ALL_CPUS)  every CPU holds 0xdeadc0de
>   delete(k1)                              element back on the freelist
>   update(k2, 0xc0ffee, BPF_F_CPU | 0)     creates, writes CPU 0 only
>   lookup(k2)                              CPU 0 0xc0ffee, rest 0xdeadc0de
> 
> Zero-fill the other CPUs on that arm too.
> 
> Fixes: c6936161fd55 ("bpf: Add BPF_F_CPU and BPF_F_ALL_CPUS flags support for percpu_hash and lru_percpu_hash maps")
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
> ---
> The merged arm no longer calls bpf_obj_cancel_fields() on the named
> CPU.  That call is inert here: it acts only on BPF_TIMER,
> BPF_WORKQUEUE and BPF_TASK_WORK, which map_check_btf() rejects for
> [lru_]percpu_hash.
> 
> v2: name the BPF_F_CPU entry condition in the block comment.
> 
>  kernel/bpf/hashtab.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 4f495dcbf670c..f7a9c432d9b76 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -1054,14 +1054,16 @@ static void pcpu_init_value(struct bpf_htab *htab, void __percpu *pptr,
>  	/* When not setting the initial value on all cpus, zero-fill element
>  	 * values for other cpus. Otherwise, bpf program has no way to ensure
>  	 * known initial values for cpus other than current one
> -	 * (onallcpus=false always when coming from bpf prog).
> +	 * (onallcpus=false always when coming from bpf prog,
> +	 *  map_flags & BPF_F_CPU when coming from syscall but setting
> +	 *  only one cpu).
>  	 */
> -	if (!onallcpus) {
> -		int current_cpu = raw_smp_processor_id();
> +	if (!onallcpus || (map_flags & BPF_F_CPU)) {
> +		int init_cpu = onallcpus ? map_flags >> 32 : raw_smp_processor_id();

'onallcpus ? map_flags >> 32' does not look reasonable.

Based on the 'if' above, either of these looks straightforward to me.

int init_cpu = !onallcpus ? raw_smp_processor_id() : map_flags >> 32;

or

int init_cpu = (map_flags & BPF_F_CPU) ? map_flags >> 32 :
raw_smp_processor_id();

Thanks,
Leon

>  		int cpu;
>  
>  		for_each_possible_cpu(cpu) {
> -			if (cpu == current_cpu)
> +			if (cpu == init_cpu)
>  				copy_map_value(&htab->map, per_cpu_ptr(pptr, cpu), value);
>  			else /* Since elem is preallocated, we cannot touch special fields */
>  				zero_map_value(&htab->map, per_cpu_ptr(pptr, cpu));

[PATCH bpf v2 2/2] selftests/bpf: Test per-cpu initialization of a BPF_F_CPU created element

[Donggeun Yoo]

The existing cpu_flag subtests always prime a key with BPF_F_ALL_CPUS
before any BPF_F_CPU write, so the create path is never covered.

Add a subtest that creates the element with BPF_F_CPU on a map with
max_entries 1, so the key can only reuse the element the previous key
released, and check that the CPUs the update did not name read back
zero.  Run it for PERCPU_HASH preallocated and BPF_F_NO_PREALLOC,
whose per-cpu areas come from different allocators, and for
LRU_PERCPU_HASH.

Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
v2: skip on a uniprocessor machine instead of failing; the block
comment uses the general kernel style.

 .../selftests/bpf/prog_tests/percpu_alloc.c   | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c b/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c
index a72ae0b29f6e9..f853f97bb882d 100644
--- a/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c
@@ -350,6 +350,80 @@ static void test_lru_percpu_hash_cpu_flag(void)
 	test_percpu_map_cpu_flag(BPF_MAP_TYPE_LRU_PERCPU_HASH);
 }
 
+/*
+ * A BPF_F_CPU update that creates an element must zero the value on the other
+ * cpus, rather than leave them holding whatever the recycled element last
+ * contained. max_entries is 1 so the second key can only reuse the element
+ * the first one released.
+ */
+static void test_percpu_map_cpu_flag_create(enum bpf_map_type map_type, __u32 map_flags)
+{
+	LIBBPF_OPTS(bpf_map_create_opts, opts, .map_flags = map_flags);
+	const u32 stale = 0xDEADC0DE, fresh = 0xC0FFEE;
+	int nr_cpus, cpu, map_fd, err, key;
+	u32 value;
+	u64 flags;
+
+	nr_cpus = libbpf_num_possible_cpus();
+	if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
+		return;
+
+	if (nr_cpus < 2) {
+		test__skip();
+		return;
+	}
+
+	map_fd = bpf_map_create(map_type, "cpu_flag_create", sizeof(key), sizeof(value), 1, &opts);
+	if (!ASSERT_GE(map_fd, 0, "bpf_map_create"))
+		return;
+
+	key = 1;
+	value = stale;
+	err = bpf_map_update_elem(map_fd, &key, &value, BPF_F_ALL_CPUS);
+	if (!ASSERT_OK(err, "bpf_map_update_elem all_cpus"))
+		goto out;
+
+	err = bpf_map_delete_elem(map_fd, &key);
+	if (!ASSERT_OK(err, "bpf_map_delete_elem"))
+		goto out;
+
+	key = 2;
+	value = fresh;
+	flags = BPF_F_CPU;
+	err = bpf_map_update_elem(map_fd, &key, &value, flags);
+	if (!ASSERT_OK(err, "bpf_map_update_elem specified cpu"))
+		goto out;
+
+	for (cpu = 0; cpu < nr_cpus; cpu++) {
+		value = 0;
+		flags = (u64)cpu << 32 | BPF_F_CPU;
+		err = bpf_map_lookup_elem_flags(map_fd, &key, &value, flags);
+		if (!ASSERT_OK(err, "bpf_map_lookup_elem_flags specified cpu"))
+			goto out;
+		if (!ASSERT_EQ(value, cpu ? 0 : fresh, "value on specified cpu"))
+			goto out;
+	}
+
+out:
+	close(map_fd);
+}
+
+static void test_percpu_hash_cpu_flag_create(void)
+{
+	test_percpu_map_cpu_flag_create(BPF_MAP_TYPE_PERCPU_HASH, 0);
+}
+
+static void test_percpu_hash_cpu_flag_create_malloc(void)
+{
+	test_percpu_map_cpu_flag_create(BPF_MAP_TYPE_PERCPU_HASH, BPF_F_NO_PREALLOC);
+}
+
+static void test_lru_percpu_hash_cpu_flag_create(void)
+{
+	/* lru without prealloc is -ENOTSUPP, so there is no malloc variant */
+	test_percpu_map_cpu_flag_create(BPF_MAP_TYPE_LRU_PERCPU_HASH, 0);
+}
+
 static void test_percpu_cgroup_storage_cpu_flag(void)
 {
 	struct percpu_alloc_array *skel = NULL;
@@ -454,6 +528,12 @@ void test_percpu_alloc(void)
 		test_percpu_hash_cpu_flag();
 	if (test__start_subtest("cpu_flag_lru_percpu_hash"))
 		test_lru_percpu_hash_cpu_flag();
+	if (test__start_subtest("cpu_flag_create_percpu_hash"))
+		test_percpu_hash_cpu_flag_create();
+	if (test__start_subtest("cpu_flag_create_percpu_hash_malloc"))
+		test_percpu_hash_cpu_flag_create_malloc();
+	if (test__start_subtest("cpu_flag_create_lru_percpu_hash"))
+		test_lru_percpu_hash_cpu_flag_create();
 	if (test__start_subtest("cpu_flag_percpu_cgroup_storage"))
 		test_percpu_cgroup_storage_cpu_flag();
 	if (test__start_subtest("cpu_flag_array"))
-- 
2.53.0

Re: [PATCH bpf v2 2/2] selftests/bpf: Test per-cpu initialization of a BPF_F_CPU created element

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c b/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c
> index a72ae0b29f6e..f853f97bb882 100644
> --- a/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/percpu_alloc.c

[ ... ]

> @@ -350,6 +350,80 @@ static void test_lru_percpu_hash_cpu_flag(void)
>  	test_percpu_map_cpu_flag(BPF_MAP_TYPE_LRU_PERCPU_HASH);
>  }
>
> +/*
> + * A BPF_F_CPU update that creates an element must zero the value on the other
> + * cpus, rather than leave them holding whatever the recycled element last
> + * contained. max_entries is 1 so the second key can only reuse the element
> + * the first one released.
> + */
> +static void test_percpu_map_cpu_flag_create(enum bpf_map_type map_type, __u32 map_flags)
> +{
> +	LIBBPF_OPTS(bpf_map_create_opts, opts, .map_flags = map_flags);
> +	const u32 stale = 0xDEADC0DE, fresh = 0xC0FFEE;
> +	int nr_cpus, cpu, map_fd, err, key;
> +	u32 value;
> +	u64 flags;
> +
> +	nr_cpus = libbpf_num_possible_cpus();
> +	if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
> +		return;
> +
> +	if (nr_cpus < 2) {
> +		test__skip();
> +		return;
> +	}
> +
> +	map_fd = bpf_map_create(map_type, "cpu_flag_create", sizeof(key), sizeof(value), 1, &opts);
> +	if (!ASSERT_GE(map_fd, 0, "bpf_map_create"))
> +		return;
> +
> +	key = 1;
> +	value = stale;
> +	err = bpf_map_update_elem(map_fd, &key, &value, BPF_F_ALL_CPUS);
> +	if (!ASSERT_OK(err, "bpf_map_update_elem all_cpus"))
> +		goto out;
> +
> +	err = bpf_map_delete_elem(map_fd, &key);
> +	if (!ASSERT_OK(err, "bpf_map_delete_elem"))
> +		goto out;
> +
> +	key = 2;
> +	value = fresh;
> +	flags = BPF_F_CPU;
> +	err = bpf_map_update_elem(map_fd, &key, &value, flags);
> +	if (!ASSERT_OK(err, "bpf_map_update_elem specified cpu"))
> +		goto out;
> +
> +	for (cpu = 0; cpu < nr_cpus; cpu++) {
> +		value = 0;
> +		flags = (u64)cpu << 32 | BPF_F_CPU;
> +		err = bpf_map_lookup_elem_flags(map_fd, &key, &value, flags);
> +		if (!ASSERT_OK(err, "bpf_map_lookup_elem_flags specified cpu"))
> +			goto out;
> +		if (!ASSERT_EQ(value, cpu ? 0 : fresh, "value on specified cpu"))
> +			goto out;
> +	}
> +
> +out:
> +	close(map_fd);
> +}
> +
> +static void test_percpu_hash_cpu_flag_create(void)
> +{
> +	test_percpu_map_cpu_flag_create(BPF_MAP_TYPE_PERCPU_HASH, 0);
> +}
> +
> +static void test_percpu_hash_cpu_flag_create_malloc(void)
> +{
> +	test_percpu_map_cpu_flag_create(BPF_MAP_TYPE_PERCPU_HASH, BPF_F_NO_PREALLOC);
> +}

Can this test give false negatives on the BPF_F_NO_PREALLOC variant?

For preallocated maps, the comment's reasoning holds: max_entries 1 means
there's a single struct htab_elem whose per-cpu area is allocated once and
never freed. When the test deletes key 1, the element goes back to the
freelist with stale 0xDEADC0DE still in its per-cpu area. Creating key 2
gets that same element back, so if pcpu_init_value() is broken, cpu 0 will
see 0xC0FFEE but other CPUs will still see 0xDEADC0DE.

With BPF_F_NO_PREALLOC, the per-cpu area uses bpf_mem_alloc:

kernel/bpf/hashtab.c:htab_elem_free()
    bpf_mem_cache_free(&htab->pcpu_ma, l->ptr_to_pptr)

which does unit_free() and pushes to the *current* CPU's free list. The
subsequent create calls bpf_mem_cache_alloc(), which does unit_alloc() and
pops from whatever CPU the second syscall runs on.

If the thread migrated between the delete and the create, unit_alloc() gets
a prefilled zero object from __alloc_percpu_gfp() rather than the
just-freed stale one, so the test passes even on a broken kernel. Only when
both syscalls run on the same CPU does the LIFO allocator return the stale
object and trigger the assertion.

Should this arm either pin the thread with sched_setaffinity() around the
delete/create pair, or loop over all CPUs to make the result deterministic?

> +
> +static void test_lru_percpu_hash_cpu_flag_create(void)
> +{
> +	/* lru without prealloc is -ENOTSUPP, so there is no malloc variant */
> +	test_percpu_map_cpu_flag_create(BPF_MAP_TYPE_LRU_PERCPU_HASH, 0);
> +}

[ ... ]


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