[PATCH bpf-next 0/2] bpf: Cancel special fields in resizable hashtab on recycle

[PATCH bpf-next 0/2] bpf: Cancel special fields in resizable hashtab on recycle

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

The resizable hashtab (BPF_MAP_TYPE_RHASH) still eagerly calls
bpf_obj_free_fields() when an element is deleted or its value is
replaced, which runs kptr destructors in the caller's execution context.
This is unsafe for BPF programs running in NMI context (e.g. perf_event
programs attached to hardware PMU overflows), where referenced kptr
destructors may take locks or otherwise cannot run.

Commit a3a81d247651 ("bpf: Cancel special fields on map value recycle")
switched the hash map and array recycle paths to bpf_obj_cancel_fields(),
which only cancels NMI-safe fields (timer, workqueue, task_work), but it
missed the resizable hashtab. Patch 1 applies the same semantics to
rhtab; patch 2 adds a regression test.

Yuan Chen (2):
  bpf: Cancel special fields in resizable hashtab on recycle
  selftests/bpf: Test rhtab kptr recycle from NMI context

 kernel/bpf/hashtab.c                          |  28 +++--
 .../selftests/bpf/prog_tests/rhtab_kptr.c     |  87 +++++++++++++++
 .../testing/selftests/bpf/progs/rhtab_kptr.c  | 101 ++++++++++++++++++
 3 files changed, 207 insertions(+), 9 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_kptr.c

-- 
2.54.0

[PATCH bpf-next 1/2] bpf: Cancel special fields in resizable hashtab on recycle

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

rhtab_delete_elem() and rhtab_map_update_existing() eagerly call
bpf_obj_free_fields() when an element is deleted or its value is
replaced. This runs kptr destructors (and unpins uptrs, frees list/rbtree
roots) in the caller's execution context, which is unsafe for BPF
programs running in NMI context (e.g. perf_event programs attached to
hardware PMU overflows): referenced kptr destructors may take locks or
otherwise cannot run in NMI.

Commit a3a81d247651 ("bpf: Cancel special fields on map value recycle")
switched the hash map and array recycle paths to bpf_obj_cancel_fields(),
which only cancels NMI-safe fields (timer, workqueue, task_work), but it
missed the resizable hashtab. rhtab_map_update_existing() even documents
the intended "cancel" semantics while still calling bpf_obj_free_fields().

Fix by cancelling only NMI-safe fields on rhtab update/delete and leaving
referenced kptrs (and other fields requiring full destruction) attached
to the recycled element, to be destroyed by rhtab_mem_dtor() when the
element is eventually freed. This matches the hash map semantics and
keeps the element's kptr reference accounting balanced: the reference
stays owned by the element and is released exactly once by the final
destruction path.

Verified with a selftest (perf_event NMI program overwriting a rhtab
element that holds a referenced task kptr): before this patch the NMI
update releases the kptr and the subsequent probe observes NULL in the
slot; after the patch the kptr is inherited and the probe observes it
non-NULL.

Fixes: a3a81d247651 ("bpf: Cancel special fields on map value recycle")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/hashtab.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 9f394e1aa2e8..70fa1e1b4b00 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -2865,14 +2865,23 @@ static int rhtab_map_alloc_check(union bpf_attr *attr)
 	return htab_map_alloc_check(attr);
 }
 
-static void rhtab_check_and_free_fields(struct bpf_rhtab *rhtab,
-					struct rhtab_elem *elem)
+static void rhtab_cancel_fields(struct bpf_rhtab *rhtab,
+				struct rhtab_elem *elem)
 {
 	if (IS_ERR_OR_NULL(rhtab->map.record))
 		return;
 
-	bpf_obj_free_fields(rhtab->map.record,
-			    rhtab_elem_value(elem, rhtab->map.key_size));
+	/*
+	 * Only cancel NMI-safe fields (timer, workqueue, task_work) here.
+	 * kptr/uptr/list/rbtree destruction must not run from arbitrary BPF
+	 * execution contexts (e.g. NMI), so leave those fields attached to
+	 * the recycled element and let rhtab_mem_dtor() destroy them once the
+	 * element is eventually freed. This matches the hash map semantics
+	 * introduced by a3a81d247651 ("bpf: Cancel special fields on map value
+	 * recycle").
+	 */
+	bpf_map_free_internal_structs(&rhtab->map,
+				      rhtab_elem_value(elem, rhtab->map.key_size));
 }
 
 static void rhtab_mem_dtor(void *obj, void *ctx)
@@ -2964,8 +2973,8 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
 		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
 		check_and_init_map_value(&rhtab->map, copy);
 	}
-	/* Release internal structs: kptr, bpf_timer, task_work, wq */
-	rhtab_check_and_free_fields(rhtab, elem);
+	/* Cancel NMI-safe fields; full destruction happens in rhtab_mem_dtor */
+	rhtab_cancel_fields(rhtab, elem);
 	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
 	return 0;
 }
@@ -3024,10 +3033,11 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
 	 * BPF_F_LOCK, matching arraymap semantics.
 	 *
 	 * copy_map_value() skips special-field offsets, so old timers/
-	 * kptrs/etc. still sit in the slot. Cancel them after the copy
-	 * to match arraymap's update semantics.
+	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
+	 * the copy to match arraymap's update semantics; referenced kptrs
+	 * stay attached and are destroyed by rhtab_mem_dtor().
 	 */
-	rhtab_check_and_free_fields(rhtab, elem);
+	rhtab_cancel_fields(rhtab, elem);
 	return 0;
 }
 
-- 
2.54.0

Re: [PATCH bpf-next 1/2] bpf: Cancel special fields in resizable hashtab on recycle

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 9f394e1aa2e8..70fa1e1b4b00 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -2865,14 +2865,23 @@ static int rhtab_map_alloc_check(union bpf_attr *attr)
>  	return htab_map_alloc_check(attr);
>  }
>
> -static void rhtab_check_and_free_fields(struct bpf_rhtab *rhtab,
> -					struct rhtab_elem *elem)
> +static void rhtab_cancel_fields(struct bpf_rhtab *rhtab,
> +				struct rhtab_elem *elem)
>  {
>  	if (IS_ERR_OR_NULL(rhtab->map.record))
>  		return;
>
> -	bpf_obj_free_fields(rhtab->map.record,
> -			    rhtab_elem_value(elem, rhtab->map.key_size));
> +	/*
> +	 * Only cancel NMI-safe fields (timer, workqueue, task_work) here.
> +	 * kptr/uptr/list/rbtree destruction must not run from arbitrary BPF
> +	 * execution contexts (e.g. NMI), so leave those fields attached to
> +	 * the recycled element and let rhtab_mem_dtor() destroy them once the
> +	 * element is eventually freed. This matches the hash map semantics
> +	 * introduced by a3a81d247651 ("bpf: Cancel special fields on map value
> +	 * recycle").
> +	 */
> +	bpf_map_free_internal_structs(&rhtab->map,
> +				      rhtab_elem_value(elem, rhtab->map.key_size));
>  }

This isn't a bug, but the comment lists field types that BPF_MAP_TYPE_RHASH
values cannot contain. In map_check_btf(), BPF_UPTR is restricted to
BPF_MAP_TYPE_TASK_STORAGE, and BPF_LIST_HEAD/BPF_RB_ROOT are allowed only
in BPF_MAP_TYPE_HASH, BPF_MAP_TYPE_LRU_HASH and BPF_MAP_TYPE_ARRAY.
BPF_MAP_TYPE_RHASH is absent from both lists.

The only field types that bpf_obj_free_fields() actually handles beyond
the cancelled ones are BPF_KPTR_REF and BPF_KPTR_PERCPU. Could the
comment be more precise about what this code path actually affects?

>
>  static void rhtab_mem_dtor(void *obj, void *ctx)

[ ... ]

> @@ -2964,8 +2973,8 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
>  		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
>  		check_and_init_map_value(&rhtab->map, copy);
>  	}
> -	/* Release internal structs: kptr, bpf_timer, task_work, wq */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	/* Cancel NMI-safe fields; full destruction happens in rhtab_mem_dtor */
> +	rhtab_cancel_fields(rhtab, elem);
>  	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
>  	return 0;
>  }

Does this leak referenced kptrs when a deleted rhtab element is recycled
by the bpf memory allocator?

Unlike htab, rhtab re-initialises special fields of every freshly
allocated element. In rhtab_map_update_elem():

    elem = bpf_mem_cache_alloc(&rhtab->ma);
    ...
    copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);
    check_and_init_map_value(map, rhtab_elem_value(elem, map->key_size));

check_and_init_map_value() calls through to bpf_obj_init_field(), which
does memset(addr, 0, field->size) for BPF_KPTR_REF fields, overwriting
the pointer without releasing the reference.

Before this patch that was safe: rhtab_delete_elem() called
rhtab_check_and_free_fields(), which ran bpf_obj_free_fields() to xchg
the kptr out and drop it, so the recycled element arrived with the field
already zeroed. After the patch rhtab_cancel_fields() only touches
BPF_TIMER/BPF_WORKQUEUE/BPF_TASK_WORK via bpf_map_free_internal_structs(),
so the live kptr stays in the element.

Can this sequence occur?

  1. BPF prog stores a referenced kptr in an RHASH element (allowed:
     map_check_btf() lists BPF_MAP_TYPE_RHASH in the BPF_KPTR_REF
     allowlist).
  2. bpf_map_delete_elem()->rhtab_map_delete_elem()->rhtab_delete_elem()
     cancels only NMI-safe fields, then bpf_mem_cache_free_rcu().
  3. unit_free_rcu() puts the element on c->free_by_rcu; after a grace
     period it lands on c->free_llist.
  4. Next insert: rhtab_map_update_elem()->bpf_mem_cache_alloc()->
     unit_alloc() pops that element off c->free_llist.
  5. check_and_init_map_value() memsets the kptr field to 0. The
     reference is now unreachable and is never dropped.

The dtor cannot save it: rhtab_mem_dtor() is only reached from
free_all() in memalloc.c, which runs immediately before free_one()
returns the object to the slab, never on the unit_alloc() reuse path.

Why htab is not affected: alloc_htab_elem() has no
check_and_init_map_value() call, so a recycled htab element genuinely
inherits the kptr, which is what a3a81d247651 relies on. Does rhtab
break that pairing?

__rhtab_map_lookup_and_delete_batch() amplifies it: the loop at line
3363 calls rhtab_delete_elem() up to attr->batch.count times, leaking
one reference per element.

> @@ -3024,10 +3033,11 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * BPF_F_LOCK, matching arraymap semantics.
>  	 *
>  	 * copy_map_value() skips special-field offsets, so old timers/
> -	 * kptrs/etc. still sit in the slot. Cancel them after the copy
> -	 * to match arraymap's update semantics.
> +	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
> +	 * the copy to match arraymap's update semantics; referenced kptrs
> +	 * stay attached and are destroyed by rhtab_mem_dtor().
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	rhtab_cancel_fields(rhtab, elem);
>  	return 0;
>  }

A subsystem pattern flags this as potentially concerning:
rhtab_map_update_existing() overwrites a live map value with
copy_map_value()/copy_map_value_locked() and, after this patch, no
longer calls bpf_obj_free_fields() on the old value.

BPF_MAP_TYPE_RHASH is in the map_check_btf() allowlists for
BPF_SPIN_LOCK/BPF_RES_SPIN_LOCK, BPF_TIMER/BPF_WORKQUEUE/BPF_TASK_WORK
and BPF_KPTR_UNREF/BPF_KPTR_REF/BPF_KPTR_PERCPU/BPF_REFCOUNT, making it
a field-capable map type. The subsystem guide notes that map operations
on field-capable types that copy values with copy_map_value() should
pair with check_and_init_map_value() (lookups) or bpf_obj_free_fields()
(updates).

Observable effect: after a successful update the element's kptr slot
still holds the previous value's kptr, which the new value inherits. A
program that writes a fresh value and then reads the kptr field sees a
pointer it never installed.

The removal appears deliberate and mirrors upstream a3a81d247651, which
made exactly this change for array_map_update_elem() and
pcpu_copy_value() so that kptr destructors are not run from NMI. That
commit's changelog explicitly accepts the relaxed semantics: "This
deliberately relaxes the eager-free semantics of map update/delete for
special fields." On that reading the field inheritance here would be
intended rather than a bug. Is that the correct interpretation?


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

[PATCH bpf-next 2/2] selftests/bpf: Test rhtab kptr recycle from NMI context

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

A perf_event program running in NMI context overwrites a rhtab element
whose value holds a referenced task kptr. The old kptr must stay attached
to the element (cancel semantics, matching hash maps); before the rhtab
recycle fix the NMI update eagerly released it and the probe observed
NULL.

The test skips when no hardware PMU is available.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 .../selftests/bpf/prog_tests/rhtab_kptr.c     |  87 +++++++++++++++
 .../testing/selftests/bpf/progs/rhtab_kptr.c  | 101 ++++++++++++++++++
 2 files changed, 188 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_kptr.c

diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
new file mode 100644
index 000000000000..79bdcda95555
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+#include <linux/perf_event.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+#include <test_progs.h>
+#include "rhtab_kptr.skel.h"
+
+static __u64 read_counter(struct rhtab_kptr *skel, u32 idx)
+{
+	__u64 vals[libbpf_num_possible_cpus()];
+	__u64 sum = 0;
+	int i, err;
+
+	err = bpf_map_lookup_elem(bpf_map__fd(skel->maps.counters), &idx, vals);
+	if (!ASSERT_OK(err, "lookup_counter"))
+		return 0;
+	for (i = 0; i < libbpf_num_possible_cpus(); i++)
+		sum += vals[i];
+	return sum;
+}
+
+void test_rhtab_kptr(void)
+{
+	struct perf_event_attr attr = {
+		.type = PERF_TYPE_HARDWARE,
+		.config = PERF_COUNT_HW_CPU_CYCLES,
+		.freq = 1,
+		.sample_freq = read_perf_max_sample_freq(),
+		.size = sizeof(struct perf_event_attr),
+	};
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	struct rhtab_kptr *skel;
+	__u32 key = 0;
+	__u64 zero = 0;
+	int pmu_fd;
+
+	skel = rhtab_kptr__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+
+	/* Create the element and stash a referenced task kptr in it. */
+	if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.rhtab),
+					   &key, &zero, BPF_ANY), "create_elem"))
+		goto out;
+	if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.init_elem),
+					      &topts), "test_run_init") ||
+	    !ASSERT_EQ(topts.retval, 0, "init_ret"))
+		goto out;
+
+	pmu_fd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, 0);
+	if (pmu_fd < 0) {
+		test__skip();
+		goto out;
+	}
+	skel->links.nmi_update = bpf_program__attach_perf_event(skel->progs.nmi_update,
+								pmu_fd);
+	if (!ASSERT_OK_PTR(skel->links.nmi_update, "attach_perf_event")) {
+		close(pmu_fd);
+		goto out;
+	}
+
+	/* Let the NMI handler overwrite the element. */
+	usleep(100000);
+
+	bpf_link__destroy(skel->links.nmi_update);
+	skel->links.nmi_update = NULL;
+	close(pmu_fd);
+
+	/*
+	 * The old kptr must still be attached to the element: the NMI update
+	 * path only cancels NMI-safe fields, mirroring hash map semantics.
+	 * Before the fix the kptr was released from the NMI context and the
+	 * probe below would see NULL.
+	 */
+	topts.retval = 0;
+	if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.probe_elem),
+					      &topts), "test_run_probe") ||
+	    !ASSERT_EQ(topts.retval, 0, "probe_ret"))
+		goto out;
+
+	ASSERT_EQ(read_counter(skel, 2), 1, "xchg_non_null");
+	ASSERT_EQ(read_counter(skel, 3), 0, "xchg_null");
+out:
+	rhtab_kptr__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/rhtab_kptr.c b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
new file mode 100644
index 000000000000..25800a2e24e6
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+/*
+ * Verify that a rhtab value update issued from a perf_event (NMI) program
+ * does not eagerly destroy referenced kptrs. rhtab must match the hash map
+ * semantics introduced by commit a3a81d247651 ("bpf: Cancel special fields
+ * on map value recycle"): only NMI-safe fields (timer, workqueue, task_work)
+ * are cancelled on update/delete, while kptrs stay attached to the recycled
+ * element until it is eventually freed.
+ *
+ * Without the fix, the NMI update releases the old kptr and probe_elem()
+ * observes NULL in the slot; with the fix the old kptr is inherited and
+ * probe_elem() observes a non-NULL pointer.
+ */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+char LICENSE[] SEC("license") = "GPL";
+
+struct val_t {
+	struct task_struct __kptr *tsk;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct val_t);
+} rhtab SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(max_entries, 4);
+	__type(key, __u32);
+	__type(value, __u64);
+} counters SEC(".maps");
+
+/* 0: init ok, 1: nmi update ok, 2: probe xchg non-NULL, 3: probe xchg NULL */
+static __always_inline void bump(u32 idx)
+{
+	u64 *v = bpf_map_lookup_elem(&counters, &idx);
+
+	if (v)
+		(*v)++;
+}
+
+extern struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+extern void bpf_task_release(struct task_struct *p) __ksym;
+
+SEC("perf_event")
+int nmi_update(struct bpf_perf_event_data *ctx)
+{
+	struct val_t val = {};
+	u32 key = 0;
+
+	if (bpf_map_update_elem(&rhtab, &key, &val, BPF_ANY) == 0)
+		bump(1);
+	return 0;
+}
+
+SEC("syscall")
+int init_elem(void *ctx)
+{
+	struct val_t *val;
+	struct task_struct *task, *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&rhtab, &key);
+	if (!val)
+		return 1;
+	task = bpf_task_acquire(bpf_get_current_task_btf());
+	if (!task)
+		return 2;
+	old = bpf_kptr_xchg(&val->tsk, task);
+	if (old)
+		bpf_task_release(old);
+	bump(0);
+	return 0;
+}
+
+SEC("syscall")
+int probe_elem(void *ctx)
+{
+	struct val_t *val;
+	struct task_struct *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&rhtab, &key);
+	if (!val)
+		return 1;
+	old = bpf_kptr_xchg(&val->tsk, NULL);
+	if (old) {
+		bpf_task_release(old);
+		bump(2);
+	} else {
+		bump(3);
+	}
+	return 0;
+}
-- 
2.54.0

Re: [PATCH bpf-next 2/2] selftests/bpf: Test rhtab kptr recycle from NMI context

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
> new file mode 100644
> index 000000000000..79bdcda95555
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
> @@ -0,0 +1,87 @@

[ ... ]

> +	pmu_fd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, 0);
> +	if (pmu_fd < 0) {
> +		test__skip();
> +		goto out;
> +	}

This isn't a bug, but should the test distinguish between the expected
no-hardware-PMU skip case (ENOENT, EOPNOTSUPP) and unexpected errors
(EACCES, EMFILE)? lru_lock_nmi.c checks errno before skipping:

    if (pmu_fd < 0) {
        if (i == 0 && (errno == ENOENT || errno == EOPNOTSUPP)) {
                test__skip();
                goto cleanup;
        }
        continue;
    }

Right now a real setup failure produces the same SKIP outcome as a
missing PMU, making it harder to debug.

> +	skel->links.nmi_update = bpf_program__attach_perf_event(skel->progs.nmi_update,
> +								pmu_fd);
> +	if (!ASSERT_OK_PTR(skel->links.nmi_update, "attach_perf_event")) {
> +		close(pmu_fd);
> +		goto out;
> +	}
> +
> +	/* Let the NMI handler overwrite the element. */
> +	usleep(100000);
> +
> +	bpf_link__destroy(skel->links.nmi_update);
> +	skel->links.nmi_update = NULL;
> +	close(pmu_fd);

Can this close a stale descriptor?

bpf_program__attach_perf_event() takes ownership of pfd on success: the
returned link stores it in bpf_link_perf->perf_event_fd, and
bpf_link_perf_detach() in libbpf closes it on both attach paths:

    if (perf_link->perf_event_fd != link->fd)
            close(perf_link->perf_event_fd);
    close(link->fd);

On the FEAT_PERF_LINK path link->fd is a separate bpf_link fd, so pfd
is closed by the first close(); on the legacy ioctl path link->fd ==
pfd, so it is closed by the second. Either way, after
bpf_link__destroy() the fd in pmu_fd is already closed, and
close(pmu_fd) operates on a stale descriptor number. Today it only
returns EBADF, but if any code added in between (or any concurrent
thread in the same test_progs worker) allocates an fd, that fd can be
closed out from under it.

The error path close(pmu_fd) after a failed attach is correct — the
attach opts only close link_fd, never pfd — so only this success-path
close is wrong.

Every comparable test in the same directory closes pmu_fd only on
attach failure and never after success: perf_event_stackmap.c:99-104,
stacktrace_build_id_nmi.c:47-52, and lru_lock_nmi.c (which states
"libbpf takes ownership of pfd on success" and only calls
bpf_link__destroy()). Should the close(pmu_fd) be dropped here?

> +	/*
> +	 * The old kptr must still be attached to the element: the NMI update
> +	 * path only cancels NMI-safe fields, mirroring hash map semantics.
> +	 * Before the fix the kptr was released from the NMI context and the
> +	 * probe below would see NULL.
> +	 */
> +	topts.retval = 0;
> +	if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.probe_elem),
> +					      &topts), "test_run_probe") ||
> +	    !ASSERT_EQ(topts.retval, 0, "probe_ret"))
> +		goto out;
> +
> +	ASSERT_EQ(read_counter(skel, 2), 1, "xchg_non_null");
> +	ASSERT_EQ(read_counter(skel, 3), 0, "xchg_null");

Does the test actually detect the bug it is meant to catch?

The BPF side bumps counters[1] on every successful NMI update, but the
harness never reads index 1 — it only checks indexes 2 and 3. If the
perf event never overflows during the 100 ms window, no rhtab update
happens at all, the kptr stashed by init_elem is trivially still there,
probe_elem takes the non-NULL branch, and both assertions
(xchg_non_null == 1, xchg_null == 0) pass — on a kernel with or without
the fix. The "no update happened" outcome and the "update happened and
correctly preserved the kptr" outcome are indistinguishable to this
test.

That outcome is plausible: the test generates no work on the monitored
CPU. The event is opened system-wide but for CPU 0 only
(syscall(__NR_perf_event_open, &attr, -1, 0, -1, 0)), the process is
never pinned to CPU 0, and the only thing it does while attached is
usleep(100000) — i.e. it sleeps. A CPU 0 that is idle in a deep C-state
does not advance PERF_COUNT_HW_CPU_CYCLES, so no sample and no NMI.
Whether the test exercises anything then depends on unrelated activity
landing on CPU 0.

Both existing NMI perf_event tests in the same directory do the two
things this one omits:

  - lru_lock_nmi.c:230 asserts the program actually ran:
    ASSERT_GT(skel->bss->hits, 0, "nmi_bpf_ran"), and drives load with
    per-CPU hammer threads;
  - stacktrace_build_id_nmi.c:61 pins its workload onto the monitored
    CPU: system("taskset 0x1 ./urandom_read 100000").

Should the test assert read_counter(skel, 1) > 0 before the kptr
assertions, and generate cycles on the monitored CPU rather than
sleeping?

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/rhtab_kptr.c b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
> new file mode 100644
> index 000000000000..25800a2e24e6
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 KylinSoft Co., Ltd. */

The commit adds a new BPF-program file progs/rhtab_kptr.c plus a new
prog_tests/rhtab_kptr.c for a single narrow rhtab behaviour (kptr
retention across an NMI-context value update). The same directory
already contains progs/rhash.c / prog_tests/rhash.c, which is the
general BPF_MAP_TYPE_RHASH test and already carries a
test__start_subtest() table for rhtab update/delete/exist semantics
(test_rhash_lookup_update, test_rhash_update_delete,
test_rhash_update_exist, test_rhash_update_any, ...), all driven from
SEC("syscall") programs against an RHASH map. Would adding one more
subtest plus one more program to that existing pair avoid a second
skeleton, a second map set and duplicated setup?

Counter-argument: progs/rhash.c has no kptr coverage at all (no __kptr
field, no map record) and prog_tests/map_kptr.c has no RHASH coverage,
so neither existing test covers this behaviour today, and the new test
additionally needs a SEC("perf_event") program which the rhash.c
skeleton does not have. On that reading the new file is a genuinely new
behaviour rather than a narrow variant of existing coverage, and a
separate file is defensible.

> +/* 0: init ok, 1: nmi update ok, 2: probe xchg non-NULL, 3: probe xchg NULL */
> +static __always_inline void bump(u32 idx)
> +{
> +	u64 *v = bpf_map_lookup_elem(&counters, &idx);
> +
> +	if (v)
> +		(*v)++;
> +}
> +
> +extern struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
> +extern void bpf_task_release(struct task_struct *p) __ksym;
> +
> +SEC("perf_event")
> +int nmi_update(struct bpf_perf_event_data *ctx)
> +{
> +	struct val_t val = {};
> +	u32 key = 0;
> +
> +	if (bpf_map_update_elem(&rhtab, &key, &val, BPF_ANY) == 0)
> +		bump(1);
> +	return 0;
> +}

The counters map is documented as

  /* 0: init ok, 1: nmi update ok, 2: probe xchg non-NULL, 3: probe xchg NULL */

but only slots 2 and 3 are ever consumed. The userspace side
(prog_tests/rhtab_kptr.c) reads exactly read_counter(skel, 2) and
read_counter(skel, 3); slots 0 and 1 are written and never read.

Slot 1 is the only evidence that nmi_update() ran at all, and dropping
it makes the test unable to distinguish "the NMI update ran and the
kptr survived" from "the PMU never overflowed, so nothing happened".
The userspace side only waits usleep(100000) on a system-wide
PERF_TYPE_HARDWARE/PERF_COUNT_HW_CPU_CYCLES event pinned to CPU 0 while
the test thread is asleep; if CPU 0 stays idle (or the event is
virtualised/throttled) no sample is delivered. In that case probe_elem()
still finds the kptr installed by init_elem(), bump(2) fires once, and
both assertions

    ASSERT_EQ(read_counter(skel, 2), 1, "xchg_non_null");
    ASSERT_EQ(read_counter(skel, 3), 0, "xchg_null");

pass on a kernel that still has the eager bpf_obj_free_fields() in
rhtab_map_update_existing() — i.e. the test silently passes on the very
kernel it is meant to catch.

Would asserting read_counter(skel, 1) > 0 (and dropping the unused slot
0, whose information is already covered by topts.retval) close the gap
and turn a non-firing PMU into an explicit failure or skip instead of a
false pass?


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

Re: [PATCH bpf-next 0/2] bpf: Cancel special fields in resizable hashtab on recycle

[Kumar Kartikeya Dwivedi]

On Tue Aug 11, 2026 at 11:55 AM CEST, chenyuan_fl wrote:
> From: Yuan Chen <chenyuan@kylinos.cn>
>
> The resizable hashtab (BPF_MAP_TYPE_RHASH) still eagerly calls
> bpf_obj_free_fields() when an element is deleted or its value is
> replaced, which runs kptr destructors in the caller's execution context.
> This is unsafe for BPF programs running in NMI context (e.g. perf_event
> programs attached to hardware PMU overflows), where referenced kptr
> destructors may take locks or otherwise cannot run.
>
> Commit a3a81d247651 ("bpf: Cancel special fields on map value recycle")
> switched the hash map and array recycle paths to bpf_obj_cancel_fields(),
> which only cancels NMI-safe fields (timer, workqueue, task_work), but it
> missed the resizable hashtab. Patch 1 applies the same semantics to
> rhtab; patch 2 adds a regression test.
>
> Yuan Chen (2):
>   bpf: Cancel special fields in resizable hashtab on recycle
>   selftests/bpf: Test rhtab kptr recycle from NMI context
>

At least go through the mailing list before sending patches.

https://lore.kernel.org/bpf/DKEVG8ZVJDDQ.2G40FTOIZKU57@gmail.com

pw-bot: cr

>  kernel/bpf/hashtab.c                          |  28 +++--
>  .../selftests/bpf/prog_tests/rhtab_kptr.c     |  87 +++++++++++++++
>  .../testing/selftests/bpf/progs/rhtab_kptr.c  | 101 ++++++++++++++++++
>  3 files changed, 207 insertions(+), 9 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
>  create mode 100644 tools/testing/selftests/bpf/progs/rhtab_kptr.c

[PATCH bpf-next v2 0/4] bpf: Cancel special fields in resizable hashtab on recycle

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

Posted in reply to Kumar Kartikeya Dwivedi's review [1] of Nuoqi Gui's
overlapping series [2]: switching rhtab to bpf_obj_cancel_fields() alone
leaks referenced kptrs, because rhtab_map_update_elem() zeroes the kptr
slot of a recycled element via check_and_init_map_value() before the
allocator destructor can release it.

The resizable hashtab still eagerly calls bpf_obj_free_fields() on element
delete/replace, which runs kptr destructors from the caller's execution
context (unsafe in NMI). Patch 1 applies the cancel semantics like hash/array
maps (a3a81d247651) and fixes the recycle-path kptr leak with
rhtab_init_map_value(); patch 2 fixes a program-BTF use-after-free in the
mem-alloc destructor found while testing; patches 3-4 add regression tests
(NMI update, and per-field delete/re-insert cycles).

Verified in QEMU (KASAN, current bpf-next): rhtab_kptr, rhtab_fields (4/4)
and rhash pass with the fix; on the unfixed kernel the kptr subtests fail at
the recycle assertions and the program-BTF UAF reproduces.

Changes since v1: fix the recycle-path kptr leak, extend the selftests with
delete/re-insert recycle coverage, add per-field combination tests, fix the
program-BTF UAF (patch 2).

[1] https://lore.kernel.org/bpf/DKEVG8ZVJDDQ.2G40FTOIZKU57@gmail.com/
[2] https://lore.kernel.org/bpf/20260726-f01-23-rhash-cancel-bpf-next-v1-0-6e5e1131d885@mails.tsinghua.edu.cn/

Yuan Chen (4):
  bpf: Cancel special fields in resizable hashtab on recycle
  bpf: Fix use-after-free of program BTF in mem-alloc destructor
  selftests/bpf: Test rhtab kptr recycle from NMI context
  selftests/bpf: Test rhtab special-field combinations

 kernel/bpf/hashtab.c                          | 115 ++++++-
 .../selftests/bpf/prog_tests/rhtab_fields.c   | 213 ++++++++++++
 .../selftests/bpf/prog_tests/rhtab_kptr.c     | 146 +++++++++
 .../selftests/bpf/progs/rhtab_fields.c        | 305 ++++++++++++++++++
 .../testing/selftests/bpf/progs/rhtab_kptr.c  | 132 ++++++++
 5 files changed, 901 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_fields.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_kptr.c

-- 
2.54.0

[PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

rhtab_delete_elem() and rhtab_map_update_existing() eagerly call
bpf_obj_free_fields() when an element is deleted or its value is
replaced. This runs kptr destructors in the caller's execution
context, which is unsafe for BPF programs running in NMI context
(e.g. perf_event programs attached to hardware PMU overflows):
referenced kptr destructors may take locks or otherwise cannot run
in NMI.

Commit a3a81d247651 ("bpf: Cancel special fields on map value
recycle") switched the hash map and array recycle paths to
bpf_obj_cancel_fields(), which only cancels NMI-safe fields (timer,
workqueue, task_work), but it missed the resizable hashtab.
rhtab_map_update_existing() even documents the intended "cancel"
semantics while still calling bpf_obj_free_fields().

Fix the resizable hashtab the same way:

  * rhtab_delete_elem() and rhtab_map_update_existing() now cancel
    only NMI-safe fields. Referenced kptrs stay attached to the
    recycled element and are destroyed by rhtab_mem_dtor() once the
    element is eventually freed, keeping the reference accounting
    balanced.

  * rhtab_map_update_elem() initializes the special fields of a
    freshly allocated element. The bpf memory allocator may return a
    recycled element that still owns a referenced kptr, and
    check_and_init_map_value() would zero that slot, dropping the
    reference without releasing it. rhtab_init_map_value()
    initializes the remaining fields (spin lock, timer, workqueue,
    task_work, refcount) but leaves kptr slots untouched, matching
    the hash map semantics.

Verified with a selftest: a perf_event (NMI) program overwrites a
rhtab element that holds a referenced task kptr, and a second phase
deletes and re-inserts the element to exercise the recycle path.
Before the patch the NMI update eagerly released the kptr and the
recycle path zeroed the inherited slot; after the patch the kptr is
inherited on both paths and the probe observes it non-NULL.

Fixes: a3a81d247651 ("bpf: Cancel special fields on map value recycle")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/hashtab.c | 70 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 60 insertions(+), 10 deletions(-)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index d40cb5dd446c..0df8db27cd8c 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -2864,14 +2864,56 @@ static int rhtab_map_alloc_check(union bpf_attr *attr)
 	return htab_map_alloc_check(attr);
 }
 
-static void rhtab_check_and_free_fields(struct bpf_rhtab *rhtab,
-					struct rhtab_elem *elem)
+static void rhtab_cancel_fields(struct bpf_rhtab *rhtab,
+				struct rhtab_elem *elem)
 {
 	if (IS_ERR_OR_NULL(rhtab->map.record))
 		return;
 
-	bpf_obj_free_fields(rhtab->map.record,
-			    rhtab_elem_value(elem, rhtab->map.key_size));
+	/*
+	 * Only cancel NMI-safe fields (timer, workqueue, task_work) here.
+	 * RHASH values can also carry referenced kptrs (and per-cpu kptrs),
+	 * whose destructors must not run from arbitrary BPF execution
+	 * contexts (e.g. NMI); leave them attached to the recycled element
+	 * and let rhtab_mem_dtor() destroy them once the element is
+	 * eventually freed. This matches the hash map semantics introduced
+	 * by a3a81d247651 ("bpf: Cancel special fields on map value
+	 * recycle").
+	 */
+	bpf_map_free_internal_structs(&rhtab->map,
+				      rhtab_elem_value(elem, rhtab->map.key_size));
+}
+
+/*
+ * Initialize special fields of a freshly allocated rhtab element, but keep
+ * kptr fields untouched. A recycled element may carry a referenced kptr from
+ * its previous life: the delete path only cancels NMI-safe fields (matching
+ * the hash map semantics), so the kptr reference stays owned by the element
+ * until rhtab_mem_dtor() destroys it. Zeroing it here (as
+ * check_and_init_map_value() would) would drop the reference without
+ * releasing it.
+ */
+static void rhtab_init_map_value(struct bpf_map *map, void *value)
+{
+	struct btf_record *rec = map->record;
+	int i;
+
+	if (IS_ERR_OR_NULL(rec))
+		return;
+
+	for (i = 0; i < rec->cnt; i++) {
+		struct btf_field *field = &rec->fields[i];
+		void *field_ptr = value + field->offset;
+
+		switch (field->type) {
+		case BPF_KPTR_UNREF:
+		case BPF_KPTR_REF:
+		case BPF_KPTR_PERCPU:
+			continue;
+		default:
+			bpf_obj_init_field(field, field_ptr);
+		}
+	}
 }
 
 static void rhtab_mem_dtor(void *obj, void *ctx)
@@ -2963,8 +3005,8 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
 		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
 		check_and_init_map_value(&rhtab->map, copy);
 	}
-	/* Release internal structs: kptr, bpf_timer, task_work, wq */
-	rhtab_check_and_free_fields(rhtab, elem);
+	/* Cancel NMI-safe fields; full destruction happens in rhtab_mem_dtor */
+	rhtab_cancel_fields(rhtab, elem);
 	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
 	return 0;
 }
@@ -3022,10 +3064,11 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
 	 * BPF_F_LOCK, matching arraymap semantics.
 	 *
 	 * copy_map_value() skips special-field offsets, so old timers/
-	 * kptrs/etc. still sit in the slot. Cancel them after the copy
-	 * to match arraymap's update semantics.
+	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
+	 * the copy to match arraymap's update semantics; referenced kptrs
+	 * stay attached and are destroyed by rhtab_mem_dtor().
 	 */
-	rhtab_check_and_free_fields(rhtab, elem);
+	rhtab_cancel_fields(rhtab, elem);
 	return 0;
 }
 
@@ -3066,7 +3109,14 @@ static long rhtab_map_update_elem(struct bpf_map *map, void *key, void *value, u
 
 	memcpy(elem->data, key, map->key_size);
 	copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);
-	check_and_init_map_value(map, rhtab_elem_value(elem, map->key_size));
+	/*
+	 * Initialize special fields of the (possibly recycled) element, but
+	 * leave kptr slots alone: a recycled element may still own a
+	 * referenced kptr that rhtab_mem_dtor() will release, so zeroing it
+	 * here would leak the reference. Fresh memory from the bpf mem
+	 * allocator is zeroed, so skipping the kptr init is safe there too.
+	 */
+	rhtab_init_map_value(map, rhtab_elem_value(elem, map->key_size));
 
 	/* Prevent deadlock for NMI programs attempting to take bucket lock */
 	bpf_disable_instrumentation();
-- 
2.54.0

Re: [PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing `bpf_obj_free_fields()` in map updates, directly violating BPF-001 guidelines.
--

--- Patch [1]: [PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle ---
commit 6f12e4dc235993c2415d956e168783cb133ed13a
Author: Yuan Chen <chenyuan@kylinos.cn>

bpf: Cancel special fields in resizable hashtab on recycle

This commit updates the resizable hashtab recycle paths to cancel only
NMI-safe fields instead of fully freeing all fields. This is intended to
prevent kptr destructors from running in NMI contexts during map updates or
deletions, delaying their destruction until the element is actually freed.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
[ ... ]
> @@ -3022,10 +3064,11 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * BPF_F_LOCK, matching arraymap semantics.
>  	 *
>  	 * copy_map_value() skips special-field offsets, so old timers/
> -	 * kptrs/etc. still sit in the slot. Cancel them after the copy
> -	 * to match arraymap's update semantics.
> +	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
> +	 * the copy to match arraymap's update semantics; referenced kptrs
> +	 * stay attached and are destroyed by rhtab_mem_dtor().
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	rhtab_cancel_fields(rhtab, elem);

[Severity: Medium]
Does this bypass the required resource cleanup for overwritten map values?

The BPF subsystem guidelines mandate that when overwriting an existing map
value using copy_map_value(), bpf_obj_free_fields() must be called to release
resources held by the old value before or after the copy overwrites them.

Since rhtab_cancel_fields() is used here instead of bpf_obj_free_fields(),
could this leak kptrs or other resources when the old value is overwritten
during a map update?

>  	return 0;
>  }

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

Re: [PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index d40cb5dd446c..0df8db27cd8c 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -2864,14 +2864,56 @@ static int rhtab_map_alloc_check(union bpf_attr *attr)
>  	return htab_map_alloc_check(attr);
>  }
>
> -static void rhtab_check_and_free_fields(struct bpf_rhtab *rhtab,
> -					struct rhtab_elem *elem)
> +static void rhtab_cancel_fields(struct bpf_rhtab *rhtab,
> +				struct rhtab_elem *elem)
>  {
>  	if (IS_ERR_OR_NULL(rhtab->map.record))
>  		return;
>
> -	bpf_obj_free_fields(rhtab->map.record,
> -			    rhtab_elem_value(elem, rhtab->map.key_size));
> +	/*
> +	 * Only cancel NMI-safe fields (timer, workqueue, task_work) here.
> +	 * RHASH values can also carry referenced kptrs (and per-cpu kptrs),
> +	 * whose destructors must not run from arbitrary BPF execution
> +	 * contexts (e.g. NMI); leave them attached to the recycled element
> +	 * and let rhtab_mem_dtor() destroy them once the element is
> +	 * eventually freed. This matches the hash map semantics introduced
> +	 * by a3a81d247651 ("bpf: Cancel special fields on map value
> +	 * recycle").
> +	 */
> +	bpf_map_free_internal_structs(&rhtab->map,
> +				      rhtab_elem_value(elem, rhtab->map.key_size));
> +}

This isn't a bug, but could the explanation live in one place with the call
sites pointing at it? The same rationale appears in five spots throughout
this change: here above rhtab_cancel_fields(), above rhtab_init_map_value(),
and in three more comments in the call sites below. That's roughly 33
comment lines against 25 lines of code, and any future adjustments to the
reasoning need to be synced across all five. For comparison, the equivalent
hash map helpers check_and_cancel_fields() and htab_elem_free() carry no
comment at all.

> +
> +/*
> + * Initialize special fields of a freshly allocated rhtab element, but keep
> + * kptr fields untouched. A recycled element may carry a referenced kptr from
> + * its previous life: the delete path only cancels NMI-safe fields (matching
> + * the hash map semantics), so the kptr reference stays owned by the element
> + * until rhtab_mem_dtor() destroys it. Zeroing it here (as
> + * check_and_init_map_value() would) would drop the reference without
> + * releasing it.
> + */
> +static void rhtab_init_map_value(struct bpf_map *map, void *value)
> +{
> +	struct btf_record *rec = map->record;
> +	int i;
> +
> +	if (IS_ERR_OR_NULL(rec))
> +		return;
> +
> +	for (i = 0; i < rec->cnt; i++) {
> +		struct btf_field *field = &rec->fields[i];
> +		void *field_ptr = value + field->offset;
> +
> +		switch (field->type) {
> +		case BPF_KPTR_UNREF:
> +		case BPF_KPTR_REF:
> +		case BPF_KPTR_PERCPU:
> +			continue;
> +		default:
> +			bpf_obj_init_field(field, field_ptr);
> +		}
> +	}
> +}

This isn't a bug, but could this kptr-filtering init be unified with
bpf_obj_init() rather than duplicated here? rhtab_init_map_value()
re-implements the field walk from bpf_obj_init() with a kptr filter in
front of bpf_obj_init_field(). The hash map path this change says it is
matching takes a different route: alloc_htab_elem() does no special-field
initialization at all on a recycled or fresh element, relying on __GFP_ZERO
for fresh memory and on bpf_obj_cancel_fields() having left the cancelled
fields reusable. Would it be simpler to drop the init call entirely the way
alloc_htab_elem() does, or to add the kptr skip as a flag or variant of
bpf_obj_init() so the field walk lives in one place?

[ ... ]

> @@ -3022,10 +3064,11 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * BPF_F_LOCK, matching arraymap semantics.
>  	 *
>  	 * copy_map_value() skips special-field offsets, so old timers/
> -	 * kptrs/etc. still sit in the slot. Cancel them after the copy
> -	 * to match arraymap's update semantics.
> +	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
> +	 * the copy to match arraymap's update semantics; referenced kptrs
> +	 * stay attached and are destroyed by rhtab_mem_dtor().
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	rhtab_cancel_fields(rhtab, elem);
>  	return 0;
>  }

A subsystem pattern flags this as potentially concerning: after this change,
rhtab_map_update_existing() overwrites an existing map value in place with
copy_map_value() but then only calls rhtab_cancel_fields(), which now calls
bpf_map_free_internal_structs() instead of bpf_obj_free_fields().
bpf_map_free_internal_structs() handles only BPF_TIMER, BPF_WORKQUEUE and
BPF_TASK_WORK. The BPF_KPTR_REF or BPF_KPTR_PERCPU reference held by the
value the user just overwrote is no longer released at update time; it stays
attached to the live element and is only destroyed when the element is
eventually freed to the slab.

Before the change:

rhtab_check_and_free_fields(rhtab, elem);   /* -> bpf_obj_free_fields() */

After:

rhtab_cancel_fields(rhtab, elem);           /* -> bpf_map_free_internal_structs() */

The change appears intentional and mirrors array_map_update_elem(), which
was itself converted from bpf_obj_free_fields() to bpf_obj_cancel_fields()
by a3a81d247651. The reference is not leaked, only its release is deferred:
the element keeps ownership, and rhtab_mem_dtor() runs bpf_obj_free_fields()
before returning memory to the allocator. The kptr slot holds at most one
reference and copy_map_value() never writes it, so repeated updates cannot
accumulate references. Is this deferred-release semantic for RHASH
intentional, matching the hash map and array behavior?

> @@ -3066,7 +3109,14 @@ static long rhtab_map_update_elem(struct bpf_map *map, void *key, void *value, u
>
>  	memcpy(elem->data, key, map->key_size);
>  	copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);
> -	check_and_init_map_value(map, rhtab_elem_value(elem, map->key_size));
> +	/*
> +	 * Initialize special fields of the (possibly recycled) element, but
> +	 * leave kptr slots alone: a recycled element may still own a
> +	 * referenced kptr that rhtab_mem_dtor() will release, so zeroing it
> +	 * here would leak the reference. Fresh memory from the bpf mem
> +	 * allocator is zeroed, so skipping the kptr init is safe there too.
> +	 */
> +	rhtab_init_map_value(map, rhtab_elem_value(elem, map->key_size));

A subsystem pattern flags this as potentially concerning: after this change,
rhtab_map_update_elem() copies the user value into a newly allocated element
with copy_map_value() and the paired check_and_init_map_value() call is
replaced by rhtab_init_map_value(), which explicitly skips BPF_KPTR_UNREF,
BPF_KPTR_REF and BPF_KPTR_PERCPU instead of calling bpf_obj_init_field().
copy_map_value() also skips special-field offsets via bpf_obj_memcpy(), so
no code writes the kptr slots of the new element. bpf_mem_cache_alloc() may
hand back an element recycled from a previous operation whose kptr slots
were left populated, and the element is then published into the rhashtable
under a new key while still carrying the previous occupant's kptr.

Before the change:

check_and_init_map_value(map, rhtab_elem_value(elem, map->key_size));

After:

rhtab_init_map_value(map, rhtab_elem_value(elem, map->key_size));

Skipping BPF_KPTR_REF and BPF_KPTR_PERCPU init appears required for the
accounting to balance: zeroing the slot with bpf_obj_init_field() would drop
the pointer without running the destructor. The equivalent reference path,
alloc_htab_elem() for non-prealloc hash maps, performs no special-field
initialization at all, so htab inherits recycled kptrs the same way. Fresh
memory from the allocator is verified to be zeroed via __GFP_ZERO. Is the
kptr inheritance across recycled elements under new keys the intended
behavior?


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

Re: [PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle

[Mykyta Yatsenko]

On 8/24/26 3:36 PM, chenyuan_fl@163.com wrote:
> From: Yuan Chen <chenyuan@kylinos.cn>
> 
> rhtab_delete_elem() and rhtab_map_update_existing() eagerly call
> bpf_obj_free_fields() when an element is deleted or its value is
> replaced. This runs kptr destructors in the caller's execution
> context, which is unsafe for BPF programs running in NMI context
> (e.g. perf_event programs attached to hardware PMU overflows):
> referenced kptr destructors may take locks or otherwise cannot run
> in NMI.
> 
> Commit a3a81d247651 ("bpf: Cancel special fields on map value
> recycle") switched the hash map and array recycle paths to
> bpf_obj_cancel_fields(), which only cancels NMI-safe fields (timer,
> workqueue, task_work), but it missed the resizable hashtab.
> rhtab_map_update_existing() even documents the intended "cancel"
> semantics while still calling bpf_obj_free_fields().
> 
> Fix the resizable hashtab the same way:
> 
>   * rhtab_delete_elem() and rhtab_map_update_existing() now cancel
>     only NMI-safe fields. Referenced kptrs stay attached to the
>     recycled element and are destroyed by rhtab_mem_dtor() once the
>     element is eventually freed, keeping the reference accounting
>     balanced.
> 
>   * rhtab_map_update_elem() initializes the special fields of a
>     freshly allocated element. The bpf memory allocator may return a
>     recycled element that still owns a referenced kptr, and
>     check_and_init_map_value() would zero that slot, dropping the
>     reference without releasing it. rhtab_init_map_value()
>     initializes the remaining fields (spin lock, timer, workqueue,
>     task_work, refcount) but leaves kptr slots untouched, matching
>     the hash map semantics.
> 
> Verified with a selftest: a perf_event (NMI) program overwrites a
> rhtab element that holds a referenced task kptr, and a second phase
> deletes and re-inserts the element to exercise the recycle path.
> Before the patch the NMI update eagerly released the kptr and the
> recycle path zeroed the inherited slot; after the patch the kptr is
> inherited on both paths and the probe observes it non-NULL.
> 
> Fixes: a3a81d247651 ("bpf: Cancel special fields on map value recycle")
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
> ---
>  kernel/bpf/hashtab.c | 70 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 60 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index d40cb5dd446c..0df8db27cd8c 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -2864,14 +2864,56 @@ static int rhtab_map_alloc_check(union bpf_attr *attr)
>  	return htab_map_alloc_check(attr);
>  }
>  
> -static void rhtab_check_and_free_fields(struct bpf_rhtab *rhtab,
> -					struct rhtab_elem *elem)
> +static void rhtab_cancel_fields(struct bpf_rhtab *rhtab,
> +				struct rhtab_elem *elem)
>  {
>  	if (IS_ERR_OR_NULL(rhtab->map.record))
>  		return;
>  
> -	bpf_obj_free_fields(rhtab->map.record,
> -			    rhtab_elem_value(elem, rhtab->map.key_size));
> +	/*
> +	 * Only cancel NMI-safe fields (timer, workqueue, task_work) here.
> +	 * RHASH values can also carry referenced kptrs (and per-cpu kptrs),
> +	 * whose destructors must not run from arbitrary BPF execution
> +	 * contexts (e.g. NMI); leave them attached to the recycled element
> +	 * and let rhtab_mem_dtor() destroy them once the element is
> +	 * eventually freed. This matches the hash map semantics introduced
> +	 * by a3a81d247651 ("bpf: Cancel special fields on map value
> +	 * recycle").
> +	 */
> +	bpf_map_free_internal_structs(&rhtab->map,
> +				      rhtab_elem_value(elem, rhtab->map.key_size));
> +}
> +
> +/*
> + * Initialize special fields of a freshly allocated rhtab element, but keep
> + * kptr fields untouched. A recycled element may carry a referenced kptr from
> + * its previous life: the delete path only cancels NMI-safe fields (matching
> + * the hash map semantics), so the kptr reference stays owned by the element
> + * until rhtab_mem_dtor() destroys it. Zeroing it here (as
> + * check_and_init_map_value() would) would drop the reference without
> + * releasing it.
> + */
> +static void rhtab_init_map_value(struct bpf_map *map, void *value)

Could you please double check if this is needed at all?
I think bpf_map_free_internal_structs() going to reset
special fields to 0, so immediate reuse by __bpf_async_init(),
bpf_task_work_schedule() correctly identifies fresh fields.

> +{
> +	struct btf_record *rec = map->record;
> +	int i;
> +
> +	if (IS_ERR_OR_NULL(rec))
> +		return;
> +
> +	for (i = 0; i < rec->cnt; i++) {
> +		struct btf_field *field = &rec->fields[i];
> +		void *field_ptr = value + field->offset;
> +
> +		switch (field->type) {
> +		case BPF_KPTR_UNREF:
> +		case BPF_KPTR_REF:
> +		case BPF_KPTR_PERCPU:
> +			continue;
> +		default:
> +			bpf_obj_init_field(field, field_ptr);
> +		}
> +	}
>  }
>  
>  static void rhtab_mem_dtor(void *obj, void *ctx)
> @@ -2963,8 +3005,8 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
>  		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
>  		check_and_init_map_value(&rhtab->map, copy);
>  	}
> -	/* Release internal structs: kptr, bpf_timer, task_work, wq */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	/* Cancel NMI-safe fields; full destruction happens in rhtab_mem_dtor */
> +	rhtab_cancel_fields(rhtab, elem);

Let's directly call bpf_obj_cancel_fields() here and below, 
so it is consistent with htab.

>  	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
>  	return 0;
>  }
> @@ -3022,10 +3064,11 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * BPF_F_LOCK, matching arraymap semantics.
>  	 *
>  	 * copy_map_value() skips special-field offsets, so old timers/
> -	 * kptrs/etc. still sit in the slot. Cancel them after the copy
> -	 * to match arraymap's update semantics.
> +	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
> +	 * the copy to match arraymap's update semantics; referenced kptrs
> +	 * stay attached and are destroyed by rhtab_mem_dtor().
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	rhtab_cancel_fields(rhtab, elem);
>  	return 0;
>  }
>  
> @@ -3066,7 +3109,14 @@ static long rhtab_map_update_elem(struct bpf_map *map, void *key, void *value, u
>  
>  	memcpy(elem->data, key, map->key_size);
>  	copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);
> -	check_and_init_map_value(map, rhtab_elem_value(elem, map->key_size));
> +	/*
> +	 * Initialize special fields of the (possibly recycled) element, but
> +	 * leave kptr slots alone: a recycled element may still own a
> +	 * referenced kptr that rhtab_mem_dtor() will release, so zeroing it
> +	 * here would leak the reference. Fresh memory from the bpf mem
> +	 * allocator is zeroed, so skipping the kptr init is safe there too.
> +	 */
> +	rhtab_init_map_value(map, rhtab_elem_value(elem, map->key_size));
>  
>  	/* Prevent deadlock for NMI programs attempting to take bucket lock */
>  	bpf_disable_instrumentation();

[PATCH bpf-next v3 0/4] bpf: Cancel special fields in resizable hashtab on recycle

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

v3 addresses Mykyta's review of v2 [3]. Both comments concern patch 1:

  1. "Could you please double check if this is needed at all? I think
     bpf_map_free_internal_structs() going to reset special fields to 0,
     so immediate reuse by __bpf_async_init(), bpf_task_work_schedule()
     correctly identifies fresh fields."
     -> Correct: bpf_obj_cancel_fields() resets the timer/workqueue/
     task_work slots in place (xchg to NULL in bpf_async_cancel_and_free()
     and bpf_task_work_cancel_and_free()), so no re-initialization is
     needed on reuse. rhtab_init_map_value() is dropped entirely; the
     element alloc path now matches htab's non-prealloc path
     (alloc_htab_elem()), which also performs no explicit init.

  2. "Let's directly call bpf_obj_cancel_fields() here and below, so it
     is consistent with htab."
     -> Done: the rhtab_cancel_fields() wrapper is removed and
     rhtab_delete_elem()/rhtab_map_update_existing() call
     bpf_obj_cancel_fields() directly.

The resizable hashtab still eagerly calls bpf_obj_free_fields() on
element delete/replace, which runs kptr destructors from the caller's
execution context (unsafe in NMI). This follows the existing discussion
on RHash special-field recycling (Nuoqi Gui's series [2] and the review
[1]): patch 1 applies the cancel semantics like hash/array maps
(a3a81d247651); patch 2 fixes a program-BTF use-after-free in the
mem-alloc destructor found while testing; patches 3-4 add regression
tests (NMI update, and per-field delete/re-insert cycles).

Changes since v2 (all from Mykyta's review [3]):

  * Drop rhtab_init_map_value(): bpf_obj_cancel_fields() resets the
    timer/workqueue/task_work slots to zero on delete, fresh elements
    come zeroed from the bpf mem allocator, and kptr slots must stay
    untouched, so no re-initialization is needed (matching htab's
    non-prealloc path).
  * Call bpf_obj_cancel_fields() directly instead of a rhtab-specific
    wrapper, consistent with htab.

[1] https://lore.kernel.org/bpf/DKEVG8ZVJDDQ.2G40FTOIZKU57@gmail.com/
[2] https://lore.kernel.org/bpf/20260726-f01-23-rhash-cancel-bpf-next-v1-0-6e5e1131d885@mails.tsinghua.edu.cn/
[3] https://lore.kernel.org/bpf/0560a24d-2cf9-4e5b-aa61-580af1e56de1@gmail.com/

Yuan Chen (4):
  bpf: Cancel special fields in resizable hashtab on recycle
  bpf: Fix use-after-free of program BTF in mem-alloc destructor
  selftests/bpf: Test rhtab kptr recycle from NMI context
  selftests/bpf: Test rhtab special-field combinations

 kernel/bpf/hashtab.c                               |  98 +++++-
 .../selftests/bpf/prog_tests/rhtab_fields.c        | 337 ++++++++++++++++++
 .../testing/selftests/bpf/prog_tests/rhtab_kptr.c  | 184 ++++++++++
 tools/testing/selftests/bpf/progs/rhtab_fields.c   | 378 +++++++++++++++++++++
 tools/testing/selftests/bpf/progs/rhtab_kptr.c     | 146 ++++++++
 tools/testing/selftests/bpf/rhtab_fields_common.h  |  19 ++
 tools/testing/selftests/bpf/rhtab_kptr_common.h    |   6 +
 7 files changed, 1152 insertions(+), 16 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_fields.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_kptr.c
 create mode 100644 tools/testing/selftests/bpf/rhtab_fields_common.h
 create mode 100644 tools/testing/selftests/bpf/rhtab_kptr_common.h

--
2.54.0

[PATCH bpf-next v3 1/4] bpf: Cancel special fields in resizable hashtab on recycle

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

rhtab_delete_elem() and rhtab_map_update_existing() eagerly call
bpf_obj_free_fields() when an element is deleted or its value is
replaced. This runs kptr destructors in the caller's execution
context, which is unsafe for BPF programs running in NMI context
(e.g. perf_event programs attached to hardware PMU overflows):
referenced kptr destructors may take locks or otherwise cannot run
in NMI.

Commit a3a81d247651 ("bpf: Cancel special fields on map value
recycle") switched the hash map and array recycle paths to
bpf_obj_cancel_fields(), which only cancels NMI-safe fields (timer,
workqueue, task_work), but it missed the resizable hashtab.
rhtab_map_update_existing() even documents the intended "cancel"
semantics while still calling bpf_obj_free_fields().

Fix the resizable hashtab the same way: call bpf_obj_cancel_fields()
on delete and in-place update, matching htab. Referenced kptrs stay
attached to the recycled element and are destroyed by rhtab_mem_dtor()
once the element is eventually freed, keeping the reference accounting
balanced. No special-field initialization is added to the element
alloc path: fresh elements come zeroed from the bpf mem allocator,
recycled elements already had their timer/workqueue/task_work slots
reset by bpf_obj_cancel_fields(), and check_and_init_map_value() would
zero the kptr slot of a recycled element, dropping the reference
without releasing it.

Verified with a selftest: a perf_event (NMI) program overwrites a
rhtab element that holds a referenced task kptr, and a second phase
deletes and re-inserts the element to exercise the recycle path.
Before the patch the NMI update eagerly released the kptr and the
recycle path zeroed the inherited slot; after the patch the kptr is
inherited on both paths and the probe observes it non-NULL.

Fixes: a3a81d247651 ("bpf: Cancel special fields on map value recycle")
Suggested-by: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/hashtab.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index d40cb5dd446c..aaedda3730f3 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -2864,16 +2864,6 @@ static int rhtab_map_alloc_check(union bpf_attr *attr)
 	return htab_map_alloc_check(attr);
 }
 
-static void rhtab_check_and_free_fields(struct bpf_rhtab *rhtab,
-					struct rhtab_elem *elem)
-{
-	if (IS_ERR_OR_NULL(rhtab->map.record))
-		return;
-
-	bpf_obj_free_fields(rhtab->map.record,
-			    rhtab_elem_value(elem, rhtab->map.key_size));
-}
-
 static void rhtab_mem_dtor(void *obj, void *ctx)
 {
 	struct htab_btf_record *hrec = ctx;
@@ -2963,8 +2953,9 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
 		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
 		check_and_init_map_value(&rhtab->map, copy);
 	}
-	/* Release internal structs: kptr, bpf_timer, task_work, wq */
-	rhtab_check_and_free_fields(rhtab, elem);
+	/* Cancel NMI-safe fields; full destruction happens in rhtab_mem_dtor */
+	bpf_obj_cancel_fields(&rhtab->map,
+			      rhtab_elem_value(elem, rhtab->map.key_size));
 	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
 	return 0;
 }
@@ -3022,10 +3013,12 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
 	 * BPF_F_LOCK, matching arraymap semantics.
 	 *
 	 * copy_map_value() skips special-field offsets, so old timers/
-	 * kptrs/etc. still sit in the slot. Cancel them after the copy
-	 * to match arraymap's update semantics.
+	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
+	 * the copy to match arraymap's update semantics; referenced kptrs
+	 * stay attached and are destroyed by rhtab_mem_dtor().
 	 */
-	rhtab_check_and_free_fields(rhtab, elem);
+	bpf_obj_cancel_fields(&rhtab->map,
+			      rhtab_elem_value(elem, rhtab->map.key_size));
 	return 0;
 }
 
@@ -3066,7 +3059,14 @@ static long rhtab_map_update_elem(struct bpf_map *map, void *key, void *value, u
 
 	memcpy(elem->data, key, map->key_size);
 	copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);
-	check_and_init_map_value(map, rhtab_elem_value(elem, map->key_size));
+	/*
+	 * No explicit special-field initialization, matching the hash map's
+	 * non-prealloc path: fresh elements come zeroed from the bpf mem
+	 * allocator, and recycled elements had their timer/workqueue/task_work
+	 * slots reset by bpf_obj_cancel_fields() on delete. kptr slots are
+	 * left untouched so a recycled element keeps owning its reference
+	 * until rhtab_mem_dtor() releases it.
+	 */
 
 	/* Prevent deadlock for NMI programs attempting to take bucket lock */
 	bpf_disable_instrumentation();
-- 
2.54.0

Re: [PATCH bpf-next v3 1/4] bpf: Cancel special fields in resizable hashtab on recycle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing bpf_obj_free_fields() when overwriting a map value in rhtab_map_update_existing(), violating the BPF-001 guideline.
--

commit a386d962d0ebafb2260b7ce5bf02d3879316b1e8
Author: Yuan Chen <chenyuan@kylinos.cn>
bpf: Cancel special fields in resizable hashtab on recycle

This patch updates the resizable hashtab to use bpf_obj_cancel_fields()
instead of eagerly calling bpf_obj_free_fields() on recycle and update
paths. This is intended to prevent running kptr destructors in NMI context.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index d40cb5dd446ca..aaedda3730f37 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c

[ ... ]

> @@ -3022,10 +3013,12 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * BPF_F_LOCK, matching arraymap semantics.
>  	 *
>  	 * copy_map_value() skips special-field offsets, so old timers/
> -	 * kptrs/etc. still sit in the slot. Cancel them after the copy
> -	 * to match arraymap's update semantics.
> +	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
> +	 * the copy to match arraymap's update semantics; referenced kptrs
> +	 * stay attached and are destroyed by rhtab_mem_dtor().
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	bpf_obj_cancel_fields(&rhtab->map,
> +			      rhtab_elem_value(elem, rhtab->map.key_size));

[Severity: High]
Does replacing this cleanup cause a resource leak during map updates?

According to the BPF subsystem guidelines, when copy_map_value() overwrites
an existing map value, bpf_obj_free_fields() must be called to release
resources held by the old value. 

Since the patch removes the underlying bpf_obj_free_fields() call in
rhtab_map_update_existing() and substitutes it with bpf_obj_cancel_fields(),
it appears kptr references might leak when a map value is updated in place.

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

Re: [PATCH bpf-next v3 1/4] bpf: Cancel special fields in resizable hashtab on recycle

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index d40cb5dd446ca..aaedda3730f37 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c

[ ... ]

> @@ -3022,10 +3013,12 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * BPF_F_LOCK, matching arraymap semantics.
>  	 *
>  	 * copy_map_value() skips special-field offsets, so old timers/
> -	 * kptrs/etc. still sit in the slot. Cancel them after the copy
> -	 * to match arraymap's update semantics.
> +	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
> +	 * the copy to match arraymap's update semantics; referenced kptrs
> +	 * stay attached and are destroyed by rhtab_mem_dtor().
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	bpf_obj_cancel_fields(&rhtab->map,
> +			      rhtab_elem_value(elem, rhtab->map.key_size));

This isn't a bug, but could the second argument use old_val here? It
already holds rhtab_elem_value(elem, map->key_size) from the top of the
function and is used by copy_map_value_locked() two lines above.

> @@ -3066,7 +3059,14 @@ static long rhtab_map_update_elem(struct bpf_map *map, void *key, void *value, u
>
>  	memcpy(elem->data, key, map->key_size);
>  	copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);
> -	check_and_init_map_value(map, rhtab_elem_value(elem, map->key_size));
> +	/*
> +	 * No explicit special-field initialization, matching the hash map's
> +	 * non-prealloc path: fresh elements come zeroed from the bpf mem
> +	 * allocator, and recycled elements had their timer/workqueue/task_work
> +	 * slots reset by bpf_obj_cancel_fields() on delete. kptr slots are
> +	 * left untouched so a recycled element keeps owning its reference
> +	 * until rhtab_mem_dtor() releases it.
> +	 */

A subsystem pattern flags this as potentially concerning: the
copy_map_value() call on a field-capable map type is no longer followed
by check_and_init_map_value().

Looking at the non-prealloc hash map path in alloc_htab_elem(), it
appears this pattern has never called check_and_init_map_value() for
fresh elements, and the bpf mem allocator does return zeroed memory. For
recycled elements, the kptr slot staying intact is the intended behavior
since rhtab_mem_dtor() handles final cleanup.

Could you confirm this removal is safe for all special field types that
BPF_MAP_TYPE_RHASH supports?

This isn't a bug, but does the seven-line comment block add clarity over
the briefer explanations already present in rhtab_delete_elem() and
rhtab_map_update_existing()? The same rationale appears three times
within about 110 lines.


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

Re: [PATCH bpf-next v3 1/4] bpf: Cancel special fields in resizable hashtab on recycle

[Mykyta Yatsenko]

On 9/1/26 7:28 AM, chenyuan_fl@163.com wrote:
> From: Yuan Chen <chenyuan@kylinos.cn>
> 
> rhtab_delete_elem() and rhtab_map_update_existing() eagerly call
> bpf_obj_free_fields() when an element is deleted or its value is
> replaced. This runs kptr destructors in the caller's execution
> context, which is unsafe for BPF programs running in NMI context
> (e.g. perf_event programs attached to hardware PMU overflows):
> referenced kptr destructors may take locks or otherwise cannot run
> in NMI.
> 
> Commit a3a81d247651 ("bpf: Cancel special fields on map value
> recycle") switched the hash map and array recycle paths to
> bpf_obj_cancel_fields(), which only cancels NMI-safe fields (timer,
> workqueue, task_work), but it missed the resizable hashtab.
> rhtab_map_update_existing() even documents the intended "cancel"
> semantics while still calling bpf_obj_free_fields().
> 
> Fix the resizable hashtab the same way: call bpf_obj_cancel_fields()
> on delete and in-place update, matching htab. Referenced kptrs stay
> attached to the recycled element and are destroyed by rhtab_mem_dtor()
> once the element is eventually freed, keeping the reference accounting
> balanced. No special-field initialization is added to the element
> alloc path: fresh elements come zeroed from the bpf mem allocator,
> recycled elements already had their timer/workqueue/task_work slots
> reset by bpf_obj_cancel_fields(), and check_and_init_map_value() would
> zero the kptr slot of a recycled element, dropping the reference
> without releasing it.
> 
> Verified with a selftest: a perf_event (NMI) program overwrites a
> rhtab element that holds a referenced task kptr, and a second phase
> deletes and re-inserts the element to exercise the recycle path.
> Before the patch the NMI update eagerly released the kptr and the
> recycle path zeroed the inherited slot; after the patch the kptr is
> inherited on both paths and the probe observes it non-NULL.
> 
> Fixes: a3a81d247651 ("bpf: Cancel special fields on map value recycle")
> Suggested-by: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
> ---
>  kernel/bpf/hashtab.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index d40cb5dd446c..aaedda3730f3 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -2864,16 +2864,6 @@ static int rhtab_map_alloc_check(union bpf_attr *attr)
>  	return htab_map_alloc_check(attr);
>  }
>  
> -static void rhtab_check_and_free_fields(struct bpf_rhtab *rhtab,
> -					struct rhtab_elem *elem)
> -{
> -	if (IS_ERR_OR_NULL(rhtab->map.record))
> -		return;
> -
> -	bpf_obj_free_fields(rhtab->map.record,
> -			    rhtab_elem_value(elem, rhtab->map.key_size));
> -}
> -
>  static void rhtab_mem_dtor(void *obj, void *ctx)
>  {
>  	struct htab_btf_record *hrec = ctx;
> @@ -2963,8 +2953,9 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
>  		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
>  		check_and_init_map_value(&rhtab->map, copy);
>  	}
> -	/* Release internal structs: kptr, bpf_timer, task_work, wq */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	/* Cancel NMI-safe fields; full destruction happens in rhtab_mem_dtor */
> +	bpf_obj_cancel_fields(&rhtab->map,
> +			      rhtab_elem_value(elem, rhtab->map.key_size));
>  	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
>  	return 0;
>  }
> @@ -3022,10 +3013,12 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * BPF_F_LOCK, matching arraymap semantics.
>  	 *
>  	 * copy_map_value() skips special-field offsets, so old timers/
> -	 * kptrs/etc. still sit in the slot. Cancel them after the copy
> -	 * to match arraymap's update semantics.
> +	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
> +	 * the copy to match arraymap's update semantics; referenced kptrs
> +	 * stay attached and are destroyed by rhtab_mem_dtor().
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	bpf_obj_cancel_fields(&rhtab->map,
> +			      rhtab_elem_value(elem, rhtab->map.key_size));
>  	return 0;
>  }
>  
> @@ -3066,7 +3059,14 @@ static long rhtab_map_update_elem(struct bpf_map *map, void *key, void *value, u
>  
>  	memcpy(elem->data, key, map->key_size);
>  	copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);
> -	check_and_init_map_value(map, rhtab_elem_value(elem, map->key_size));
> +	/*
> +	 * No explicit special-field initialization, matching the hash map's
> +	 * non-prealloc path: fresh elements come zeroed from the bpf mem
> +	 * allocator, and recycled elements had their timer/workqueue/task_work
> +	 * slots reset by bpf_obj_cancel_fields() on delete. kptr slots are
> +	 * left untouched so a recycled element keeps owning its reference
> +	 * until rhtab_mem_dtor() releases it.
> +	 */

I'm not sure this comment is useful, we don't comment on why we are not zeroing
special fields in htab, so why here.
Please address the finding of the bot regarding the old_val variable and for the
next respin send the patch series independently, not as a response to an old thread.
>  
>  	/* Prevent deadlock for NMI programs attempting to take bucket lock */
>  	bpf_disable_instrumentation();

[PATCH bpf-next v3 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

bpf_ma_set_dtor() duplicates the map's btf_record for the bpf_mem_alloc
destructor. btf_record_dup() only borrows the BTF references held by
the fields: kptrs and list_head/rb_root fields point at the program
BTF, whose lifetime is independent of the map. The duplicated record is
used from the deferred mem-alloc destructor workqueue, which can run
after the program BTF is gone (bpf_map_free() drops the map's
reference, and the RCU callback may run first). Reading the borrowed
descriptors there is a use-after-free, detected by KASAN as
"slab-use-after-free in btf_is_kernel".

Keep a reference on the borrowed program BTFs for the lifetime of the
duplicated record. The record is freed from a preemptible worker, so
the last btf_put() (which only schedules RCU destruction) does not make
the field descriptors safe to read; snapshot the borrowed BTFs, free
the record, then drop the references.

The rhtab kptr selftests exercise this path on every map teardown and
triggered the bug under KASAN; with this fix they pass cleanly.

Fixes: 1df97a7453ee ("bpf: Register dtor for freeing special fields")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/hashtab.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index aaedda3730f3..30bcc573772e 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -493,11 +493,76 @@ static void htab_pcpu_mem_dtor(void *obj, void *ctx)
 		bpf_obj_free_fields(hrec->record, per_cpu_ptr(pptr, cpu));
 }
 
+/*
+ * The duplicated record borrows program BTFs, but is freed from a deferred
+ * workqueue that may run after the program BTF is gone. Hold a reference for
+ * the record's lifetime and snapshot the borrowed BTFs, since
+ * btf_record_free() frees the record.
+ */
+
+/* Program BTF borrowed by the field, or NULL. */
+static struct btf *htab_field_borrowed_btf(const struct btf_field *field)
+{
+	struct btf *btf = NULL;
+
+	switch (field->type) {
+	case BPF_KPTR_UNREF:
+	case BPF_KPTR_REF:
+	case BPF_KPTR_PERCPU:
+	case BPF_UPTR:
+		btf = field->kptr.btf;
+		break;
+	case BPF_LIST_HEAD:
+	case BPF_RB_ROOT:
+		btf = field->graph_root.btf;
+		break;
+	default:
+		break;
+	}
+
+	if (btf && !btf_is_kernel(btf))
+		return btf;
+	return NULL;
+}
+
+/* Snapshot the program BTFs @rec borrows into @btfs; returns their count. */
+static int htab_record_prog_btfs_snapshot(struct btf_record *rec,
+					  struct btf **btfs)
+{
+	int i, n = 0;
+
+	if (IS_ERR_OR_NULL(rec))
+		return 0;
+
+	for (i = 0; i < rec->cnt; i++) {
+		struct btf *btf = htab_field_borrowed_btf(&rec->fields[i]);
+
+		if (btf)
+			btfs[n++] = btf;
+	}
+	return n;
+}
+
+static void htab_record_prog_btf_get(struct btf_record *rec)
+{
+	struct btf *btfs[BTF_FIELDS_MAX];
+	int i, n;
+
+	n = htab_record_prog_btfs_snapshot(rec, btfs);
+	for (i = 0; i < n; i++)
+		btf_get(btfs[i]);
+}
+
 static void htab_dtor_ctx_free(void *ctx)
 {
 	struct htab_btf_record *hrec = ctx;
+	struct btf *btfs[BTF_FIELDS_MAX];
+	int i, n;
 
+	n = htab_record_prog_btfs_snapshot(hrec->record, btfs);
 	btf_record_free(hrec->record);
+	for (i = 0; i < n; i++)
+		btf_put(btfs[i]);
 	kfree(ctx);
 }
 
@@ -521,6 +586,7 @@ static int bpf_ma_set_dtor(struct bpf_map *map, struct bpf_mem_alloc *ma,
 		kfree(hrec);
 		return err;
 	}
+	htab_record_prog_btf_get(hrec->record);
 	bpf_mem_alloc_set_dtor(ma, dtor, htab_dtor_ctx_free, hrec);
 	return 0;
 }
-- 
2.54.0

Re: [PATCH bpf-next v3 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor

[Mykyta Yatsenko]

On 9/1/26 7:28 AM, chenyuan_fl@163.com wrote:
> From: Yuan Chen <chenyuan@kylinos.cn>
> 
> bpf_ma_set_dtor() duplicates the map's btf_record for the bpf_mem_alloc
> destructor. btf_record_dup() only borrows the BTF references held by
> the fields: kptrs and list_head/rb_root fields point at the program
> BTF, whose lifetime is independent of the map. The duplicated record is
> used from the deferred mem-alloc destructor workqueue, which can run
> after the program BTF is gone (bpf_map_free() drops the map's
> reference, and the RCU callback may run first). Reading the borrowed
> descriptors there is a use-after-free, detected by KASAN as
> "slab-use-after-free in btf_is_kernel".
> 
> Keep a reference on the borrowed program BTFs for the lifetime of the
> duplicated record. The record is freed from a preemptible worker, so
> the last btf_put() (which only schedules RCU destruction) does not make
> the field descriptors safe to read; snapshot the borrowed BTFs, free
> the record, then drop the references.
> 
> The rhtab kptr selftests exercise this path on every map teardown and
> triggered the bug under KASAN; with this fix they pass cleanly.

Does htab trigger the same kasan?

> 
> Fixes: 1df97a7453ee ("bpf: Register dtor for freeing special fields")
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
> ---
>  kernel/bpf/hashtab.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index aaedda3730f3..30bcc573772e 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -493,11 +493,76 @@ static void htab_pcpu_mem_dtor(void *obj, void *ctx)
>  		bpf_obj_free_fields(hrec->record, per_cpu_ptr(pptr, cpu));
>  }
>  
> +/*
> + * The duplicated record borrows program BTFs, but is freed from a deferred
> + * workqueue that may run after the program BTF is gone. Hold a reference for
> + * the record's lifetime and snapshot the borrowed BTFs, since
> + * btf_record_free() frees the record.
> + */
> +
> +/* Program BTF borrowed by the field, or NULL. */
> +static struct btf *htab_field_borrowed_btf(const struct btf_field *field)
> +{
> +	struct btf *btf = NULL;
> +
> +	switch (field->type) {
> +	case BPF_KPTR_UNREF:
> +	case BPF_KPTR_REF:
> +	case BPF_KPTR_PERCPU:
> +	case BPF_UPTR:
> +		btf = field->kptr.btf;
> +		break;
> +	case BPF_LIST_HEAD:
> +	case BPF_RB_ROOT:
> +		btf = field->graph_root.btf;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (btf && !btf_is_kernel(btf))
> +		return btf;
> +	return NULL;
> +}
> +
> +/* Snapshot the program BTFs @rec borrows into @btfs; returns their count. */
> +static int htab_record_prog_btfs_snapshot(struct btf_record *rec,
> +					  struct btf **btfs)
> +{
> +	int i, n = 0;
> +
> +	if (IS_ERR_OR_NULL(rec))
> +		return 0;
> +
> +	for (i = 0; i < rec->cnt; i++) {
> +		struct btf *btf = htab_field_borrowed_btf(&rec->fields[i]);
> +
> +		if (btf)
> +			btfs[n++] = btf;
> +	}
> +	return n;
> +}
> +
> +static void htab_record_prog_btf_get(struct btf_record *rec)
> +{
> +	struct btf *btfs[BTF_FIELDS_MAX];

it looks like temporary array and htab_record_prog_btfs_snapshot()
are unnecessary:

for (i = 0; i < rec->cnt; i++) {
        struct btf *btf = htab_field_borrowed_btf(&rec->fields[i]);

        if (btf)
	        btf_get(btf);
}

> +	int i, n;
> +
> +	n = htab_record_prog_btfs_snapshot(rec, btfs);
> +	for (i = 0; i < n; i++)
> +		btf_get(btfs[i]);
> +}
> +
>  static void htab_dtor_ctx_free(void *ctx)
>  {
>  	struct htab_btf_record *hrec = ctx;
> +	struct btf *btfs[BTF_FIELDS_MAX];
> +	int i, n;
>  
> +	n = htab_record_prog_btfs_snapshot(hrec->record, btfs);
>  	btf_record_free(hrec->record);
> +	for (i = 0; i < n; i++)
> +		btf_put(btfs[i]);
>  	kfree(ctx);
>  }
>  
> @@ -521,6 +586,7 @@ static int bpf_ma_set_dtor(struct bpf_map *map, struct bpf_mem_alloc *ma,
>  		kfree(hrec);
>  		return err;
>  	}
> +	htab_record_prog_btf_get(hrec->record);
>  	bpf_mem_alloc_set_dtor(ma, dtor, htab_dtor_ctx_free, hrec);
>  	return 0;
>  }

[PATCH bpf-next v3 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

A perf_event program running in NMI context overwrites a rhtab
element whose value holds a referenced task kptr. The old kptr must
stay attached to the element (cancel semantics, matching hash maps);
before the rhtab recycle fix the NMI update eagerly released it and
the probe observed NULL. The test asserts the NMI program actually
ran, so the probe result is meaningful.

A second phase deletes and re-inserts the element 2000 times. The
re-insertion may recycle the freed element, which still owns the
kptr; before the fix the alloc path zeroed the inherited slot via
check_and_init_map_value(), leaking the reference, and the probe
never observed a non-NULL pointer. The test requires at least one
recycle to inherit the kptr, and also verifies that plain
(non-special) value bytes still round-trip through the recycled
element on every iteration.

The NMI phase is skipped when no hardware PMU is available.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 .../selftests/bpf/prog_tests/rhtab_kptr.c     | 184 ++++++++++++++++++
 .../testing/selftests/bpf/progs/rhtab_kptr.c  | 146 ++++++++++++++
 .../testing/selftests/bpf/rhtab_kptr_common.h |   6 +
 3 files changed, 336 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_kptr.c
 create mode 100644 tools/testing/selftests/bpf/rhtab_kptr_common.h

diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
new file mode 100644
index 000000000000..4bdcc9ce5500
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+#include <test_progs.h>
+#include <linux/perf_event.h>
+#include <stddef.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+#include "rhtab_kptr.skel.h"
+
+/* Userspace mirror of the BPF-side struct val_t (progs/rhtab_kptr.c). The
+ * update syscall copies map->value_size bytes from the buffer, so it must
+ * be at least that large; special fields are skipped by the value copy but
+ * the kernel still reads the full value_size from userspace.
+ */
+struct val_t_user {
+	__u64 tsk;
+	__u32 magic;
+	__u32 pad;
+};
+
+_Static_assert(sizeof(struct val_t_user) == 16, "val_t layout drift");
+_Static_assert(offsetof(struct val_t_user, magic) == 8, "val_t magic offset drift");
+
+/* Zeroed value for creating/recreating elements; BSS is zero-filled. */
+static struct val_t_user zero;
+
+/* Cached CPU count and scratch buffer for percpu counter summation. */
+static __u64 *cpu_vals;
+static int ncpu = -1;
+
+static __u64 read_counter(struct rhtab_kptr *skel, u32 idx)
+{
+	__u64 sum = 0;
+	int i, err;
+
+	if (!cpu_vals)
+		return 0;
+	err = bpf_map_lookup_elem(bpf_map__fd(skel->maps.counters), &idx,
+				  cpu_vals);
+	if (!ASSERT_OK(err, "lookup_counter"))
+		return 0;
+	for (i = 0; i < ncpu; i++)
+		sum += cpu_vals[i];
+	return sum;
+}
+
+/* Run @name via BPF_PROG_TEST_RUN, asserting both the syscall status and
+ * that the program exited 0. Returns 0 on success.
+ */
+static int run_prog_ok(struct rhtab_kptr *skel, const char *name)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	struct bpf_program *prog;
+	int err;
+
+	prog = bpf_object__find_program_by_name(skel->obj, name);
+	if (!ASSERT_OK_PTR(prog, name))
+		return -1;
+	err = bpf_prog_test_run_opts(bpf_program__fd(prog), &topts);
+	if (!ASSERT_OK(err, name))
+		return -1;
+	if (!ASSERT_EQ(topts.retval, 0, name))
+		return -1;
+	return 0;
+}
+
+void test_rhtab_kptr(void)
+{
+	struct perf_event_attr attr = {
+		.type = PERF_TYPE_HARDWARE,
+		.config = PERF_COUNT_HW_CPU_CYCLES,
+		.freq = 1,
+		.sample_freq = read_perf_max_sample_freq(),
+		.size = sizeof(struct perf_event_attr),
+	};
+	struct rhtab_kptr *skel;
+	__u64 init_before, nonnull_before;
+	__u32 key = 0;
+	int pmu_fd, i, retries = 0;
+
+	ncpu = libbpf_num_possible_cpus();
+	if (!ASSERT_GT(ncpu, 0, "num_possible_cpus"))
+		return;
+	cpu_vals = calloc(ncpu, sizeof(*cpu_vals));
+	if (!ASSERT_OK_PTR(cpu_vals, "calloc_cpu_vals"))
+		return;
+
+	skel = rhtab_kptr__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		goto out_free;
+
+	/* Create the element and stash a referenced task kptr in it. */
+	if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.rhtab),
+					   &key, &zero, BPF_ANY), "create_elem"))
+		goto out;
+	if (run_prog_ok(skel, "init_elem") != 0)
+		goto out;
+
+	pmu_fd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, 0);
+	if (pmu_fd >= 0) {
+		skel->links.nmi_update = bpf_program__attach_perf_event(skel->progs.nmi_update,
+									pmu_fd);
+		if (!ASSERT_OK_PTR(skel->links.nmi_update, "attach_perf_event")) {
+			close(pmu_fd);
+			goto out;
+		}
+
+		/* Let the NMI handler overwrite the element, and make sure it
+		 * actually ran before probing (otherwise the probe would pass
+		 * vacuously even on an unfixed kernel).
+		 */
+		for (i = 0; i < 20 && read_counter(skel, 1) == 0; i++)
+			usleep(100000);
+		ASSERT_GT(read_counter(skel, 1), 0, "nmi_update_ran");
+
+		bpf_link__destroy(skel->links.nmi_update);
+		skel->links.nmi_update = NULL;
+		close(pmu_fd);
+
+		/*
+		 * The old kptr must still be attached to the element: the
+		 * NMI update path only cancels NMI-safe fields, mirroring
+		 * hash map semantics. Before the fix the kptr was released
+		 * from the NMI context and the probe below would see NULL.
+		 */
+		if (run_prog_ok(skel, "probe_elem") != 0)
+			goto out;
+
+		ASSERT_EQ(read_counter(skel, 2), 1, "xchg_non_null");
+		ASSERT_EQ(read_counter(skel, 3), 0, "xchg_null");
+	} else {
+		test__skip();
+	}
+
+	/*
+	 * Now exercise the delete/re-insert recycle path. The delete only
+	 * cancels NMI-safe fields, so the freed element still owns the kptr.
+	 * If the re-insertion recycles that element, the kptr must be
+	 * inherited; zeroing it (as check_and_init_map_value() did before
+	 * the fix) leaks the reference and probe_elem() observes NULL.
+	 * Fresh memory handed out by the allocator is zeroed, so NULL probes
+	 * are expected too; only require that the inherited kptr survives at
+	 * least one recycle. Every iteration runs exactly one probe, so the
+	 * counters must add up to the loop count.
+	 */
+	init_before = read_counter(skel, 0);
+	nonnull_before = read_counter(skel, 2);
+	for (i = 0; i < 2000; i++) {
+		if (run_prog_ok(skel, "init_elem") != 0) {
+			/* init_elem fails only if the element is missing,
+			 * which must not happen in this single-threaded
+			 * loop; count it so a rhtab bug cannot be absorbed
+			 * silently.
+			 */
+			retries++;
+			if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.rhtab),
+							   &key, &zero, BPF_ANY),
+				       "recreate_elem"))
+				goto out;
+			if (run_prog_ok(skel, "init_elem") != 0)
+				goto out;
+		}
+		if (run_prog_ok(skel, "del_elem") != 0 ||
+		    run_prog_ok(skel, "upd_elem") != 0 ||
+		    run_prog_ok(skel, "probe_elem") != 0)
+			goto out;
+	}
+
+	/*
+	 * Plain (non-special) value bytes must survive the recycle path:
+	 * every probe must observe the magic value written by upd_elem() in
+	 * the same iteration, regardless of whether the element memory was
+	 * recycled or freshly allocated.
+	 */
+	ASSERT_EQ(retries, 0, "no_unexpected_recreate");
+	ASSERT_EQ(read_counter(skel, 0) - init_before, 2000, "init_loop_count");
+	ASSERT_EQ(read_counter(skel, 4), 2000, "recycle_magic_roundtrip");
+	ASSERT_GT(read_counter(skel, 2), nonnull_before, "recycle_xchg_non_null");
+out:
+	rhtab_kptr__destroy(skel);
+out_free:
+	free(cpu_vals);
+}
diff --git a/tools/testing/selftests/bpf/progs/rhtab_kptr.c b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
new file mode 100644
index 000000000000..c96cf7f2d799
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+/*
+ * Verify that the rhtab update/delete recycle paths do not eagerly destroy
+ * referenced kptrs. rhtab must match the hash map semantics introduced by
+ * commit a3a81d247651 ("bpf: Cancel special fields on map value recycle"):
+ * only NMI-safe fields (timer, workqueue, task_work) are cancelled on
+ * update/delete, while kptrs stay attached to the recycled element until it
+ * is eventually freed.
+ *
+ * Two paths are exercised:
+ *  1. a perf_event (NMI) program overwrites an existing element; without the
+ *     fix the NMI update releases the old kptr and probe_elem() observes
+ *     NULL;
+ *  2. the element is deleted and re-inserted; the re-insertion may recycle
+ *     the freed element, and zeroing the inherited kptr slot (as
+ *     check_and_init_map_value() did before the fix) would drop the
+ *     reference without releasing it. probe_elem() must observe the
+ *     inherited non-NULL pointer, and plain (non-special) value bytes must
+ *     still round-trip through the recycled element.
+ *
+ * The delete program checks that the element really disappeared, otherwise
+ * the following update would be an in-place update whose value copy skips
+ * the special fields, and the surviving kptr would prove nothing about the
+ * recycle path.
+ */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "rhtab_kptr_common.h"
+
+char LICENSE[] SEC("license") = "GPL";
+
+struct val_t {
+	struct task_struct __kptr *tsk;
+	__u32 magic;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct val_t);
+} rhtab SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(max_entries, 5);
+	__type(key, __u32);
+	__type(value, __u64);
+} counters SEC(".maps");
+
+/* 0: init ok, 1: nmi update ok, 2: probe xchg non-NULL, 3: probe xchg NULL,
+ * 4: probe saw expected magic value
+ */
+static __always_inline void bump(u32 idx)
+{
+	u64 *v = bpf_map_lookup_elem(&counters, &idx);
+
+	if (v)
+		(*v)++;
+}
+
+extern struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+extern void bpf_task_release(struct task_struct *p) __ksym;
+
+SEC("perf_event")
+int nmi_update(struct bpf_perf_event_data *ctx)
+{
+	struct val_t val = {};
+	u32 key = 0;
+
+	if (bpf_map_update_elem(&rhtab, &key, &val, BPF_ANY) == 0)
+		bump(1);
+	return 0;
+}
+
+SEC("syscall")
+int init_elem(void *ctx)
+{
+	struct val_t *val;
+	struct task_struct *task, *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&rhtab, &key);
+	if (!val)
+		return 1;
+	task = bpf_task_acquire(bpf_get_current_task_btf());
+	if (!task)
+		return 2;
+	old = bpf_kptr_xchg(&val->tsk, task);
+	if (old)
+		bpf_task_release(old);
+	bump(0);
+	return 0;
+}
+
+SEC("syscall")
+int del_elem(void *ctx)
+{
+	u32 key = 0;
+
+	if (bpf_map_delete_elem(&rhtab, &key))
+		return 1;
+	/* The element must really be gone: otherwise the following upd_elem()
+	 * is an in-place update on the surviving element and the kptr that
+	 * probe_elem() observes never went through a recycle.
+	 */
+	if (bpf_map_lookup_elem(&rhtab, &key))
+		return 2;
+	return 0;
+}
+
+SEC("syscall")
+int upd_elem(void *ctx)
+{
+	struct val_t val = { .magic = RHTAB_MAGIC };
+	u32 key = 0;
+
+	if (bpf_map_update_elem(&rhtab, &key, &val, BPF_ANY))
+		return 1;
+	return 0;
+}
+
+SEC("syscall")
+int probe_elem(void *ctx)
+{
+	struct val_t *val;
+	struct task_struct *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&rhtab, &key);
+	if (!val)
+		return 1;
+	old = bpf_kptr_xchg(&val->tsk, NULL);
+	if (old) {
+		bpf_task_release(old);
+		bump(2);
+	} else {
+		bump(3);
+	}
+	if (val->magic == RHTAB_MAGIC)
+		bump(4);
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/rhtab_kptr_common.h b/tools/testing/selftests/bpf/rhtab_kptr_common.h
new file mode 100644
index 000000000000..1b629fcb6742
--- /dev/null
+++ b/tools/testing/selftests/bpf/rhtab_kptr_common.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+#pragma once
+
+/* Magic value stored in the plain bytes of rhtab values ("RHAS"). */
+#define RHTAB_MAGIC 0x52484153
-- 
2.54.0

Re: [PATCH bpf-next v3 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Double close of `pmu_fd` in the success path.
- [Low] Missing FD validation for manually looked up BPF program.
- [Low] Multi-line comments do not follow the mandatory BPF subsystem style.
- [Low] Calling test__skip() marks the entire test as skipped, but execution continues to the recycle phase, masking its success on systems without a PMU.
--

commit 2b5822e0c1417c4403ffbea06426d15e411775a3
Author: Yuan Chen <chenyuan@kylinos.cn>

selftests/bpf: Test rhtab kptr recycle from NMI context

A perf_event program running in NMI context overwrites a rhtab element whose
value holds a referenced task kptr to verify it stays attached on recycle. A
second phase deletes and re-inserts the element 2000 times, requiring at
least one recycle to inherit the kptr. The NMI phase is skipped when no
hardware PMU is available.

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
> index 0000000000000..4bdcc9ce5500f
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
[ ... ]
> +/* Userspace mirror of the BPF-side struct val_t (progs/rhtab_kptr.c). The
> + * update syscall copies map->value_size bytes from the buffer, so it must
> + * be at least that large; special fields are skipped by the value copy but
> + * the kernel still reads the full value_size from userspace.
> + */
> +struct val_t_user {

[Severity: Low]
This isn't a bug, but does this comment violate the BPF subsystem multi-line
comment style? The guidelines mandate that multi-line comments must have the
opening /* on its own line, with text starting on the subsequent line.

[ ... ]
> +/* Run @name via BPF_PROG_TEST_RUN, asserting both the syscall status and
> + * that the program exited 0. Returns 0 on success.
> + */
> +static int run_prog_ok(struct rhtab_kptr *skel, const char *name)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> +	struct bpf_program *prog;
> +	int err;
> +
> +	prog = bpf_object__find_program_by_name(skel->obj, name);
> +	if (!ASSERT_OK_PTR(prog, name))
> +		return -1;
> +	err = bpf_prog_test_run_opts(bpf_program__fd(prog), &topts);

[Severity: Low]
Is it safe to use the file descriptor returned by bpf_program__fd() here
without checking it?

The BPF Skeleton API guidelines require that file descriptors returned by
bpf_program__fd() when using manual lookup APIs like
bpf_object__find_program_by_name() must be validated (e.g., using ASSERT_GE)
before use.

[ ... ]
> +	pmu_fd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, 0);
> +	if (pmu_fd >= 0) {
> +		skel->links.nmi_update = bpf_program__attach_perf_event(skel->progs.nmi_update,
> +									pmu_fd);
> +		if (!ASSERT_OK_PTR(skel->links.nmi_update, "attach_perf_event")) {
> +			close(pmu_fd);
> +			goto out;
> +		}
> +
> +		/* Let the NMI handler overwrite the element, and make sure it
> +		 * actually ran before probing (otherwise the probe would pass
> +		 * vacuously even on an unfixed kernel).
> +		 */

[Severity: Low]
This isn't a bug, but there's another instance of the multi-line comment
style violation here.

> +		for (i = 0; i < 20 && read_counter(skel, 1) == 0; i++)
> +			usleep(100000);
> +		ASSERT_GT(read_counter(skel, 1), 0, "nmi_update_ran");
> +
> +		bpf_link__destroy(skel->links.nmi_update);
> +		skel->links.nmi_update = NULL;
> +		close(pmu_fd);

[Severity: Medium]
Will this result in a double close of pmu_fd?

When bpf_link__destroy() is called on this link, it invokes
bpf_link_perf_detach(), which internally closes the pmu_fd. Calling
close(pmu_fd) immediately after could inadvertently close an unrelated file
descriptor opened by a concurrent test thread.

[ ... ]
> +		ASSERT_EQ(read_counter(skel, 2), 1, "xchg_non_null");
> +		ASSERT_EQ(read_counter(skel, 3), 0, "xchg_null");
> +	} else {
> +		test__skip();
> +	}

[Severity: Low]
When test__skip() is called here, execution continues to the recycle phase
below rather than returning early. Could this hide the success of the recycle
phase under a SKIPPED test status on systems without a PMU?

> +
> +	/*
> +	 * Now exercise the delete/re-insert recycle path. The delete only
> +	 * cancels NMI-safe fields, so the freed element still owns the kptr.
[ ... ]
> +		if (run_prog_ok(skel, "init_elem") != 0) {
> +			/* init_elem fails only if the element is missing,
> +			 * which must not happen in this single-threaded
> +			 * loop; count it so a rhtab bug cannot be absorbed
> +			 * silently.
> +			 */

[Severity: Low]
This isn't a bug, but this comment also violates the BPF subsystem multi-line
comment style.

> +			retries++;
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/rhtab_kptr.c b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
> index 0000000000000..c96cf7f2d799b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
[ ... ]
> +/* 0: init ok, 1: nmi update ok, 2: probe xchg non-NULL, 3: probe xchg NULL,
> + * 4: probe saw expected magic value
> + */
> +static __always_inline void bump(u32 idx)

[Severity: Low]
This isn't a bug, but this comment also violates the BPF subsystem multi-line
comment style.

[ ... ]
> +SEC("syscall")
> +int del_elem(void *ctx)
> +{
> +	u32 key = 0;
> +
> +	if (bpf_map_delete_elem(&rhtab, &key))
> +		return 1;
> +	/* The element must really be gone: otherwise the following upd_elem()
> +	 * is an in-place update on the surviving element and the kptr that
> +	 * probe_elem() observes never went through a recycle.
> +	 */

[Severity: Low]
This isn't a bug, but this comment also violates the BPF subsystem multi-line
comment style.

> +	if (bpf_map_lookup_elem(&rhtab, &key))
> +		return 2;
> +	return 0;
> +}

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

Re: [PATCH bpf-next v3 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/progs/rhtab_kptr.c b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
> new file mode 100644
> index 0000000000000..c96cf7f2d799b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 KylinSoft Co., Ltd. */
> +
> +/*
> + * Verify that the rhtab update/delete recycle paths do not eagerly destroy
> + * referenced kptrs. rhtab must match the hash map semantics introduced by
> + * commit a3a81d247651 ("bpf: Cancel special fields on map value recycle"):
> + * only NMI-safe fields (timer, workqueue, task_work) are cancelled on
> + * update/delete, while kptrs stay attached to the recycled element until it
> + * is eventually freed.

[ ... ]

> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include "rhtab_kptr_common.h"

[ ... ]

> diff --git a/tools/testing/selftests/bpf/rhtab_kptr_common.h b/tools/testing/selftests/bpf/rhtab_kptr_common.h
> new file mode 100644
> index 0000000000000..1b629fcb6742d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/rhtab_kptr_common.h
> @@ -0,0 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2026 KylinSoft Co., Ltd. */
> +#pragma once
> +
> +/* Magic value stored in the plain bytes of rhtab values ("RHAS"). */
> +#define RHTAB_MAGIC 0x52484153

This isn't a bug, but since only progs/rhtab_kptr.c uses RHTAB_MAGIC,
could the define live in that file instead of a separate _common.h
header?

The *_common.h idiom at the selftests root is for constants that
genuinely have to be identical on both the BPF and the userspace side.
Here the userspace side never includes the header and never uses
RHTAB_MAGIC. Moving the define into progs/rhtab_kptr.c next to struct
val_t would avoid relying on the root directory being on the include
path for .bpf.o builds, and root-level headers are not listed in the
.bpf.o prerequisites, so edits to the header do not trigger a rebuild.


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

[PATCH bpf-next v3 4/4] selftests/bpf: Test rhtab special-field combinations

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

BPF_MAP_TYPE_RHASH allows spin locks, timers, workqueues, task_work,
kptrs (referenced, untrusted, per-cpu) and refcounts in map values.
The recycle fix only changes kptr slot handling, so verify each field
combination end to end:

  * lock_kptr: bpf_spin_lock + referenced kptr + plain data in one
    value. BPF_F_LOCK syscall updates/lookups must work before and
    after many delete/re-insert recycle cycles, the referenced kptr
    must be inherited on recycled elements (zeroing it would leak the
    reference), and the plain bytes must round-trip every iteration.
  * timer: arm a bpf_timer and verify it fires, delete the element and
    verify the timer is cancelled, then re-insert (possibly recycling
    the freed element) and arm a fresh timer again.
  * kptr_untrusted: the untrusted kptr must survive the recycle like a
    referenced one.
  * kptr_percpu: the per-cpu kptr reference must survive the recycle
    (zeroing it would leak the reference).

On the unfixed kernel the three kptr subtests fail at the recycle
assertions while the lock and timer paths still pass, isolating the
behavior change to kptr slots only.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 .../selftests/bpf/prog_tests/rhtab_fields.c   | 337 ++++++++++++++++
 .../selftests/bpf/progs/rhtab_fields.c        | 378 ++++++++++++++++++
 .../selftests/bpf/rhtab_fields_common.h       |  19 +
 3 files changed, 734 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_fields.c
 create mode 100644 tools/testing/selftests/bpf/rhtab_fields_common.h

diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
new file mode 100644
index 000000000000..93e8cbacca65
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
@@ -0,0 +1,337 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <test_progs.h>
+#include "rhtab_fields.skel.h"
+#include "rhtab_fields_common.h"
+
+#define RECYCLE_LOOPS 2000
+
+/* Userspace view of the lkmap value. The BPF side owns the real layout;
+ * the spin lock and the kptr are special fields that value copies skip,
+ * so only the plain bytes actually matter here.
+ */
+struct lock_kptr_val_user {
+	__u32 lock;
+	__u32 pad;
+	__u64 tsk;
+	__u32 magic;
+	__u32 pad2;
+};
+
+_Static_assert(sizeof(struct lock_kptr_val_user) == 24, "lkmap layout drift");
+_Static_assert(offsetof(struct lock_kptr_val_user, magic) == 16,
+	       "lkmap magic offset drift");
+
+/*
+ * Zeroed value buffer shared by every create/update issued from userspace.
+ * The update syscall copies map->value_size bytes from this buffer (special
+ * fields among them), so a short stack variable would be read past its end;
+ * BSS is zero-filled and 64 bytes cover every map in the skeleton. Each
+ * caller re-checks the size to keep that true as maps are added.
+ */
+static __u8 zero_val[64];
+
+/* Cached CPU count and scratch buffer for percpu counter summation. */
+static __u64 *cpu_vals;
+static int ncpu;
+
+static __u64 read_counter(struct rhtab_fields *skel, __u32 idx)
+{
+	__u64 sum = 0;
+	int i, err;
+
+	err = bpf_map_lookup_elem(bpf_map__fd(skel->maps.counters), &idx,
+				  cpu_vals);
+	if (!ASSERT_OK(err, "lookup_counter"))
+		return 0;
+	for (i = 0; i < ncpu; i++)
+		sum += cpu_vals[i];
+	return sum;
+}
+
+/*
+ * Run @name through BPF_PROG_TEST_RUN. Returns 0 on success and reports the
+ * program retval through @retval, so callers can tell a syscall failure,
+ * a prog that exited nonzero, and a prog that exited 0 apart.
+ */
+static int run_prog(struct rhtab_fields *skel, const char *name, int *retval)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	struct bpf_program *prog;
+	int err;
+
+	prog = bpf_object__find_program_by_name(skel->obj, name);
+	if (!ASSERT_OK_PTR(prog, name))
+		return -1;
+	err = bpf_prog_test_run_opts(bpf_program__fd(prog), &topts);
+	if (!ASSERT_OK(err, name))
+		return -1;
+	if (retval)
+		*retval = topts.retval;
+	return 0;
+}
+
+/* run_prog() plus the assertion that the program exited 0. */
+static int run_prog_ok(struct rhtab_fields *skel, const char *name)
+{
+	int retval = -1;
+
+	if (!ASSERT_OK(run_prog(skel, name, &retval), name))
+		return -1;
+	if (!ASSERT_EQ(retval, 0, name))
+		return -1;
+	return 0;
+}
+
+/* Create one element filled with zero_val in @map. */
+static int create_zero_elem(struct bpf_map *map, const char *name)
+{
+	__u32 key = 0;
+	int fd;
+
+	if (!ASSERT_LE(bpf_map__value_size(map), sizeof(zero_val),
+		       "value_size_fits"))
+		return -1;
+	fd = bpf_map__fd(map);
+	return bpf_map_update_elem(fd, &key, zero_val, BPF_ANY);
+}
+
+static void recycle_loop(struct rhtab_fields *skel, int map_fd,
+			 const char *init, const char *del,
+			 const char *upd, const char *probe,
+			 int *retries)
+{
+	__u32 key = 0;
+	int i;
+
+	for (i = 0; i < RECYCLE_LOOPS; i++) {
+		if (run_prog_ok(skel, init) != 0) {
+			/* init fails only if the element is missing, which
+			 * must not happen in this single-threaded loop. Count
+			 * it so a rhtab bug cannot be absorbed silently; the
+			 * caller asserts the count is zero.
+			 */
+			(*retries)++;
+			if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key,
+							   zero_val, BPF_ANY),
+				       "recreate_elem"))
+				return;
+			if (run_prog_ok(skel, init) != 0)
+				return;
+		}
+		if (run_prog_ok(skel, del) != 0)
+			return;
+		if (run_prog_ok(skel, upd) != 0)
+			return;
+		if (run_prog_ok(skel, probe) != 0)
+			return;
+	}
+}
+
+static void subtest_lock_kptr(struct rhtab_fields *skel)
+{
+	struct lock_kptr_val_user val = {};
+	struct lock_kptr_val_user out = {};
+	__u64 nonnull_before, total;
+	int map_fd, retries = 0;
+	__u32 key = 0;
+
+	map_fd = bpf_map__fd(skel->maps.lkmap);
+
+	if (!ASSERT_OK(create_zero_elem(skel->maps.lkmap, "create_elem"),
+		       "create_elem"))
+		return;
+
+	/* The spin lock must be usable from the syscall path (BPF_F_LOCK). */
+	val.magic = LK_MAGIC;
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &val, BPF_F_LOCK),
+		       "locked_update"))
+		return;
+	if (!ASSERT_OK(bpf_map_lookup_elem_flags(map_fd, &key, &out, BPF_F_LOCK),
+		       "locked_lookup"))
+		return;
+	ASSERT_EQ(out.magic, LK_MAGIC, "locked_lookup_magic");
+
+	/*
+	 * Delete/re-insert recycle cycles: the referenced kptr must be
+	 * inherited on recycled elements (zeroing it would leak the
+	 * reference) and the plain magic bytes must round-trip every time.
+	 * Every iteration runs exactly one probe, so the two probe counters
+	 * must add up to the loop count; the magic check must hold on every
+	 * single probe.
+	 */
+	nonnull_before = read_counter(skel, 1);
+	recycle_loop(skel, map_fd, "lk_init", "lk_del", "lk_upd", "lk_probe",
+		     &retries);
+	ASSERT_EQ(retries, 0, "no_unexpected_recreate");
+	ASSERT_EQ(read_counter(skel, 0), RECYCLE_LOOPS, "lk_init_count");
+	total = read_counter(skel, 1) + read_counter(skel, 2);
+	ASSERT_EQ(total, RECYCLE_LOOPS, "lk_probe_count");
+	ASSERT_EQ(read_counter(skel, 3), RECYCLE_LOOPS,
+		  "recycle_magic_roundtrip");
+	ASSERT_GT(read_counter(skel, 1), nonnull_before,
+		  "recycle_xchg_non_null");
+
+	/* The spin lock must still work after many recycles. */
+	val.magic = LK_MAGIC + 1;
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &val, BPF_F_LOCK),
+		       "post_recycle_locked_update"))
+		return;
+	memset(&out, 0, sizeof(out));
+	if (!ASSERT_OK(bpf_map_lookup_elem_flags(map_fd, &key, &out, BPF_F_LOCK),
+		       "post_recycle_locked_lookup"))
+		return;
+	ASSERT_EQ(out.magic, LK_MAGIC + 1, "post_recycle_locked_magic");
+}
+
+static void subtest_timer(struct rhtab_fields *skel)
+{
+	int fired, map_fd;
+	__u32 key = 0;
+
+	map_fd = bpf_map__fd(skel->maps.tmap);
+
+	if (!ASSERT_OK(create_zero_elem(skel->maps.tmap, "create_elem"),
+		       "create_elem"))
+		return;
+
+	/* 1. A short-delay timer must fire. */
+	skel->data->timer_delay_ns = 50000;
+	if (run_prog_ok(skel, "arm_timer") != 0)
+		return;
+	usleep(300000);
+	if (!ASSERT_GT(skel->bss->timer_fired, 0, "timer_fired_first"))
+		return;
+
+	/*
+	 * 2. The real cancellation test: arm a long-delay timer on a freshly
+	 * recycled element and delete the element while the timer is still
+	 * pending. If the delete failed to cancel it, the callback would run
+	 * before the sleep below ends. The element must be deleted and
+	 * recreated between arms: bpf_timer_init() returns -EBUSY on an
+	 * element whose timer has not been cancelled and freed yet.
+	 */
+	fired = skel->bss->timer_fired;
+	if (!ASSERT_OK(bpf_map_delete_elem(map_fd, &key),
+		       "delete_before_rearm"))
+		return;
+	if (!ASSERT_OK(create_zero_elem(skel->maps.tmap, "recreate_elem"),
+		       "recreate_before_rearm"))
+		return;
+	skel->data->timer_delay_ns = 200000000;
+	if (run_prog_ok(skel, "arm_timer") != 0)
+		return;
+	if (!ASSERT_OK(bpf_map_delete_elem(map_fd, &key),
+		       "delete_cancels_timer"))
+		return;
+	usleep(300000);
+	ASSERT_EQ(skel->bss->timer_fired, fired, "timer_cancelled_after_delete");
+
+	/* 3. A recycled element can arm and fire a fresh timer again. */
+	skel->data->timer_delay_ns = 50000;
+	if (!ASSERT_OK(create_zero_elem(skel->maps.tmap, "recreate_elem"),
+		       "recreate_second"))
+		return;
+	if (run_prog_ok(skel, "arm_timer") != 0)
+		return;
+	usleep(300000);
+	ASSERT_GT(skel->bss->timer_fired, fired, "timer_fired_second");
+}
+
+struct kptr_recycle_case {
+	const char *name;
+	struct bpf_map **map;
+	const char *init;
+	const char *del;
+	const char *upd;
+	const char *probe;
+	__u32 init_idx;    /* counter bumped on every successful init */
+	__u32 nonnull_idx; /* counter bumped when the kptr was inherited */
+	__u32 null_idx;    /* counter bumped when the kptr was not inherited */
+	__u32 marker_idx;  /* percpu data roundtrip counter, 0 if none */
+};
+
+static void subtest_kptr_recycle(struct rhtab_fields *skel,
+				 const struct kptr_recycle_case *c)
+{
+	__u64 nonnull_before, total;
+	struct bpf_map *map;
+	int map_fd, retries = 0;
+
+	map = *c->map;
+	map_fd = bpf_map__fd(map);
+
+	if (!ASSERT_OK(create_zero_elem(map, "create_elem"), "create_elem"))
+		return;
+
+	/* The kptr must survive the recycle without leaking its reference,
+	 * exactly like the referenced kptr in the lkmap subtest.
+	 */
+	nonnull_before = read_counter(skel, c->nonnull_idx);
+	recycle_loop(skel, map_fd, c->init, c->del, c->upd, c->probe,
+		     &retries);
+	ASSERT_EQ(retries, 0, "no_unexpected_recreate");
+	ASSERT_EQ(read_counter(skel, c->init_idx), RECYCLE_LOOPS,
+		  "init_count");
+	total = read_counter(skel, c->nonnull_idx) +
+		read_counter(skel, c->null_idx);
+	ASSERT_EQ(total, RECYCLE_LOOPS, "probe_count");
+	ASSERT_GT(read_counter(skel, c->nonnull_idx), nonnull_before,
+		  "recycle_non_null");
+	/* The marker read is CPU-local, so assert at least one hit. */
+	if (c->marker_idx)
+		ASSERT_GT(read_counter(skel, c->marker_idx), 0,
+			  "recycle_data_roundtrip");
+}
+
+static void subtest_kptr_recycles(struct rhtab_fields *skel)
+{
+	const struct kptr_recycle_case cases[] = {
+		{ .name = "kptr_untrusted", .map = &skel->maps.umap,
+		  .init = "u_init", .del = "u_del", .upd = "u_upd",
+		  .probe = "u_probe", .init_idx = 4, .nonnull_idx = 5,
+		  .null_idx = 6 },
+		{ .name = "kptr_percpu", .map = &skel->maps.pcmap,
+		  .init = "pc_init", .del = "pc_del", .upd = "pc_upd",
+		  .probe = "pc_probe", .init_idx = 7, .nonnull_idx = 8,
+		  .null_idx = 9, .marker_idx = 10 },
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(cases); i++) {
+		if (test__start_subtest(cases[i].name))
+			subtest_kptr_recycle(skel, &cases[i]);
+	}
+}
+
+void test_rhtab_fields(void)
+{
+	struct rhtab_fields *skel;
+
+	ncpu = libbpf_num_possible_cpus();
+	if (!ASSERT_GT(ncpu, 0, "num_possible_cpus"))
+		return;
+	cpu_vals = calloc(ncpu, sizeof(*cpu_vals));
+	if (!ASSERT_OK_PTR(cpu_vals, "calloc_cpu_vals"))
+		return;
+
+	skel = rhtab_fields__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load")) {
+		free(cpu_vals);
+		return;
+	}
+
+	if (test__start_subtest("lock_kptr"))
+		subtest_lock_kptr(skel);
+	if (test__start_subtest("timer"))
+		subtest_timer(skel);
+	subtest_kptr_recycles(skel);
+
+	rhtab_fields__destroy(skel);
+	free(cpu_vals);
+}
diff --git a/tools/testing/selftests/bpf/progs/rhtab_fields.c b/tools/testing/selftests/bpf/progs/rhtab_fields.c
new file mode 100644
index 000000000000..f8bcd88b7f34
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/rhtab_fields.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+/*
+ * Combined special-field tests for BPF_MAP_TYPE_RHASH. Each map carries a
+ * different field combination and is exercised through delete/re-insert
+ * cycles so the bpf memory allocator recycles element memory:
+ *
+ *  1. lkmap: bpf_spin_lock + referenced kptr + plain data in one value.
+ *     After every recycle the spin lock must still be usable (initialized by
+ *     the alloc path), the referenced kptr must be inherited instead of
+ *     zeroed (zeroing would leak the reference), and the plain bytes must
+ *     round-trip.
+ *  2. tmap: bpf_timer. The delete path must cancel a timer that is still
+ *     pending (the delay is tunable through timer_delay_ns so userspace can
+ *     arm a long timer and delete the element before it fires), and a
+ *     recycled element must be able to arm a fresh timer again.
+ *  3. umap: untrusted (unreferenced) kptr. The inherited pointer must be
+ *     preserved on recycle, matching hash map behavior.
+ *  4. pcmap: per-cpu kptr. Like the referenced kptr, the per-cpu reference
+ *     must not be dropped on recycle, and the object's contents must
+ *     survive the recycle round-trip.
+ *
+ * The delete programs check that the element really disappeared, otherwise
+ * the following update would be an in-place update whose value copy skips
+ * the special fields, and the surviving kptr would prove nothing about the
+ * recycle path.
+ */
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_experimental.h"
+#include "rhtab_fields_common.h"
+
+char LICENSE[] SEC("license") = "GPL";
+
+struct lock_kptr_val {
+	struct bpf_spin_lock lock;
+	struct task_struct __kptr *tsk;
+	__u32 magic;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct lock_kptr_val);
+} lkmap SEC(".maps");
+
+struct timer_val {
+	struct bpf_timer timer;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct timer_val);
+} tmap SEC(".maps");
+
+struct unref_val {
+	struct task_struct __kptr_untrusted *tsk;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct unref_val);
+} umap SEC(".maps");
+
+struct pcval {
+	__u64 v;
+};
+
+struct pcpu_val {
+	struct pcval __percpu_kptr *pc;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct pcpu_val);
+} pcmap SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(max_entries, 11);
+	__type(key, __u32);
+	__type(value, __u64);
+} counters SEC(".maps");
+
+/*  0: lk init ok,       1: lk probe xchg non-NULL,  2: lk probe xchg NULL,
+ *  3: lk probe magic ok,
+ *  4: u init ok,        5: u probe ptr non-NULL,    6: u probe ptr NULL,
+ *  7: pc init ok,       8: pc probe xchg non-NULL,  9: pc probe xchg NULL,
+ * 10: pc probe data roundtrip
+ */
+static __always_inline void bump(u32 idx)
+{
+	u64 *v = bpf_map_lookup_elem(&counters, &idx);
+
+	if (v)
+		(*v)++;
+}
+
+extern struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+extern void bpf_task_release(struct task_struct *p) __ksym;
+extern void bpf_rcu_read_lock(void) __ksym;
+extern void bpf_rcu_read_unlock(void) __ksym;
+
+/* Tunable from userspace (an initialized global lands in .data, so the
+ * driver writes it through skel->data, not skel->bss).
+ */
+int timer_delay_ns = 50000;
+int timer_fired;
+
+/* Map 1: spin lock + referenced kptr + plain data. */
+
+SEC("syscall")
+int lk_init(void *ctx)
+{
+	struct lock_kptr_val *val;
+	struct task_struct *task, *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&lkmap, &key);
+	if (!val)
+		return 1;
+	task = bpf_task_acquire(bpf_get_current_task_btf());
+	if (!task)
+		return 2;
+	old = bpf_kptr_xchg(&val->tsk, task);
+	if (old)
+		bpf_task_release(old);
+	bump(0);
+	return 0;
+}
+
+SEC("syscall")
+int lk_del(void *ctx)
+{
+	u32 key = 0;
+
+	if (bpf_map_delete_elem(&lkmap, &key))
+		return 1;
+	/* The element must really be gone: otherwise the following lk_upd()
+	 * is an in-place update on the surviving element and the kptr that
+	 * lk_probe() then observes never went through a recycle.
+	 */
+	if (bpf_map_lookup_elem(&lkmap, &key))
+		return 2;
+	return 0;
+}
+
+SEC("syscall")
+int lk_upd(void *ctx)
+{
+	struct lock_kptr_val val = { .magic = LK_MAGIC };
+	u32 key = 0;
+
+	/* BPF_ANY is safe even though the value holds a spin lock: value
+	 * copies skip special fields, so the lock word is never written and
+	 * the prog side does not need to take the lock for an update.
+	 */
+	bpf_map_update_elem(&lkmap, &key, &val, BPF_ANY);
+	return 0;
+}
+
+SEC("syscall")
+int lk_probe(void *ctx)
+{
+	struct lock_kptr_val *val;
+	struct task_struct *old;
+	__u32 magic;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&lkmap, &key);
+	if (!val)
+		return 1;
+	/* Take the lock directly: a recycled element that came back with a
+	 * corrupted lock word deadlocks here instead of passing. Helpers are
+	 * forbidden while the lock is held, so the xchg stays outside.
+	 */
+	bpf_spin_lock(&val->lock);
+	magic = val->magic;
+	bpf_spin_unlock(&val->lock);
+	old = bpf_kptr_xchg(&val->tsk, NULL);
+	if (old) {
+		bpf_task_release(old);
+		bump(1);
+	} else {
+		bump(2);
+	}
+	if (magic == LK_MAGIC)
+		bump(3);
+	return 0;
+}
+
+/* Map 2: bpf_timer. */
+
+static int timer_cb(void *map, void *key, struct timer_val *value)
+{
+	timer_fired++;
+	return 0;
+}
+
+SEC("syscall")
+int arm_timer(void *ctx)
+{
+	struct timer_val *val;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&tmap, &key);
+	if (!val)
+		return 1;
+	/* 1 == CLOCK_MONOTONIC */
+	if (bpf_timer_init(&val->timer, &tmap, 1))
+		return 2;
+	bpf_timer_set_callback(&val->timer, timer_cb);
+	if (bpf_timer_start(&val->timer, timer_delay_ns, 0))
+		return 3;
+	return 0;
+}
+
+/* Map 3: untrusted kptr. */
+
+SEC("syscall")
+int u_init(void *ctx)
+{
+	struct unref_val *val;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&umap, &key);
+	if (!val)
+		return 1;
+	val->tsk = bpf_get_current_task_btf();
+	bump(4);
+	return 0;
+}
+
+SEC("syscall")
+int u_del(void *ctx)
+{
+	u32 key = 0;
+
+	if (bpf_map_delete_elem(&umap, &key))
+		return 1;
+	if (bpf_map_lookup_elem(&umap, &key))
+		return 2;
+	return 0;
+}
+
+SEC("syscall")
+int u_upd(void *ctx)
+{
+	struct unref_val val = {};
+	u32 key = 0;
+
+	bpf_map_update_elem(&umap, &key, &val, BPF_ANY);
+	return 0;
+}
+
+SEC("syscall")
+int u_probe(void *ctx)
+{
+	struct unref_val *val;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&umap, &key);
+	if (!val)
+		return 1;
+	if (val->tsk)
+		bump(5);
+	else
+		bump(6);
+	val->tsk = NULL;
+	return 0;
+}
+
+/* Map 4: per-cpu kptr. */
+
+SEC("syscall")
+int pc_init(void *ctx)
+{
+	struct pcpu_val *val;
+	struct pcval *p, *cp, *q, *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&pcmap, &key);
+	if (!val)
+		return 1;
+	p = bpf_percpu_obj_new(struct pcval);
+	if (!p)
+		return 2;
+	old = bpf_kptr_xchg(&val->pc, p);
+	if (old)
+		bpf_percpu_obj_drop(old);
+	/* After the xchg the slot holds p, so q aliases p. Syscall progs are
+	 * sleepable, so the kptr field load only yields a trusted per-cpu
+	 * view under an explicit RCU read lock.
+	 */
+	bpf_rcu_read_lock();
+	q = val->pc;
+	if (q) {
+		cp = bpf_this_cpu_ptr(q);
+		cp->v = PC_MAGIC;
+	}
+	bpf_rcu_read_unlock();
+	bump(7);
+	return 0;
+}
+
+SEC("syscall")
+int pc_del(void *ctx)
+{
+	u32 key = 0;
+
+	if (bpf_map_delete_elem(&pcmap, &key))
+		return 1;
+	if (bpf_map_lookup_elem(&pcmap, &key))
+		return 2;
+	return 0;
+}
+
+SEC("syscall")
+int pc_upd(void *ctx)
+{
+	struct pcpu_val val = {};
+	u32 key = 0;
+
+	bpf_map_update_elem(&pcmap, &key, &val, BPF_ANY);
+	return 0;
+}
+
+SEC("syscall")
+int pc_probe(void *ctx)
+{
+	struct pcpu_val *val;
+	struct pcval *cp, *q, *old;
+	u32 key = 0;
+	int marker = 0;
+
+	val = bpf_map_lookup_elem(&pcmap, &key);
+	if (!val)
+		return 1;
+	/* The object was freshly allocated and marked by pc_init() within
+	 * the same iteration, so an inherited object must still carry the
+	 * marker; q aliases old until the drop. Read the marker under the
+	 * RCU read lock (which makes the field load trusted) and before the
+	 * xchg NULLs the field. The read is CPU-local and the loop may
+	 * migrate between CPUs, so userspace only asserts that this fired
+	 * at least once.
+	 */
+	bpf_rcu_read_lock();
+	q = val->pc;
+	if (q) {
+		cp = bpf_this_cpu_ptr(q);
+		if (cp->v == PC_MAGIC)
+			marker = 1;
+	}
+	bpf_rcu_read_unlock();
+	old = bpf_kptr_xchg(&val->pc, NULL);
+	if (old) {
+		if (marker)
+			bump(10);
+		bpf_percpu_obj_drop(old);
+		bump(8);
+	} else {
+		bump(9);
+	}
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/rhtab_fields_common.h b/tools/testing/selftests/bpf/rhtab_fields_common.h
new file mode 100644
index 000000000000..b52fb5e62807
--- /dev/null
+++ b/tools/testing/selftests/bpf/rhtab_fields_common.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+#pragma once
+
+/*
+ * Constants shared between the rhtab_fields BPF program (progs/) and its
+ * userspace driver (prog_tests/) so the two sides cannot drift apart
+ * silently.
+ */
+
+/* Magic value stored in the plain bytes of lkmap values ("RHAB"). */
+#define LK_MAGIC 0x52484142
+
+/*
+ * Marker written into freshly allocated per-cpu objects of pcmap. Distinct
+ * from LK_MAGIC on purpose: a hit proves the data came from our own
+ * pc_init(), not from anything else.
+ */
+#define PC_MAGIC 0x43504d47
-- 
2.54.0

Re: [PATCH bpf-next v3 4/4] selftests/bpf: Test rhtab special-field combinations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing file descriptor validation after manual program lookup.
- [Low] Multi-line comments violate the modern BPF kernel comment style by placing text on the opening line.
--

commit c8d6d3baf7be1c36675cff2687886747c8f66dba
Author: Yuan Chen <chenyuan@kylinos.cn>

selftests/bpf: Test rhtab special-field combinations
    
This commit introduces tests for BPF_MAP_TYPE_RHASH with different special
field combinations like spin locks, timers, and kptrs. It verifies element
recycling behavior and ensures references are correctly maintained.

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
> new file mode 100644
> index 0000000000000..93e8cbacca656
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
> @@ -0,0 +1,337 @@
[ ... ]
> +/* Userspace view of the lkmap value. The BPF side owns the real layout;
> + * the spin lock and the kptr are special fields that value copies skip,
> + * so only the plain bytes actually matter here.
> + */

[Severity: Low]
This isn't a bug, but does this violate the modern kernel multi-line comment
style?

The preferred BPF style requires the opening /* to be on its own line. There
are several multi-line comments in this patch (both here and in
tools/testing/selftests/bpf/progs/rhtab_fields.c) that put text on the same
line as the opening /*.

[ ... ]
> +static int run_prog(struct rhtab_fields *skel, const char *name, int *retval)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> +	struct bpf_program *prog;
> +	int err;
> +
> +	prog = bpf_object__find_program_by_name(skel->obj, name);
> +	if (!ASSERT_OK_PTR(prog, name))
> +		return -1;
> +	err = bpf_prog_test_run_opts(bpf_program__fd(prog), &topts);

[Severity: Medium]
When manually looking up a program with bpf_object__find_program_by_name(),
bpf_program__fd() can return a negative value if the program is not valid.

Could we pass a negative file descriptor into bpf_prog_test_run_opts() here
if that happens? Should we validate the file descriptor with ASSERT_GE()
before using it?

> +	if (!ASSERT_OK(err, name))
> +		return -1;
> +	if (retval)
> +		*retval = topts.retval;
> +	return 0;
> +}

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

Re: [PATCH bpf-next v3 4/4] selftests/bpf: Test rhtab special-field combinations

[bot+bpf-ci]

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

> BPF_MAP_TYPE_RHASH allows spin locks, timers, workqueues, task_work,
> kptrs (referenced, untrusted, per-cpu) and refcounts in map values.
> The recycle fix only changes kptr slot handling, so verify each field
> combination end to end:

The commit message says it verifies "each field combination" from the
enumerated list of seven special-field kinds (spin locks, timers,
workqueues, task_work, kptrs, refcounts). However, test_rhtab_fields()
only dispatches four subtests covering five field types: BPF_SPIN_LOCK,
BPF_TIMER, BPF_KPTR_REF, BPF_KPTR_UNREF and BPF_KPTR_PERCPU.

Three field kinds listed in the message are never exercised: BPF_WORKQUEUE
(struct bpf_wq), BPF_TASK_WORK (struct bpf_task_work) and BPF_REFCOUNT
(struct bpf_refcount). These are supported configurations that the recycle
path must handle - map_check_btf() explicitly whitelists BPF_MAP_TYPE_RHASH
for all three in kernel/bpf/syscall.c.

Could the commit message be narrowed to match the actual coverage (e.g.
"verify the spin-lock, timer and kptr combinations")?

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
> new file mode 100644
> index 0000000000000..93e8cbacca656
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
> @@ -0,0 +1,337 @@

[ ... ]

> +/* Userspace view of the lkmap value. The BPF side owns the real layout;
> + * the spin lock and the kptr are special fields that value copies skip,
> + * so only the plain bytes actually matter here.
> + */

Three multi-line comments in this file start their text on the opening
'/*' line instead of putting '/*' on its own line (here at line 14, line
114, and line 272). The BPF subsystem's comment style guide requires the
opening '/*' on its own line even in selftests, and the same file already
uses this style correctly elsewhere (lines 30, 57, 160, 211). Could these
three be adjusted to match?

[ ... ]

> +static void recycle_loop(struct rhtab_fields *skel, int map_fd,
> +			 const char *init, const char *del,
> +			 const char *upd, const char *probe,
> +			 int *retries)
> +{

[ ... ]

> +	for (i = 0; i < RECYCLE_LOOPS; i++) {
> +		if (run_prog_ok(skel, init) != 0) {
> +			/* init fails only if the element is missing, which
> +			 * must not happen in this single-threaded loop. Count
> +			 * it so a rhtab bug cannot be absorbed silently; the
> +			 * caller asserts the count is zero.
> +			 */
> +			(*retries)++;

The comment states "init fails only if the element is missing" and says
the caller asserts the count is zero, but run_prog_ok() has already
reported the failure through ASSERT_OK()/ASSERT_EQ() which call test__fail(),
so by the time (*retries)++ executes the subtest is already marked failed.

The subsequent ASSERT_EQ(retries, 0, "no_unexpected_recreate") can only add
a second failure line for an already-failed test, and the recreate+retry
recovery merely lets the loop continue after a hard failure.

Also, run_prog_ok() returns non-zero not only when the element is missing,
but also when bpf_object__find_program_by_name() returns NULL, when
bpf_prog_test_run_opts() fails with any errno, or when the BPF program
returns any non-zero value. The BPF-side init programs return 2 on
bpf_task_acquire()/bpf_percpu_obj_new() failure, not only on a missing
element.

Should this branch be dropped and the loop return on first failure (as the
neighboring prog_tests/rhtab_kptr.c effectively does), or should the
failure path not assert before deciding to retry?

[ ... ]

> +static void subtest_lock_kptr(struct rhtab_fields *skel)
> +{
> +	struct lock_kptr_val_user val = {};
> +	struct lock_kptr_val_user out = {};

[ ... ]

> +	val.magic = LK_MAGIC;
> +	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &val, BPF_F_LOCK),
> +		       "locked_update"))

The syscall value buffers are sized by the userspace mirror struct (24
bytes) while the kernel writes map->value_size bytes. If the BPF-side value
struct in progs/rhtab_fields.c ever grows, the test will overrun its own
stack rather than failing cleanly.

create_zero_elem() has the right guard - ASSERT_LE(bpf_map__value_size(map),
sizeof(zero_val), "value_size_fits") - but only for the shared 64-byte
zero_val buffer, not for val/out. The two _Static_assert()s at lines 26-27
only check the mirror against itself.

Would adding an ASSERT_LE(bpf_map__value_size(skel->maps.lkmap),
sizeof(val), ...) make the contract explicit?

[ ... ]

> +static void subtest_timer(struct rhtab_fields *skel)
> +{

[ ... ]

> +	/*
> +	 * 2. The real cancellation test: arm a long-delay timer on a freshly
> +	 * recycled element and delete the element while the timer is still
> +	 * pending. If the delete failed to cancel it, the callback would run

The timer subtest documents itself as exercising the recycle path (the
comment says "arm a long-delay timer on a freshly recycled element", and
step 3's comment at line 235 says "A recycled element can arm and fire a
fresh timer again"), but the element can never be recycled.

rhtab_delete_elem() frees via bpf_mem_cache_free_rcu() -> unit_free_rcu(),
which puts the object on c->free_by_rcu then c->waiting_for_gp. unit_alloc()
pops only from c->free_llist, and alloc_bulk() refills only from
c->free_by_rcu_ttrace / c->waiting_for_gp_ttrace - an object reaches
free_by_rcu_ttrace only after a full RCU grace period.

The subtest deletes and immediately re-creates the element with no grace
period in between, and each rhtab map owns its own bpf_mem_alloc, so tmap
has only ever freed one or two elements. The re-inserted element is
therefore always a fresh allocation, not a recycled one.

This means steps 2 and 3 verify only that delete cancels a pending timer and
that a new element can arm a timer - both useful, but neither is the recycle
behaviour the comments and the commit message claim. Unlike the kptr
subtests, which loop RECYCLE_LOOPS=2000 times and therefore do get recycled
elements, the timer subtest has no loop and no mechanism to force reuse.

Should the timer subtest either loop like the kptr tests do, or have its
comments and the commit message adjusted to reflect that it does not
actually test recycle?

[ ... ]

The immediately preceding commit in the same series (b51e79dfbe300) added
prog_tests/rhtab_kptr.c with largely identical infrastructure. A subsystem
pattern flags adding a new selftest file for a narrow variant of behavior
an existing test already covers: this file's percpu-counter summation helper
read_counter(), its run_prog_ok() wrapper, and its 2000-iteration recycle
loop with the identical retries+recreate_elem recovery block all appear in
rhtab_kptr.c with the same assertion strings ("no_unexpected_recreate",
"recycle_magic_roundtrip", "recycle_xchg_non_null").

The new kptr_untrusted and kptr_percpu cases are variants of coverage that
rhtab_kptr.c provides for referenced kptrs, and the lock/timer cases are
field variants of the same map type the pre-existing rhash.c in this
directory already covers.

Worth asking whether these should be additional subtests and BPF programs in
rhtab_kptr.c or rhash.c, with one shared helper set and one common header,
rather than a third parallel file? The current arrangement means any change
to the recycle-loop helper has to be made in two places.

> diff --git a/tools/testing/selftests/bpf/progs/rhtab_fields.c b/tools/testing/selftests/bpf/progs/rhtab_fields.c
> new file mode 100644
> index 0000000000000..f8bcd88b7f345
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/rhtab_fields.c
> @@ -0,0 +1,378 @@

[ ... ]

> + *  1. lkmap: bpf_spin_lock + referenced kptr + plain data in one value.
> + *     After every recycle the spin lock must still be usable (initialized by
> + *     the alloc path), the referenced kptr must be inherited instead of

The comment says the spin lock of a recycled element is "initialized by the
alloc path". Nothing in the recycle path initializes it.

rhtab_map_update_elem() performs no special-field initialization - commit
8244b62c3a5ed in this same series removed check_and_init_map_value() and
documents that fresh elements come zeroed from the allocator but recycled
elements are not re-initialized. copy_map_value() skips every special-field
offset. bpf_obj_cancel_fields() only touches timer, workqueue and task_work,
not BPF_SPIN_LOCK. __alloc() in kernel/bpf/memalloc.c uses __GFP_ZERO only
for fresh slab memory, not for objects from the per-cpu free list.

So for "after every recycle" - the case this sentence is about - the alloc
path contributes nothing. The lock word is unlocked on a recycled element
only because no one held it when the element was deleted, not because
anything initialized it. The parenthetical asserts an invariant enforced by
initialization when it is only a consequence of the delete-time state.

Could the parenthetical be reworded to say the lock word is left untouched
by the recycle, so it must still be unlocked?

[ ... ]

> +/*  0: lk init ok,       1: lk probe xchg non-NULL,  2: lk probe xchg NULL,
> + *  3: lk probe magic ok,
> + *  4: u init ok,        5: u probe ptr non-NULL,    6: u probe ptr NULL,
> + *  7: pc init ok,       8: pc probe xchg non-NULL,  9: pc probe xchg NULL,
> + * 10: pc probe data roundtrip
> + */

Seven new multi-line comments in this file start their text on the opening
'/*' line instead of on the following line: here at line 98 (counter
legend), line 117 (timer_delay_ns), line 152 (lk_del), line 167 (lk_upd),
line 186 (lk_probe), line 304 (pc_init), and line 352 (pc_probe).

The file's own top-of-file comment (lines 4-28) and the new shared header
rhtab_fields_common.h both already use the required style, so the seven
sites are inconsistent even within this patch. The BPF subsystem comment
style guide explicitly extends the "opening /* on its own line" rule to
selftests even if surrounding code uses the old style.

Could these seven be adjusted to match the file's own top comment?

[ ... ]

The immediately preceding commit in this series added progs/rhtab_kptr.c
with nearly identical init/delete/update/probe program quartets. lk_init/
lk_del/lk_upd/lk_probe here are near-verbatim re-implementations of
init_elem/del_elem/upd_elem/probe_elem there, down to the comment prose. For
example rhtab_kptr.c has upd_elem() with the same body structure as lk_upd()
here, just a different map and struct.

The genuinely new coverage in this patch is the spin-lock/timer/
untrusted-kptr/percpu-kptr field combinations; the plumbing around them is
duplicated. A subsystem pattern flags this: a new selftest file added for a
narrow variant of behavior an existing test already covers more generally.

Worth asking whether rhtab_fields should have been additional map/prog cases
inside the existing rhtab_kptr test (or whether the two files should share a
helper/macro) rather than a second parallel test binary?

[ ... ]

> +SEC("syscall")
> +int lk_upd(void *ctx)
> +{
> +	struct lock_kptr_val val = { .magic = LK_MAGIC };
> +	u32 key = 0;
> +
> +	/* BPF_ANY is safe even though the value holds a spin lock: value
> +	 * copies skip special fields, so the lock word is never written and
> +	 * the prog side does not need to take the lock for an update.
> +	 */
> +	bpf_map_update_elem(&lkmap, &key, &val, BPF_ANY);
> +	return 0;
> +}

All three update programs (lk_upd at line 171, u_upd at line 265, pc_upd at
line 337) discard the bpf_map_update_elem() return value and unconditionally
return 0.

The error is reachable - rhtab_map_update_elem() allocates the element
through bpf_mem_cache_alloc() and returns -ENOMEM on allocation failure (and
-E2BIG when the map is full). Because the prog returns 0 anyway, the
userspace run_prog_ok() wrapper cannot see the failure; it only surfaces one
step later as the following *_probe() returning 1 from its lookup NULL
check, i.e. as a misattributed failure in a different program.

The equivalent function added by the preceding commit in this series does
check it (progs/rhtab_kptr.c upd_elem(): 'if (bpf_map_update_elem(&rhtab,
&key, &val, BPF_ANY)) return 1;'). Could these three match upd_elem() and
return non-zero on failure?

[ ... ]

> +SEC("syscall")
> +int lk_probe(void *ctx)
> +{
> +	struct lock_kptr_val *val;
> +	struct task_struct *old;
> +	__u32 magic;
> +	u32 key = 0;
> +
> +	val = bpf_map_lookup_elem(&lkmap, &key);
> +	if (!val)
> +		return 1;
> +	/* Take the lock directly: a recycled element that came back with a
> +	 * corrupted lock word deadlocks here instead of passing. Helpers are
> +	 * forbidden while the lock is held, so the xchg stays outside.
> +	 */
> +	bpf_spin_lock(&val->lock);

The comment accurately describes the intended behaviour on failure: "a
recycled element that came back with a corrupted lock word deadlocks here
instead of passing". That makes a hard lockup the test's designed failure
signal.

bpf_spin_lock() resolves to __bpf_spin_lock(), which disables preemption
and then spins in arch_spin_lock() with no timeout, no trylock fallback and
no other wake condition. A recycled element whose lock word is non-zero
hangs that CPU inside a BPF program with preemption disabled - the machine
wedges (hard lockup watchdog at best) rather than the selftest reporting a
failure. RECYCLE_LOOPS is 2000, so a bad iteration will be hit.

This matters specifically because, as the FILE-3-CHANGE-1-R1 issue documents,
nothing in the recycle path re-initializes the lock word: bpf_obj_cancel_
fields() does not touch BPF_SPIN_LOCK, copy_map_value() skips it, and only
fresh mem-allocator memory is __GFP_ZERO'd. So the exact regression this
subtest is meant to detect is the one that hangs the box instead of failing.

Would a bpf_spin_lock-free check (reading val->magic plainly, or driving the
locked read from userspace via BPF_F_LOCK as subtest_lock_kptr() already
does) report a failure instead of potentially taking the machine down?

[ ... ]


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

Re: [PATCH bpf-next v3 0/4] bpf: Cancel special fields in resizable hashtab on recycle

[Kumar Kartikeya Dwivedi]

On Tue Sep 1, 2026 at 8:28 AM CEST, chenyuan_fl wrote:
> From: Yuan Chen <chenyuan@kylinos.cn>
>
> v3 addresses Mykyta's review of v2 [3]. Both comments concern patch 1:
>
>   1. "Could you please double check if this is needed at all? I think
>      bpf_map_free_internal_structs() going to reset special fields to 0,
>      so immediate reuse by __bpf_async_init(), bpf_task_work_schedule()
>      correctly identifies fresh fields."
>      -> Correct: bpf_obj_cancel_fields() resets the timer/workqueue/
>      task_work slots in place (xchg to NULL in bpf_async_cancel_and_free()
>      and bpf_task_work_cancel_and_free()), so no re-initialization is
>      needed on reuse. rhtab_init_map_value() is dropped entirely; the
>      element alloc path now matches htab's non-prealloc path
>      (alloc_htab_elem()), which also performs no explicit init.
>
>   2. "Let's directly call bpf_obj_cancel_fields() here and below, so it
>      is consistent with htab."
>      -> Done: the rhtab_cancel_fields() wrapper is removed and
>      rhtab_delete_elem()/rhtab_map_update_existing() call
>      bpf_obj_cancel_fields() directly.
>

I split out two fixes from this, given Nuoqi's patch was already acked before, I
decided to take that one with Mykyta's ack, and also rewrote commit log and
simplified tests where possible. The remaining issue should be th BTF related
problem, please send that separately, but also simplify it.

The current fix is too convoluted. Just taking extra map BTF refcount for the
dtor record should hopefully suffice.

> The resizable hashtab still eagerly calls bpf_obj_free_fields() on
> element delete/replace, which runs kptr destructors from the caller's
> execution context (unsafe in NMI). This follows the existing discussion
> on RHash special-field recycling (Nuoqi Gui's series [2] and the review
> [1]): patch 1 applies the cancel semantics like hash/array maps
> (a3a81d247651); patch 2 fixes a program-BTF use-after-free in the
> mem-alloc destructor found while testing; patches 3-4 add regression
> tests (NMI update, and per-field delete/re-insert cycles).
>
> Changes since v2 (all from Mykyta's review [3]):
>
>   * Drop rhtab_init_map_value(): bpf_obj_cancel_fields() resets the
>     timer/workqueue/task_work slots to zero on delete, fresh elements
>     come zeroed from the bpf mem allocator, and kptr slots must stay
>     untouched, so no re-initialization is needed (matching htab's
>     non-prealloc path).
>   * Call bpf_obj_cancel_fields() directly instead of a rhtab-specific
>     wrapper, consistent with htab.
>
> [1] https://lore.kernel.org/bpf/DKEVG8ZVJDDQ.2G40FTOIZKU57@gmail.com/
> [2] https://lore.kernel.org/bpf/20260726-f01-23-rhash-cancel-bpf-next-v1-0-6e5e1131d885@mails.tsinghua.edu.cn/
> [3] https://lore.kernel.org/bpf/0560a24d-2cf9-4e5b-aa61-580af1e56de1@gmail.com/
>
> Yuan Chen (4):
>   bpf: Cancel special fields in resizable hashtab on recycle
>   bpf: Fix use-after-free of program BTF in mem-alloc destructor
>   selftests/bpf: Test rhtab kptr recycle from NMI context
>   selftests/bpf: Test rhtab special-field combinations
>
>  kernel/bpf/hashtab.c                               |  98 +++++-
>  .../selftests/bpf/prog_tests/rhtab_fields.c        | 337 ++++++++++++++++++
>  .../testing/selftests/bpf/prog_tests/rhtab_kptr.c  | 184 ++++++++++
>  tools/testing/selftests/bpf/progs/rhtab_fields.c   | 378 +++++++++++++++++++++
>  tools/testing/selftests/bpf/progs/rhtab_kptr.c     | 146 ++++++++
>  tools/testing/selftests/bpf/rhtab_fields_common.h  |  19 ++
>  tools/testing/selftests/bpf/rhtab_kptr_common.h    |   6 +
>  7 files changed, 1152 insertions(+), 16 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
>  create mode 100644 tools/testing/selftests/bpf/progs/rhtab_fields.c
>  create mode 100644 tools/testing/selftests/bpf/progs/rhtab_kptr.c
>  create mode 100644 tools/testing/selftests/bpf/rhtab_fields_common.h
>  create mode 100644 tools/testing/selftests/bpf/rhtab_kptr_common.h
>
> --
> 2.54.0

[PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

bpf_ma_set_dtor() duplicates the map's btf_record for the bpf_mem_alloc
destructor. For kptr fields backed by the program BTF (MEM_ALLOC kptrs,
e.g. objects allocated with bpf_obj_new()/bpf_percpu_obj_new()),
btf_record_dup() only borrows the reference, matching what
btf_parse_fields() did for the map's own record.

The duplicated record, however, is released later from the deferred
bpf_mem_alloc destructor workqueue (free_mem_alloc_deferred), by which
time the program BTF may already have been freed: bpf_map_free() drops
the map's own reference, and the RCU callback can run before the
workqueue. Reading field->kptr.btf in btf_record_free() (via
btf_is_kernel()) is then a use-after-free, detected by KASAN as
"slab-use-after-free in btf_is_kernel" when a map with a MEM_ALLOC kptr
field is destroyed.

Hold a reference on program BTF for the lifetime of the duplicated
record and drop it right before the record is freed. The last btf_put()
only schedules the object for RCU destruction, so btf_record_free() can
still safely read the field descriptors.

The rhtab kptr selftests exercise this path on every map teardown and
triggered the bug under KASAN; with this fix they pass cleanly.

Fixes: 1df97a7453ee ("bpf: Register dtor for freeing special fields")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/hashtab.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 0df8db27cd8c..b8df2bc9a9a0 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -493,10 +493,54 @@ static void htab_pcpu_mem_dtor(void *obj, void *ctx)
 		bpf_obj_free_fields(hrec->record, per_cpu_ptr(pptr, cpu));
 }
 
+/*
+ * bpf_ma_set_dtor() duplicates the map's btf_record. For kptr fields whose
+ * btf is the program BTF (MEM_ALLOC kptrs, e.g. objects allocated with
+ * bpf_obj_new()/bpf_percpu_obj_new()) btf_record_dup() only borrows the
+ * reference, like btf_parse_fields() did for the map's own record. The
+ * duplicated record is released later from the deferred bpf_mem_alloc
+ * destructor workqueue, by which time the program BTF may already have been
+ * freed (the map dropped its own reference in bpf_map_free()), so reading
+ * field->kptr.btf there would be a use-after-free.
+ *
+ * Hold a reference on non-kernel (program) BTF for the lifetime of the
+ * duplicated record and release it before the record is freed. After the
+ * last btf_put() the object is only destroyed after an RCU grace period, so
+ * btf_record_free() can still safely read the field descriptors.
+ */
+static void htab_record_prog_btf_ref(struct btf_record *rec, bool get)
+{
+	int i;
+
+	if (IS_ERR_OR_NULL(rec))
+		return;
+
+	for (i = 0; i < rec->cnt; i++) {
+		const struct btf_field *field = &rec->fields[i];
+
+		switch (field->type) {
+		case BPF_KPTR_UNREF:
+		case BPF_KPTR_REF:
+		case BPF_KPTR_PERCPU:
+		case BPF_UPTR:
+			if (field->kptr.btf && !btf_is_kernel(field->kptr.btf)) {
+				if (get)
+					btf_get(field->kptr.btf);
+				else
+					btf_put(field->kptr.btf);
+			}
+			break;
+		default:
+			break;
+		}
+	}
+}
+
 static void htab_dtor_ctx_free(void *ctx)
 {
 	struct htab_btf_record *hrec = ctx;
 
+	htab_record_prog_btf_ref(hrec->record, false);
 	btf_record_free(hrec->record);
 	kfree(ctx);
 }
@@ -521,6 +565,7 @@ static int bpf_ma_set_dtor(struct bpf_map *map, struct bpf_mem_alloc *ma,
 		kfree(hrec);
 		return err;
 	}
+	htab_record_prog_btf_ref(hrec->record, true);
 	bpf_mem_alloc_set_dtor(ma, dtor, htab_dtor_ctx_free, hrec);
 	return 0;
 }
-- 
2.54.0

Re: [PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor

[sashiko-bot]

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

New issues:
- [High] Race condition in `htab_dtor_ctx_free` leading to a use-after-free of the program BTF.

Pre-existing issues:
- [Critical] The patch fails to fix the use-after-free for BPF_LIST_HEAD and BPF_RB_ROOT fields, which also rely on the program BTF.
--

--- Patch [2]: [PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor ---
commit 67cf446d3408eefa2548db164b549cbc9a9e17a2
Author: Yuan Chen <chenyuan@kylinos.cn>
Subject: bpf: Fix use-after-free of program BTF in mem-alloc destructor

This commit addresses a use-after-free issue of the program BTF in the
bpf_mem_alloc destructor. It holds a reference to the program BTF for the
lifetime of the duplicated record and drops it right before the record is
freed.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 0df8db27cd8cc..b8df2bc9a9a00 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
[ ... ]
> @@ -493,10 +493,54 @@ static void htab_pcpu_mem_dtor(void *obj, void *ctx)
>  		bpf_obj_free_fields(hrec->record, per_cpu_ptr(pptr, cpu));
>  }
>  
> +/*
> + * bpf_ma_set_dtor() duplicates the map's btf_record. For kptr fields whose
> + * btf is the program BTF (MEM_ALLOC kptrs, e.g. objects allocated with
> + * bpf_obj_new()/bpf_percpu_obj_new()) btf_record_dup() only borrows the
> + * reference, like btf_parse_fields() did for the map's own record. The
> + * duplicated record is released later from the deferred bpf_mem_alloc
> + * destructor workqueue, by which time the program BTF may already have been
> + * freed (the map dropped its own reference in bpf_map_free()), so reading
> + * field->kptr.btf there would be a use-after-free.
> + *
> + * Hold a reference on non-kernel (program) BTF for the lifetime of the
> + * duplicated record and release it before the record is freed. After the
> + * last btf_put() the object is only destroyed after an RCU grace period, so
> + * btf_record_free() can still safely read the field descriptors.
> + */
> +static void htab_record_prog_btf_ref(struct btf_record *rec, bool get)
> +{
> +	int i;
> +
> +	if (IS_ERR_OR_NULL(rec))
> +		return;
> +
> +	for (i = 0; i < rec->cnt; i++) {
> +		const struct btf_field *field = &rec->fields[i];
> +
> +		switch (field->type) {
> +		case BPF_KPTR_UNREF:
> +		case BPF_KPTR_REF:
> +		case BPF_KPTR_PERCPU:
> +		case BPF_UPTR:

[Severity: Critical]
This is a pre-existing issue, but does this switch statement also need to
handle BPF_LIST_HEAD and BPF_RB_ROOT fields?

These field types also rely on the program BTF. When a map containing them
is destroyed, the deferred mem-alloc destructor is scheduled. Because we
don't retain a reference to the program BTF for these types, the program
BTF can be freed when the map drops its reference.

Later, when the deferred destructor calls bpf_obj_free_fields(),
bpf_list_head_free() could dereference the dangling
field->graph_root.value_rec pointer (which points into the freed program
BTF's struct_meta_tab):

kernel/bpf/helpers.c:bpf_list_head_free() {
    ...
    __bpf_obj_drop_impl((char *)pos - field->graph_root.node_offset,
                        field->graph_root.value_rec, false);
    ...
}

> +			if (field->kptr.btf && !btf_is_kernel(field->kptr.btf)) {
> +				if (get)
> +					btf_get(field->kptr.btf);
> +				else
> +					btf_put(field->kptr.btf);
> +			}
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
>  static void htab_dtor_ctx_free(void *ctx)
>  {
>  	struct htab_btf_record *hrec = ctx;
>  
> +	htab_record_prog_btf_ref(hrec->record, false);
> +	btf_record_free(hrec->record);

[Severity: High]
Can this sequence introduce a race condition leading to a use-after-free of
the program BTF?

This code executes in a preemptible workqueue context
(free_mem_alloc_deferred) without RCU read protection. When
htab_record_prog_btf_ref() is called with get=false, it drops the program
BTF reference. If this is the last reference, the BTF object is scheduled
for RCU destruction.

If the thread is preempted immediately after this call and an RCU grace
period elapses, the BTF memory could be freed. Then the next line calls
btf_record_free(), which iterates over the fields and dereferences the
potentially freed kptr.btf inside btf_is_kernel():

kernel/bpf/syscall.c:btf_record_free() {
    ...
    if (btf_is_kernel(rec->fields[i].kptr.btf))
        btf_put(rec->fields[i].kptr.btf);
    ...
}

>  	kfree(ctx);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824143621.2098856-1-chenyuan_fl@163.com?part=2

Re: [PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 0df8db27cd8c..b8df2bc9a9a0 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c

[ ... ]

> +/*
> + * bpf_ma_set_dtor() duplicates the map's btf_record. For kptr fields whose
> + * btf is the program BTF (MEM_ALLOC kptrs, e.g. objects allocated with
> + * bpf_obj_new()/bpf_percpu_obj_new()) btf_record_dup() only borrows the
> + * reference, like btf_parse_fields() did for the map's own record. The
> + * duplicated record is released later from the deferred bpf_mem_alloc
> + * destructor workqueue, by which time the program BTF may already have been
> + * freed (the map dropped its own reference in bpf_map_free()), so reading
> + * field->kptr.btf there would be a use-after-free.
> + *
> + * Hold a reference on non-kernel (program) BTF for the lifetime of the
> + * duplicated record and release it before the record is freed. After the
> + * last btf_put() the object is only destroyed after an RCU grace period, so
> + * btf_record_free() can still safely read the field descriptors.
> + */
> +static void htab_record_prog_btf_ref(struct btf_record *rec, bool get)
> +{
> +	int i;
> +
> +	if (IS_ERR_OR_NULL(rec))
> +		return;
> +
> +	for (i = 0; i < rec->cnt; i++) {
> +		const struct btf_field *field = &rec->fields[i];
> +
> +		switch (field->type) {
> +		case BPF_KPTR_UNREF:
> +		case BPF_KPTR_REF:
> +		case BPF_KPTR_PERCPU:
> +		case BPF_UPTR:
> +			if (field->kptr.btf && !btf_is_kernel(field->kptr.btf)) {
> +				if (get)
> +					btf_get(field->kptr.btf);
> +				else
> +					btf_put(field->kptr.btf);
> +			}
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
>  static void htab_dtor_ctx_free(void *ctx)
>  {
>  	struct htab_btf_record *hrec = ctx;
>
> +	htab_record_prog_btf_ref(hrec->record, false);
>  	btf_record_free(hrec->record);
>  	kfree(ctx);
>  }

Does this ordering still have a use-after-free?

The comment states that "After the last btf_put() the object is only
destroyed after an RCU grace period, so btf_record_free() can still safely
read the field descriptors." But in htab_dtor_ctx_free(), the program-BTF
reference is dropped one statement before the record is freed:

    htab_record_prog_btf_ref(hrec->record, false);   /* btf_put() */
    btf_record_free(hrec->record);                   /* reads field->kptr.btf */

htab_record_prog_btf_ref(..., false) ends in btf_put(field->kptr.btf).
Looking at btf_put() in kernel/bpf/btf.c, it only queues the object for RCU
destruction if this was the last reference:

    void btf_put(struct btf *btf)
    {
            if (btf && refcount_dec_and_test(&btf->refcnt)) {
                    btf_free_id(btf);
                    call_rcu(&btf->rcu, btf_free_rcu);
            }
    }

and btf_free_rcu() -> btf_free() -> kfree(btf). The very next statement,
btf_record_free() in kernel/bpf/syscall.c, does for every kptr field:

            if (btf_is_kernel(rec->fields[i].kptr.btf))
                    btf_put(rec->fields[i].kptr.btf);

and btf_is_kernel() is 'return btf->kernel_btf;' - a dereference of the
object whose kfree() was just queued.

In the deferred path this is the last reference, so the drop queues an RCU
free and the following read can land on freed memory. The reachable path is
exactly the scenario described in the commit message:

  bpf_map_free_deferred()
    -> bpf_map_free()
       -> map->ops->map_free (rhtab_map_free / htab_map_free)
          -> bpf_mem_alloc_destroy()
             -> destroy_mem_alloc(), rcu_in_progress != 0
                -> queue_work(system_dfl_wq, &copy->work)
       -> btf_record_free(rec); btf_put(btf);   /* map drops its own BTF ref */
    ... later, on the workqueue ...
  free_mem_alloc_deferred()
    -> free_mem_alloc()
       -> free_mem_alloc_no_barrier()
          -> ma->dtor_ctx_free(ma->dtor_ctx) == htab_dtor_ctx_free

By then the map's reference is gone, so the btf_put() inside
htab_record_prog_btf_ref() drops the last reference and queues
btf_free_rcu. free_mem_alloc_deferred() is a plain, fully preemptible
workqueue worker; there is no rcu_read_lock() anywhere between the work
callback and htab_dtor_ctx_free() or btf_record_free(). Being preempted
outside an RCU read-side critical section is itself a quiescent state, so
on CONFIG_PREEMPT/PREEMPT_RT the grace period can complete and
btf_free_rcu() can run before the worker resumes in btf_record_free().

Could the reference be held until after btf_record_free() has read the
descriptors, or could the pair be wrapped in rcu_read_lock()/
rcu_read_unlock()?

[ ... ]


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

[PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

A perf_event program running in NMI context overwrites a rhtab
element whose value holds a referenced task kptr. The old kptr must
stay attached to the element (cancel semantics, matching hash maps);
before the rhtab recycle fix the NMI update eagerly released it and
the probe observed NULL. The test asserts the NMI program actually
ran, so the probe result is meaningful.

A second phase deletes and re-inserts the element 2000 times. The
re-insertion may recycle the freed element, which still owns the
kptr; before the fix the alloc path zeroed the inherited slot via
check_and_init_map_value(), leaking the reference, and the probe
never observed a non-NULL pointer. The test requires at least one
recycle to inherit the kptr, and also verifies that plain
(non-special) value bytes still round-trip through the recycled
element on every iteration.

The NMI phase is skipped when no hardware PMU is available.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 .../selftests/bpf/prog_tests/rhtab_kptr.c     | 146 ++++++++++++++++++
 .../testing/selftests/bpf/progs/rhtab_kptr.c  | 132 ++++++++++++++++
 2 files changed, 278 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_kptr.c

diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
new file mode 100644
index 000000000000..13158d74cbc1
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+#include <linux/perf_event.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+#include <test_progs.h>
+#include "rhtab_kptr.skel.h"
+
+static __u64 read_counter(struct rhtab_kptr *skel, u32 idx)
+{
+	__u64 vals[libbpf_num_possible_cpus()];
+	__u64 sum = 0;
+	int i, err;
+
+	err = bpf_map_lookup_elem(bpf_map__fd(skel->maps.counters), &idx, vals);
+	if (!ASSERT_OK(err, "lookup_counter"))
+		return 0;
+	for (i = 0; i < libbpf_num_possible_cpus(); i++)
+		sum += vals[i];
+	return sum;
+}
+
+void test_rhtab_kptr(void)
+{
+	struct perf_event_attr attr = {
+		.type = PERF_TYPE_HARDWARE,
+		.config = PERF_COUNT_HW_CPU_CYCLES,
+		.freq = 1,
+		.sample_freq = read_perf_max_sample_freq(),
+		.size = sizeof(struct perf_event_attr),
+	};
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	struct rhtab_kptr *skel;
+	__u32 key = 0;
+	__u64 zero = 0;
+	__u64 nonnull_before;
+	int pmu_fd, i, err;
+
+	skel = rhtab_kptr__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+
+	/* Create the element and stash a referenced task kptr in it. */
+	if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.rhtab),
+					   &key, &zero, BPF_ANY), "create_elem"))
+		goto out;
+	if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.init_elem),
+					      &topts), "test_run_init") ||
+	    !ASSERT_EQ(topts.retval, 0, "init_ret"))
+		goto out;
+
+	pmu_fd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, 0);
+	if (pmu_fd >= 0) {
+		skel->links.nmi_update = bpf_program__attach_perf_event(skel->progs.nmi_update,
+									pmu_fd);
+		if (!ASSERT_OK_PTR(skel->links.nmi_update, "attach_perf_event")) {
+			close(pmu_fd);
+			goto out;
+		}
+
+		/* Let the NMI handler overwrite the element, and make sure it
+		 * actually ran before probing (otherwise the probe would pass
+		 * vacuously even on an unfixed kernel).
+		 */
+		for (i = 0; i < 20 && read_counter(skel, 1) == 0; i++)
+			usleep(100000);
+		ASSERT_GT(read_counter(skel, 1), 0, "nmi_update_ran");
+
+		bpf_link__destroy(skel->links.nmi_update);
+		skel->links.nmi_update = NULL;
+		close(pmu_fd);
+
+		/*
+		 * The old kptr must still be attached to the element: the
+		 * NMI update path only cancels NMI-safe fields, mirroring
+		 * hash map semantics. Before the fix the kptr was released
+		 * from the NMI context and the probe below would see NULL.
+		 */
+		topts.retval = 0;
+		if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.probe_elem),
+						      &topts), "test_run_probe") ||
+		    !ASSERT_EQ(topts.retval, 0, "probe_ret"))
+			goto out;
+
+		ASSERT_EQ(read_counter(skel, 2), 1, "xchg_non_null");
+		ASSERT_EQ(read_counter(skel, 3), 0, "xchg_null");
+	} else {
+		test__skip();
+	}
+
+	/*
+	 * Now exercise the delete/re-insert recycle path. The delete only
+	 * cancels NMI-safe fields, so the freed element still owns the kptr.
+	 * If the re-insertion recycles that element, the kptr must be
+	 * inherited; zeroing it (as check_and_init_map_value() did before
+	 * the fix) leaks the reference and probe_elem() observes NULL.
+	 * Fresh memory handed out by the allocator is zeroed, so NULL probes
+	 * are expected too; only require that the inherited kptr survives at
+	 * least one recycle.
+	 */
+	nonnull_before = read_counter(skel, 2);
+	for (i = 0; i < 2000; i++) {
+		topts.retval = 0;
+		err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.init_elem),
+					     &topts);
+		if (err || topts.retval) {
+			/* Element may be gone; recreate and retry once. */
+			if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.rhtab),
+							   &key, &zero, BPF_ANY),
+				       "recreate_elem"))
+				goto out;
+			topts.retval = 0;
+			err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.init_elem),
+						     &topts);
+		}
+		if (!ASSERT_OK(err, "test_run_init_loop") ||
+		    !ASSERT_EQ(topts.retval, 0, "init_loop_ret"))
+			goto out;
+
+		topts.retval = 0;
+		if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.del_elem),
+						      &topts), "test_run_del"))
+			goto out;
+		topts.retval = 0;
+		if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.upd_elem),
+						      &topts), "test_run_upd"))
+			goto out;
+		topts.retval = 0;
+		if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.probe_elem),
+						      &topts), "test_run_probe"))
+			goto out;
+	}
+
+	/*
+	 * Plain (non-special) value bytes must survive the recycle path:
+	 * every probe must observe the magic value written by upd_elem() in
+	 * the same iteration, regardless of whether the element memory was
+	 * recycled or freshly allocated.
+	 */
+	ASSERT_EQ(read_counter(skel, 4), 2000, "recycle_magic_roundtrip");
+
+	ASSERT_GT(read_counter(skel, 2), nonnull_before, "recycle_xchg_non_null");
+out:
+	rhtab_kptr__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/rhtab_kptr.c b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
new file mode 100644
index 000000000000..fd6bd63cb405
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+/*
+ * Verify that the rhtab update/delete recycle paths do not eagerly destroy
+ * referenced kptrs. rhtab must match the hash map semantics introduced by
+ * commit a3a81d247651 ("bpf: Cancel special fields on map value recycle"):
+ * only NMI-safe fields (timer, workqueue, task_work) are cancelled on
+ * update/delete, while kptrs stay attached to the recycled element until it
+ * is eventually freed.
+ *
+ * Two paths are exercised:
+ *  1. a perf_event (NMI) program overwrites an existing element; without the
+ *     fix the NMI update releases the old kptr and probe_elem() observes
+ *     NULL;
+ *  2. the element is deleted and re-inserted; the re-insertion may recycle
+ *     the freed element, and zeroing the inherited kptr slot (as
+ *     check_and_init_map_value() did before the fix) would drop the
+ *     reference without releasing it. probe_elem() must observe the
+ *     inherited non-NULL pointer, and plain (non-special) value bytes must
+ *     still round-trip through the recycled element.
+ */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+char LICENSE[] SEC("license") = "GPL";
+
+struct val_t {
+	struct task_struct __kptr * tsk;
+	__u32 magic;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct val_t);
+} rhtab SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(max_entries, 5);
+	__type(key, __u32);
+	__type(value, __u64);
+} counters SEC(".maps");
+
+/* 0: init ok, 1: nmi update ok, 2: probe xchg non-NULL, 3: probe xchg NULL,
+ * 4: probe saw expected magic value
+ */
+static __always_inline void bump(u32 idx)
+{
+	u64 *v = bpf_map_lookup_elem(&counters, &idx);
+
+	if (v)
+		(*v)++;
+}
+
+extern struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+extern void bpf_task_release(struct task_struct *p) __ksym;
+
+SEC("perf_event")
+int nmi_update(struct bpf_perf_event_data *ctx)
+{
+	struct val_t val = {};
+	u32 key = 0;
+
+	if (bpf_map_update_elem(&rhtab, &key, &val, BPF_ANY) == 0)
+		bump(1);
+	return 0;
+}
+
+SEC("syscall")
+int init_elem(void *ctx)
+{
+	struct val_t *val;
+	struct task_struct *task, *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&rhtab, &key);
+	if (!val)
+		return 1;
+	task = bpf_task_acquire(bpf_get_current_task_btf());
+	if (!task)
+		return 2;
+	old = bpf_kptr_xchg(&val->tsk, task);
+	if (old)
+		bpf_task_release(old);
+	bump(0);
+	return 0;
+}
+
+SEC("syscall")
+int del_elem(void *ctx)
+{
+	u32 key = 0;
+
+	bpf_map_delete_elem(&rhtab, &key);
+	return 0;
+}
+
+SEC("syscall")
+int upd_elem(void *ctx)
+{
+	struct val_t val = { .magic = 0x52484153 }; /* "RHAS" */
+	u32 key = 0;
+
+	bpf_map_update_elem(&rhtab, &key, &val, BPF_ANY);
+	return 0;
+}
+
+SEC("syscall")
+int probe_elem(void *ctx)
+{
+	struct val_t *val;
+	struct task_struct *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&rhtab, &key);
+	if (!val)
+		return 1;
+	old = bpf_kptr_xchg(&val->tsk, NULL);
+	if (old) {
+		bpf_task_release(old);
+		bump(2);
+	} else {
+		bump(3);
+	}
+	if (val->magic == 0x52484153)
+		bump(4);
+	return 0;
+}
-- 
2.54.0

Re: [PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Userspace stack out-of-bounds read in `bpf_map_update_elem()` due to mismatched value size.
- [Medium] Double close of the perf_event file descriptor (pmu_fd).
- [Low] Multi-line comments do not follow the mandatory BPF subsystem comment style.
--

--- Patch [3]: [PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context ---
commit ce5b7999a24e0290b526f254441d70dcbf59a6cf
Author: Yuan Chen <chenyuan@kylinos.cn>

selftests/bpf: Test rhtab kptr recycle from NMI context

This patch introduces a test to verify that the rhtab update and delete
recycle paths do not eagerly destroy referenced kptrs from NMI context.

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
> index 0000000000000..13158d74cbc1b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c

[ ... ]

> +void test_rhtab_kptr(void)
> +{
> +	struct perf_event_attr attr = {
> +		.type = PERF_TYPE_HARDWARE,
> +		.config = PERF_COUNT_HW_CPU_CYCLES,
> +		.freq = 1,
> +		.sample_freq = read_perf_max_sample_freq(),
> +		.size = sizeof(struct perf_event_attr),
> +	};
> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> +	struct rhtab_kptr *skel;
> +	__u32 key = 0;
> +	__u64 zero = 0;
> +	__u64 nonnull_before;
> +	int pmu_fd, i, err;
> +
> +	skel = rhtab_kptr__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open_and_load"))
> +		return;
> +
> +	/* Create the element and stash a referenced task kptr in it. */
> +	if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.rhtab),
> +					   &key, &zero, BPF_ANY), "create_elem"))

[Severity: Medium]
Does this code cause an out-of-bounds stack read?

The rhtab map expects a value of type struct val_t, which is 16 bytes in size
on 64-bit systems (an 8-byte pointer plus a 4-byte u32 and padding).

However, zero is defined as an 8-byte __u64 local variable. When
bpf_map_update_elem() copies 16 bytes from this address, it will read 8 bytes
of uninitialized or unrelated stack memory adjacent to zero.

> +		goto out;
> +	if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.init_elem),
> +					      &topts), "test_run_init") ||
> +	    !ASSERT_EQ(topts.retval, 0, "init_ret"))
> +		goto out;
> +
> +	pmu_fd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, 0);
> +	if (pmu_fd >= 0) {
> +		skel->links.nmi_update = bpf_program__attach_perf_event(skel->progs.nmi_update,
> +									pmu_fd);
> +		if (!ASSERT_OK_PTR(skel->links.nmi_update, "attach_perf_event")) {
> +			close(pmu_fd);
> +			goto out;
> +		}
> +
> +		/* Let the NMI handler overwrite the element, and make sure it

[Severity: Low]
This isn't a bug, but does this comment violate the BPF subsystem multi-line
comment style?

The BPF subsystem requires multi-line comments to have the opening /* on its
own line.

> +		 * actually ran before probing (otherwise the probe would pass
> +		 * vacuously even on an unfixed kernel).
> +		 */
> +		for (i = 0; i < 20 && read_counter(skel, 1) == 0; i++)
> +			usleep(100000);
> +		ASSERT_GT(read_counter(skel, 1), 0, "nmi_update_ran");
> +
> +		bpf_link__destroy(skel->links.nmi_update);
> +		skel->links.nmi_update = NULL;
> +		close(pmu_fd);

[Severity: Medium]
Does this result in a double close of pmu_fd?

The function bpf_program__attach_perf_event() takes ownership of the passed
perf_event file descriptor upon successful attachment. When bpf_link__destroy()
is called, it automatically closes the file descriptor.

Calling close(pmu_fd) here explicitly could accidentally close an unrelated file
descriptor opened by another thread.

> +
> +		/*
> +		 * The old kptr must still be attached to the element: the

[ ... ]

> +	for (i = 0; i < 2000; i++) {
> +		topts.retval = 0;
> +		err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.init_elem),
> +					     &topts);
> +		if (err || topts.retval) {
> +			/* Element may be gone; recreate and retry once. */
> +			if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.rhtab),
> +							   &key, &zero, BPF_ANY),

[Severity: Medium]
Can this also cause an out-of-bounds stack read?

This is the same issue as above where zero is 8 bytes but the map expects
16 bytes.

> +				       "recreate_elem"))
> +				goto out;

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/rhtab_kptr.c b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
> index 0000000000000..fd6bd63cb405e
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/rhtab_kptr.c

[ ... ]

> +struct {
> +	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
> +	__uint(max_entries, 5);
> +	__type(key, __u32);
> +	__type(value, __u64);
> +} counters SEC(".maps");
> +
> +/* 0: init ok, 1: nmi update ok, 2: probe xchg non-NULL, 3: probe xchg NULL,

[Severity: Low]
This isn't a bug, but should the opening /* be on its own line to match the BPF
subsystem multi-line comment style?

> + * 4: probe saw expected magic value
> + */
> +static __always_inline void bump(u32 idx)
> +{

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

Re: [PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
> new file mode 100644
> index 000000000000..13158d74cbc1
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhtab_kptr.c
> @@ -0,0 +1,146 @@

[ ... ]

> +	skel = rhtab_kptr__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open_and_load"))
> +		return;
> +
> +	/* Create the element and stash a referenced task kptr in it. */
> +	if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.rhtab),
> +					   &key, &zero, BPF_ANY), "create_elem"))
> +		goto out;

Does this code pass the correct value size to bpf_map_update_elem()?

The map value type is struct val_t which contains an 8-byte kptr and a
u32 magic field, making sizeof(struct val_t) == 16 bytes (with 4 bytes
of trailing padding). The code passes &zero where zero is a single __u64
(8 bytes).

kernel/bpf/syscall.c:map_update_elem() copies the full 16 bytes from
&zero via kvmemdup_bpfptr(), reading 8 bytes past the end of the stack
variable. This has two consequences: the test performs an out-of-bounds
read of its own stack, and the element's magic field gets initialised
with whatever stack garbage followed zero.

The same undersized buffer is used again for the recreate_elem call
further down. Would declaring a correctly sized buffer fix this?

> +	if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.init_elem),
> +					      &topts), "test_run_init") ||
> +	    !ASSERT_EQ(topts.retval, 0, "init_ret"))
> +		goto out;
> +
> +	pmu_fd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, 0);
> +	if (pmu_fd >= 0) {
> +		skel->links.nmi_update = bpf_program__attach_perf_event(skel->progs.nmi_update,
> +									pmu_fd);
> +		if (!ASSERT_OK_PTR(skel->links.nmi_update, "attach_perf_event")) {
> +			close(pmu_fd);
> +			goto out;
> +		}
> +
> +		/* Let the NMI handler overwrite the element, and make sure it
> +		 * actually ran before probing (otherwise the probe would pass
> +		 * vacuously even on an unfixed kernel).
> +		 */

This isn't a bug, but the opening /* of a multi-line comment should sit
on its own line per BPF subsystem style. The file's other block comments
already follow that form.

> +		for (i = 0; i < 20 && read_counter(skel, 1) == 0; i++)
> +			usleep(100000);
> +		ASSERT_GT(read_counter(skel, 1), 0, "nmi_update_ran");

Can this assertion fail for environmental reasons on an otherwise correct
kernel?

The perf event is opened with pid == -1 and cpu == 0, so it only samples
CPU 0. PERF_COUNT_HW_CPU_CYCLES does not advance while a core is in a
deep C-state, and the polling thread is not pinned to CPU 0. On an
otherwise idle host the counter can stay at 0 and the test hard-fails
here, even though nothing is wrong with the kernel.

Contrast lru_lock_nmi.c, which makes the same assertion safe by attaching
on every CPU and running busy threads pinned per CPU during the sampling
window.

> +
> +		bpf_link__destroy(skel->links.nmi_update);
> +		skel->links.nmi_update = NULL;
> +		close(pmu_fd);

Does this code close pmu_fd twice?

bpf_program__attach_perf_event() takes ownership of the perf fd on
success: libbpf stores it in bpf_link_perf::perf_event_fd and
bpf_link_perf_detach() closes it. So bpf_link__destroy() above already
closed pmu_fd, and this explicit close() closes an fd number that is no
longer owned by the test.

The file gets the ownership rule right on the failure path: it closes
pmu_fd only because the attach failed. The neighbouring NMI test
lru_lock_nmi.c documents it explicitly at lines 203-206. Today nothing
allocates a descriptor between the destroy and close so the stray close()
just returns EBADF, but a double close in a test that later grows an
intervening open()/socket()/map creation will silently close an unrelated
descriptor.

[ ... ]

> +		ASSERT_EQ(read_counter(skel, 2), 1, "xchg_non_null");
> +		ASSERT_EQ(read_counter(skel, 3), 0, "xchg_null");
> +	} else {
> +		test__skip();
> +	}

Does calling test__skip() in a test that defines no subtests make the
whole test report SKIP even though the second phase still runs?

test_progs.c:print_test_result() reads the subtests_cnt and with
subtest_num == 0, a single test__skip() turns the entire test into
'#N rhtab_kptr:SKIP'. On the common BPF CI setup (QEMU without a vPMU)
perf_event_open() always fails, so this test would permanently print SKIP
while silently exercising the delete/re-insert recycle path - a failure
there is still reported, but a pass is indistinguishable from nothing ran.

The neighbouring lru_lock_nmi.c wraps each phase in test__start_subtest()
so that skips and passes are accounted per phase.

A subsystem pattern flags this as potentially concerning: the NMI phase is
skipped with a bare test__skip() that records no reason, so a reader of
the CI log cannot tell whether the PMU was absent, perf_event_paranoid
blocked the open, or something else failed. errno from the failed
perf_event_open() is discarded. The guide warns about tests that skip
silently with no message, though test__skip() takes no reason argument and
this is the dominant idiom across prog_tests/.

[ ... ]

> +	for (i = 0; i < 2000; i++) {
> +		topts.retval = 0;
> +		err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.init_elem),
> +					     &topts);
> +		if (err || topts.retval) {
> +			/* Element may be gone; recreate and retry once. */
> +			if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.rhtab),
> +							   &key, &zero, BPF_ANY),
> +				       "recreate_elem"))
> +				goto out;

Same value-size issue here as the earlier create_elem call.

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/rhtab_kptr.c b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
> new file mode 100644
> index 000000000000..fd6bd63cb405
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/rhtab_kptr.c
> @@ -0,0 +1,132 @@

[ ... ]

> +} counters SEC(".maps");
> +
> +/* 0: init ok, 1: nmi update ok, 2: probe xchg non-NULL, 3: probe xchg NULL,
> + * 4: probe saw expected magic value
> + */

This isn't a bug, but the opening /* of a multi-line comment should sit on
its own line. The file's own header comment already uses the correct form,
so this is inconsistent with its own neighbours as well as with the
subsystem style.

> +static __always_inline void bump(u32 idx)
> +{
> +	u64 *v = bpf_map_lookup_elem(&counters, &idx);
> +
> +	if (v)
> +		(*v)++;
> +}

[ ... ]

> +SEC("syscall")
> +int upd_elem(void *ctx)
> +{
> +	struct val_t val = { .magic = 0x52484153 }; /* "RHAS" */
> +	u32 key = 0;
> +
> +	bpf_map_update_elem(&rhtab, &key, &val, BPF_ANY);
> +	return 0;
> +}

Can upd_elem() and del_elem() communicate map operation failures to the
userspace loop?

Both discard the return value of bpf_map_update_elem() and
bpf_map_delete_elem() and unconditionally return 0. The userspace loop
only checks bpf_prog_test_run_opts() and never inspects topts.retval for
these two programs.

rhtab_map_update_elem() has several real failure returns reachable from
this loop - -ENOMEM from bpf_mem_cache_alloc(), -EBUSY when
rhtab->freeing_internal is set, and -E2BIG/insert errors - and
rhtab_map_delete_elem() returns -ENOENT. If any of them fires, the
following probe_elem() returns 1 early, so neither counter 2/3 nor counter
4 is bumped, and the only symptom is the final ASSERT_EQ for
recycle_magic_roundtrip reporting an off-by-N count with no indication of
which operation failed or why.

The sibling programs init_elem()/probe_elem() already use the return value
as a status code, so propagating the map-op error here would turn a silent
count mismatch into a diagnosable failure.


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

[PATCH 4/4] selftests/bpf: Test rhtab special-field combinations

[chenyuan_fl]

From: Yuan Chen <chenyuan@kylinos.cn>

BPF_MAP_TYPE_RHASH allows spin locks, timers, workqueues, task_work,
kptrs (referenced, untrusted, per-cpu) and refcounts in map values.
The recycle fix only changes kptr slot handling, so verify each field
combination end to end:

  * lock_kptr: bpf_spin_lock + referenced kptr + plain data in one
    value. BPF_F_LOCK syscall updates/lookups must work before and
    after many delete/re-insert recycle cycles, the referenced kptr
    must be inherited on recycled elements (zeroing it would leak the
    reference), and the plain bytes must round-trip every iteration.
  * timer: arm a bpf_timer and verify it fires, delete the element and
    verify the timer is cancelled, then re-insert (possibly recycling
    the freed element) and arm a fresh timer again.
  * kptr_untrusted: the untrusted kptr must survive the recycle like a
    referenced one.
  * kptr_percpu: the per-cpu kptr reference must survive the recycle
    (zeroing it would leak the reference).

On the unfixed kernel the three kptr subtests fail at the recycle
assertions while the lock and timer paths still pass, isolating the
behavior change to kptr slots only.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 .../selftests/bpf/prog_tests/rhtab_fields.c   | 213 ++++++++++++
 .../selftests/bpf/progs/rhtab_fields.c        | 305 ++++++++++++++++++
 2 files changed, 518 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhtab_fields.c

diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
new file mode 100644
index 000000000000..29de05bcbd4b
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+#include <unistd.h>
+#include <test_progs.h>
+#include "rhtab_fields.skel.h"
+
+#define RECYCLE_LOOPS 2000
+#define LK_MAGIC 0x52484142
+
+/* Userspace view of the BPF value types (layouts must match the progs). */
+struct lock_kptr_val_user {
+	__u32 lock;
+	__u32 pad;
+	__u64 tsk;
+	__u32 magic;
+	__u32 pad2;
+};
+
+static __u64 read_counter(struct rhtab_fields *skel, u32 idx)
+{
+	__u64 vals[libbpf_num_possible_cpus()];
+	__u64 sum = 0;
+	int i, err;
+
+	err = bpf_map_lookup_elem(bpf_map__fd(skel->maps.counters), &idx, vals);
+	if (!ASSERT_OK(err, "lookup_counter"))
+		return 0;
+	for (i = 0; i < libbpf_num_possible_cpus(); i++)
+		sum += vals[i];
+	return sum;
+}
+
+/* Returns the program retval; asserts the test_run itself succeeded. */
+static int run_prog(struct rhtab_fields *skel, const char *name)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	struct bpf_program *prog;
+	int err;
+
+	prog = bpf_object__find_program_by_name(skel->obj, name);
+	if (!ASSERT_OK_PTR(prog, name))
+		return -1;
+	err = bpf_prog_test_run_opts(bpf_program__fd(prog), &topts);
+	if (!ASSERT_OK(err, name))
+		return -1;
+	return topts.retval;
+}
+
+static void recycle_loop(struct rhtab_fields *skel, int map_fd,
+			 const char *init, const char *del,
+			 const char *upd, const char *probe)
+{
+	u64 zero = 0;
+	u32 key = 0;
+	int i;
+
+	for (i = 0; i < RECYCLE_LOOPS; i++) {
+		if (run_prog(skel, init) != 0) {
+			/* Element may be gone; recreate and retry once. */
+			if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &zero, BPF_ANY),
+				       "recreate_elem"))
+				return;
+			if (!ASSERT_OK(run_prog(skel, init), init))
+				return;
+		}
+		if (!ASSERT_OK(run_prog(skel, del), del))
+			return;
+		if (!ASSERT_OK(run_prog(skel, upd), upd))
+			return;
+		if (!ASSERT_OK(run_prog(skel, probe), probe))
+			return;
+	}
+}
+
+static void subtest_lock_kptr(struct rhtab_fields *skel)
+{
+	struct lock_kptr_val_user val = {};
+	struct lock_kptr_val_user out = {};
+	u64 nonnull_before;
+	u32 key = 0;
+	int map_fd;
+
+	map_fd = bpf_map__fd(skel->maps.lkmap);
+
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &val, BPF_ANY),
+		       "create_elem"))
+		return;
+
+	/* Spin lock must be usable from the syscall path (BPF_F_LOCK). */
+	val.magic = LK_MAGIC;
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &val, BPF_F_LOCK),
+		       "locked_update"))
+		return;
+	if (!ASSERT_OK(bpf_map_lookup_elem_flags(map_fd, &key, &out, BPF_F_LOCK),
+		       "locked_lookup"))
+		return;
+	ASSERT_EQ(out.magic, LK_MAGIC, "locked_lookup_magic");
+
+	/*
+	 * Delete/re-insert recycle cycles: the referenced kptr must be
+	 * inherited on recycled elements (zeroing it would leak the
+	 * reference) and the plain magic bytes must round-trip every time.
+	 */
+	nonnull_before = read_counter(skel, 1);
+	recycle_loop(skel, map_fd, "lk_init", "lk_del", "lk_upd", "lk_probe");
+	ASSERT_GT(read_counter(skel, 1), nonnull_before, "recycle_xchg_non_null");
+	ASSERT_EQ(read_counter(skel, 3), RECYCLE_LOOPS, "recycle_magic_roundtrip");
+
+	/* The spin lock must still work after many recycles. */
+	val.magic = LK_MAGIC + 1;
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &val, BPF_F_LOCK),
+		       "post_recycle_locked_update"))
+		return;
+	memset(&out, 0, sizeof(out));
+	if (!ASSERT_OK(bpf_map_lookup_elem_flags(map_fd, &key, &out, BPF_F_LOCK),
+		       "post_recycle_locked_lookup"))
+		return;
+	ASSERT_EQ(out.magic, LK_MAGIC + 1, "post_recycle_locked_magic");
+}
+
+static void subtest_timer(struct rhtab_fields *skel)
+{
+	u64 zero = 0;
+	u32 key = 0;
+	int fired, map_fd;
+
+	map_fd = bpf_map__fd(skel->maps.tmap);
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &zero, BPF_ANY),
+		       "create_elem"))
+		return;
+
+	if (!ASSERT_OK(run_prog(skel, "arm_timer"), "arm_timer_first"))
+		return;
+	usleep(300000);
+	if (!ASSERT_GT(skel->bss->timer_fired, 0, "timer_fired_first"))
+		return;
+
+	/* Deleting the element must cancel the timer. */
+	fired = skel->bss->timer_fired;
+	if (!ASSERT_OK(bpf_map_delete_elem(map_fd, &key), "delete_elem"))
+		return;
+	usleep(300000);
+	ASSERT_EQ(skel->bss->timer_fired, fired, "timer_cancelled_after_delete");
+
+	/*
+	 * Re-insert (may recycle the freed element): the timer field must be
+	 * re-initialized so a fresh timer can be armed again.
+	 */
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &zero, BPF_ANY),
+		       "recreate_elem"))
+		return;
+	if (!ASSERT_OK(run_prog(skel, "arm_timer"), "arm_timer_second"))
+		return;
+	usleep(300000);
+	ASSERT_GT(skel->bss->timer_fired, fired, "timer_fired_second");
+}
+
+static void subtest_kptr_untrusted(struct rhtab_fields *skel)
+{
+	u64 nonnull_before;
+	u64 zero = 0;
+	u32 key = 0;
+	int map_fd;
+
+	map_fd = bpf_map__fd(skel->maps.umap);
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &zero, BPF_ANY),
+		       "create_elem"))
+		return;
+
+	/* The untrusted kptr must survive the recycle like a referenced one. */
+	nonnull_before = read_counter(skel, 5);
+	recycle_loop(skel, map_fd, "u_init", "u_del", "u_upd", "u_probe");
+	ASSERT_GT(read_counter(skel, 5), nonnull_before, "recycle_unref_non_null");
+}
+
+static void subtest_kptr_percpu(struct rhtab_fields *skel)
+{
+	u64 nonnull_before;
+	u64 zero = 0;
+	u32 key = 0;
+	int map_fd;
+
+	map_fd = bpf_map__fd(skel->maps.pcmap);
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &zero, BPF_ANY),
+		       "create_elem"))
+		return;
+
+	/* The per-cpu kptr reference must survive the recycle (no leak). */
+	nonnull_before = read_counter(skel, 7);
+	recycle_loop(skel, map_fd, "pc_init", "pc_del", "pc_upd", "pc_probe");
+	ASSERT_GT(read_counter(skel, 7), nonnull_before, "recycle_pcpu_non_null");
+}
+
+void test_rhtab_fields(void)
+{
+	struct rhtab_fields *skel;
+
+	skel = rhtab_fields__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+
+	if (test__start_subtest("lock_kptr"))
+		subtest_lock_kptr(skel);
+	if (test__start_subtest("timer"))
+		subtest_timer(skel);
+	if (test__start_subtest("kptr_untrusted"))
+		subtest_kptr_untrusted(skel);
+	if (test__start_subtest("kptr_percpu"))
+		subtest_kptr_percpu(skel);
+
+	rhtab_fields__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/rhtab_fields.c b/tools/testing/selftests/bpf/progs/rhtab_fields.c
new file mode 100644
index 000000000000..85335f19f172
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/rhtab_fields.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Co., Ltd. */
+
+/*
+ * Combined special-field tests for BPF_MAP_TYPE_RHASH. Each map carries a
+ * different field combination and is exercised through delete/re-insert
+ * cycles so the bpf memory allocator recycles element memory:
+ *
+ *  1. lkmap: bpf_spin_lock + referenced kptr + plain data in one value.
+ *     After every recycle the spin lock must still be usable (initialized by
+ *     the alloc path), the referenced kptr must be inherited instead of
+ *     zeroed (zeroing would leak the reference), and the plain bytes must
+ *     round-trip.
+ *  2. tmap: bpf_timer. The delete path must cancel the timer, and a recycled
+ *     element must be able to arm a fresh timer again.
+ *  3. umap: untrusted (unreferenced) kptr. The inherited pointer must be
+ *     preserved on recycle, matching hash map behavior.
+ *  4. pcmap: per-cpu kptr. Like the referenced kptr, the per-cpu reference
+ *     must not be dropped on recycle.
+ */
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_experimental.h"
+
+char LICENSE[] SEC("license") = "GPL";
+
+struct lock_kptr_val {
+	struct bpf_spin_lock lock;
+	struct task_struct __kptr * tsk;
+	__u32 magic;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct lock_kptr_val);
+} lkmap SEC(".maps");
+
+struct timer_val {
+	struct bpf_timer timer;
+	__u64 data;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct timer_val);
+} tmap SEC(".maps");
+
+struct unref_val {
+	struct task_struct __kptr_untrusted * tsk;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct unref_val);
+} umap SEC(".maps");
+
+struct pcval {
+	__u64 v;
+};
+
+struct pcpu_val {
+	struct pcval __percpu_kptr * pc;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(max_entries, 16);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, struct pcpu_val);
+} pcmap SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(max_entries, 9);
+	__type(key, __u32);
+	__type(value, __u64);
+} counters SEC(".maps");
+
+/* 0: lk init ok, 1: lk probe xchg non-NULL, 2: lk probe xchg NULL,
+ * 3: lk probe magic ok, 4: u init ok, 5: u probe ptr non-NULL,
+ * 6: pc init ok, 7: pc probe xchg non-NULL, 8: pc probe xchg NULL
+ */
+static __always_inline void bump(u32 idx)
+{
+	u64 *v = bpf_map_lookup_elem(&counters, &idx);
+
+	if (v)
+		(*v)++;
+}
+
+extern struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+extern void bpf_task_release(struct task_struct *p) __ksym;
+
+int timer_fired;
+
+/* Map 1: spin lock + referenced kptr + plain data. */
+
+SEC("syscall")
+int lk_init(void *ctx)
+{
+	struct lock_kptr_val *val;
+	struct task_struct *task, *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&lkmap, &key);
+	if (!val)
+		return 1;
+	task = bpf_task_acquire(bpf_get_current_task_btf());
+	if (!task)
+		return 2;
+	old = bpf_kptr_xchg(&val->tsk, task);
+	if (old)
+		bpf_task_release(old);
+	bump(0);
+	return 0;
+}
+
+SEC("syscall")
+int lk_del(void *ctx)
+{
+	u32 key = 0;
+
+	bpf_map_delete_elem(&lkmap, &key);
+	return 0;
+}
+
+SEC("syscall")
+int lk_upd(void *ctx)
+{
+	struct lock_kptr_val val = { .magic = 0x52484142 };
+	u32 key = 0;
+
+	bpf_map_update_elem(&lkmap, &key, &val, BPF_ANY);
+	return 0;
+}
+
+SEC("syscall")
+int lk_probe(void *ctx)
+{
+	struct lock_kptr_val *val;
+	struct task_struct *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&lkmap, &key);
+	if (!val)
+		return 1;
+	old = bpf_kptr_xchg(&val->tsk, NULL);
+	if (old) {
+		bpf_task_release(old);
+		bump(1);
+	} else {
+		bump(2);
+	}
+	if (val->magic == 0x52484142)
+		bump(3);
+	return 0;
+}
+
+/* Map 2: bpf_timer. */
+
+static int timer_cb(void *map, void *key, struct timer_val *value)
+{
+	timer_fired++;
+	return 0;
+}
+
+SEC("syscall")
+int arm_timer(void *ctx)
+{
+	struct timer_val *val;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&tmap, &key);
+	if (!val)
+		return 1;
+	/* 1 == CLOCK_MONOTONIC */
+	if (bpf_timer_init(&val->timer, &tmap, 1))
+		return 2;
+	bpf_timer_set_callback(&val->timer, timer_cb);
+	if (bpf_timer_start(&val->timer, 50000, 0))
+		return 3;
+	return 0;
+}
+
+/* Map 3: untrusted kptr. */
+
+SEC("syscall")
+int u_init(void *ctx)
+{
+	struct unref_val *val;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&umap, &key);
+	if (!val)
+		return 1;
+	val->tsk = bpf_get_current_task_btf();
+	bump(4);
+	return 0;
+}
+
+SEC("syscall")
+int u_del(void *ctx)
+{
+	u32 key = 0;
+
+	bpf_map_delete_elem(&umap, &key);
+	return 0;
+}
+
+SEC("syscall")
+int u_upd(void *ctx)
+{
+	struct unref_val val = {};
+	u32 key = 0;
+
+	bpf_map_update_elem(&umap, &key, &val, BPF_ANY);
+	return 0;
+}
+
+SEC("syscall")
+int u_probe(void *ctx)
+{
+	struct unref_val *val;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&umap, &key);
+	if (!val)
+		return 1;
+	if (val->tsk)
+		bump(5);
+	val->tsk = NULL;
+	return 0;
+}
+
+/* Map 4: per-cpu kptr. */
+
+SEC("syscall")
+int pc_init(void *ctx)
+{
+	struct pcpu_val *val;
+	struct pcval *p, *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&pcmap, &key);
+	if (!val)
+		return 1;
+	p = bpf_percpu_obj_new(struct pcval);
+	if (!p)
+		return 2;
+	old = bpf_kptr_xchg(&val->pc, p);
+	if (old)
+		bpf_percpu_obj_drop(old);
+	bump(6);
+	return 0;
+}
+
+SEC("syscall")
+int pc_del(void *ctx)
+{
+	u32 key = 0;
+
+	bpf_map_delete_elem(&pcmap, &key);
+	return 0;
+}
+
+SEC("syscall")
+int pc_upd(void *ctx)
+{
+	struct pcpu_val val = {};
+	u32 key = 0;
+
+	bpf_map_update_elem(&pcmap, &key, &val, BPF_ANY);
+	return 0;
+}
+
+SEC("syscall")
+int pc_probe(void *ctx)
+{
+	struct pcpu_val *val;
+	struct pcval *old;
+	u32 key = 0;
+
+	val = bpf_map_lookup_elem(&pcmap, &key);
+	if (!val)
+		return 1;
+	old = bpf_kptr_xchg(&val->pc, NULL);
+	if (old) {
+		bpf_percpu_obj_drop(old);
+		bump(7);
+	} else {
+		bump(8);
+	}
+	return 0;
+}
-- 
2.54.0

Re: [PATCH 4/4] selftests/bpf: Test rhtab special-field combinations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Passing an undersized buffer to `bpf_map_update_elem` causes an out-of-bounds read on the userspace stack.
- [Medium] Missing file descriptor validation after manual BPF program lookup.
- [Medium] The timer cancellation test is ineffective because it attempts to cancel a one-shot timer that has already expired.
- [Low] Multi-line comment does not follow the mandatory BPF subsystem style.
--

--- Patch [4]: [PATCH 4/4] selftests/bpf: Test rhtab special-field combinations ---
commit 97a820b266ff0381e757c9ee521aee4aca7caef2
Author: Yuan Chen <chenyuan@kylinos.cn>

selftests/bpf: Test rhtab special-field combinations

This commit adds selftests for BPF_MAP_TYPE_RHASH special-field combinations,
verifying the recycling of memory for various field types like spin locks,
timers, and kptrs.

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
> new file mode 100644
> index 0000000000000..29de05bcbd4b0
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c

[ ... ]

> @@ -28,34 +28,45 @@ static int run_prog(struct rhtab_fields *skel, const char *name)
>  {
>  	LIBBPF_OPTS(bpf_test_run_opts, topts);
>  	struct bpf_program *prog;
>  	int err;
>  
>  	prog = bpf_object__find_program_by_name(skel->obj, name);
>  	if (!ASSERT_OK_PTR(prog, name))
>  		return -1;
>  	err = bpf_prog_test_run_opts(bpf_program__fd(prog), &topts);

[Severity: Medium]
Is it necessary to validate the file descriptor returned by bpf_program__fd()
before using it? According to BPF selftest patterns, file descriptors from
manual lookups should be verified (>= 0) before use, as they can return a
negative value if the program is invalid.

>  	if (!ASSERT_OK(err, name))
>  		return -1;
>  	return topts.retval;
>  }
>  
>  static void recycle_loop(struct rhtab_fields *skel, int map_fd,
>  			 const char *init, const char *del,
>  			 const char *upd, const char *probe)
>  {
>  	u64 zero = 0;
>  	u32 key = 0;
>  	int i;
>  
>  	for (i = 0; i < RECYCLE_LOOPS; i++) {
>  		if (run_prog(skel, init) != 0) {
>  			/* Element may be gone; recreate and retry once. */
>  			if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &zero, BPF_ANY),

[Severity: Medium]
Could this result in an out-of-bounds read on the userspace stack? The zero
variable is 8 bytes, but this function is called for maps like lkmap which
expects a 24-byte value structure. bpf_map_update_elem() will read
map->value_size bytes from this pointer.

>  				       "recreate_elem"))
>  				return;

[ ... ]

> @@ -119,25 +130,24 @@ static void subtest_timer(struct rhtab_fields *skel)
>  {
>  	u64 zero = 0;
>  	u32 key = 0;
>  	int fired, map_fd;
>  
>  	map_fd = bpf_map__fd(skel->maps.tmap);
>  	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &zero, BPF_ANY),

[Severity: Medium]
Could this also lead to an out-of-bounds read? The tmap map value
(struct timer_val) is 24 bytes in size, but it's being updated from the
8-byte zero variable.

>  		       "create_elem"))
>  		return;
>  
>  	if (!ASSERT_OK(run_prog(skel, "arm_timer"), "arm_timer_first"))
>  		return;
>  	usleep(300000);
>  	if (!ASSERT_GT(skel->bss->timer_fired, 0, "timer_fired_first"))
>  		return;
>  
>  	/* Deleting the element must cancel the timer. */
>  	fired = skel->bss->timer_fired;
>  	if (!ASSERT_OK(bpf_map_delete_elem(map_fd, &key), "delete_elem"))
>  		return;
>  	usleep(300000);
>  	ASSERT_EQ(skel->bss->timer_fired, fired, "timer_cancelled_after_delete");

[Severity: Medium]
Does this assertion actually test the cancellation of the timer? Since
arm_timer() sets a 50us one-shot timer and this code sleeps for 300ms
before calling bpf_map_delete_elem(), the timer has already fired by the
time the element is deleted.

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/rhtab_fields.c b/tools/testing/selftests/bpf/progs/rhtab_fields.c
> new file mode 100644
> index 0000000000000..85335f19f1722
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/rhtab_fields.c

[ ... ]

> @@ -89,20 +89,20 @@ struct {
>  	__uint(max_entries, 9);
>  	__type(key, __u32);
>  	__type(value, __u64);
>  } counters SEC(".maps");
>  
> +/* 0: lk init ok, 1: lk probe xchg non-NULL, 2: lk probe xchg NULL,
> + * 3: lk probe magic ok, 4: u init ok, 5: u probe ptr non-NULL,
> + * 6: pc init ok, 7: pc probe xchg non-NULL, 8: pc probe xchg NULL
> + */

[Severity: Low]
This isn't a bug, but does this multi-line comment format violate the
BPF subsystem style? BPF subsystem multi-line comments should have the opening
/* on its own line.

>  static __always_inline void bump(u32 idx)
>  {
>  	u64 *v = bpf_map_lookup_elem(&counters, &idx);
>  
>  	if (v)
>  		(*v)++;
>  }

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

Re: [PATCH 4/4] selftests/bpf: Test rhtab special-field combinations

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c
> new file mode 100644
> index 000000000000..29de05bcbd4b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhtab_fields.c

[ ... ]

> +static void subtest_timer(struct rhtab_fields *skel)
> +{
> +	u64 zero = 0;
> +	u32 key = 0;
> +	int fired, map_fd;
> +
> +	map_fd = bpf_map__fd(skel->maps.tmap);
> +	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &zero, BPF_ANY),
> +		       "create_elem"))
> +		return;

Does this read past the end of the zero variable?  The tmap value type
is struct timer_val which is 24 bytes (16-byte bpf_timer plus 8-byte
data field), but zero is only 8 bytes.  The syscall copies value_size
bytes from the user pointer in kernel/bpf/syscall.c:

    value_size = bpf_map_value_size(map);
    value = kvmemdup_bpfptr(uvalue, value_size);

so it appears 16 bytes above zero on the stack would be read.  The same
pattern is also used later when recreating the element.

> +
> +	if (!ASSERT_OK(run_prog(skel, "arm_timer"), "arm_timer_first"))
> +		return;
> +	usleep(300000);
> +	if (!ASSERT_GT(skel->bss->timer_fired, 0, "timer_fired_first"))
> +		return;
> +
> +	/* Deleting the element must cancel the timer. */
> +	fired = skel->bss->timer_fired;
> +	if (!ASSERT_OK(bpf_map_delete_elem(map_fd, &key), "delete_elem"))
> +		return;
> +	usleep(300000);
> +	ASSERT_EQ(skel->bss->timer_fired, fired, "timer_cancelled_after_delete");

Can this assertion actually verify timer cancellation?  Looking at
arm_timer() in progs/rhtab_fields.c, it arms the timer with a 50us
expiry (50000ns), and the callback doesn't re-arm.  The test waits
300ms before the delete, and the preceding ASSERT_GT confirms the timer
has already fired.  So by the time bpf_map_delete_elem() runs, the
timer has been inactive for ~300ms.  Whether or not the delete path
cancels the timer field, timer_fired cannot advance during the second
sleep, so the assertion would pass even on a kernel that doesn't cancel
the timer at all.

The commit message says "delete the element and verify the timer is
cancelled" - to actually test cancellation, wouldn't the timer need to
still be pending at delete time?  For example, arming it with a delay
longer than the deletion (say 200ms), deleting immediately, then
sleeping past the original expiry and checking that timer_fired did not
change.

> diff --git a/tools/testing/selftests/bpf/progs/rhtab_fields.c b/tools/testing/selftests/bpf/progs/rhtab_fields.c
> new file mode 100644
> index 000000000000..85335f19f172
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/rhtab_fields.c

[ ... ]

> +struct timer_val {
> +	struct bpf_timer timer;
> +	__u64 data;
> +};

Is the data field used anywhere?  I don't see it read or written in
any of the programs or in the test harness.  Compare struct
lock_kptr_val::magic which exists for a similar purpose (plain bytes
alongside a special field) and is actually exercised by lk_upd/lk_probe
to verify plain bytes round-trip through recycled elements.

The field does affect the value_size (making it 24 bytes instead of
16), which creates a mismatch with prog_tests/rhtab_fields.c where
subtest_timer() seeds the map from an 8-byte local (u64 zero).

> +} counters SEC(".maps");
> +
> +/* 0: lk init ok, 1: lk probe xchg non-NULL, 2: lk probe xchg NULL,
> + * 3: lk probe magic ok, 4: u init ok, 5: u probe ptr non-NULL,
> + * 6: pc init ok, 7: pc probe xchg non-NULL, 8: pc probe xchg NULL
> + */

This isn't a bug, but the comment style here differs from the file
header above.  The BPF subsystem prefers the opening /* on its own
line:

    /*
     * 0: lk init ok, 1: lk probe xchg non-NULL, ...
     */

> +static __always_inline void bump(u32 idx)
> +{
> +	u64 *v = bpf_map_lookup_elem(&counters, &idx);
> +
> +	if (v)
> +		(*v)++;
> +}
> +
> +extern struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
> +extern void bpf_task_release(struct task_struct *p) __ksym;
> +
> +int timer_fired;
> +
> +/* Map 1: spin lock + referenced kptr + plain data. */
> +
> +SEC("syscall")
> +int lk_init(void *ctx)
> +{
> +	struct lock_kptr_val *val;
> +	struct task_struct *task, *old;
> +	u32 key = 0;
> +
> +	val = bpf_map_lookup_elem(&lkmap, &key);
> +	if (!val)
> +		return 1;
> +	task = bpf_task_acquire(bpf_get_current_task_btf());
> +	if (!task)
> +		return 2;
> +	old = bpf_kptr_xchg(&val->tsk, task);
> +	if (old)
> +		bpf_task_release(old);
> +	bump(0);
> +	return 0;
> +}
> +
> +SEC("syscall")
> +int lk_del(void *ctx)
> +{
> +	u64 key = 0;
> +
> +	bpf_map_delete_elem(&lkmap, &key);
> +	return 0;
> +}
> +
> +SEC("syscall")
> +int lk_upd(void *ctx)
> +{
> +	struct lock_kptr_val val = { .magic = 0x52484142 };
> +	u32 key = 0;
> +
> +	bpf_map_update_elem(&lkmap, &key, &val, BPF_ANY);
> +	return 0;
> +}
> +
> +SEC("syscall")
> +int lk_probe(void *ctx)
> +{
> +	struct lock_kptr_val *val;
> +	struct task_struct *old;
> +	u32 key = 0;
> +
> +	val = bpf_map_lookup_elem(&lkmap, &key);
> +	if (!val)
> +		return 1;
> +	old = bpf_kptr_xchg(&val->tsk, NULL);
> +	if (old) {
> +		bpf_task_release(old);
> +		bump(1);
> +	} else {
> +		bump(2);
> +	}
> +	if (val->magic == 0x52484142)
> +		bump(3);
> +	return 0;
> +}

A subsystem pattern flags this as potentially concerning: the
lk_init/lk_del/lk_upd/lk_probe quartet appears to re-implement a test
that the immediately preceding commit in this same series (81f409ba449d)
just added to the same directory.  progs/rhtab_kptr.c already covers
the rhtab referenced-kptr recycle path with an identical four-program
template over the same map type.

The shared scaffolding is duplicated as well: bump() and the extern
bpf_task_acquire/bpf_task_release declarations are byte-identical
between the two files.

Given that rhtab_fields.c adds the spin-lock, timer, untrusted-kptr,
and per-cpu-kptr combinations that rhtab_kptr.c doesn't cover, while
rhtab_kptr.c covers the NMI (perf_event) update path that
rhtab_fields.c doesn't, neither file strictly subsumes the other.
Should the two files share a header for the common helpers, or would it
make sense to merge them into one test that covers all field
combinations including the NMI case?

> +
> +/* Map 2: bpf_timer. */
> +
> +static int timer_cb(void *map, void *key, struct timer_val *value)
> +{
> +	timer_fired++;
> +	return 0;
> +}
> +
> +SEC("syscall")
> +int arm_timer(void *ctx)
> +{
> +	struct timer_val *val;
> +	u32 key = 0;
> +
> +	val = bpf_map_lookup_elem(&tmap, &key);
> +	if (!val)
> +		return 1;
> +	/* 1 == CLOCK_MONOTONIC */
> +	if (bpf_timer_init(&val->timer, &tmap, 1))
> +		return 2;
> +	bpf_timer_set_callback(&val->timer, timer_cb);
> +	if (bpf_timer_start(&val->timer, 50000, 0))
> +		return 3;
> +	return 0;
> +}

Does arming a one-shot timer with a 50us expiry make the timer subtest's
delete-cancellation assertion unfalsifiable?  The callback doesn't
re-arm, so the timer fires exactly once.  The consumer in
prog_tests/rhtab_fields.c waits 300ms (6000x the expiry) and confirms
the timer has already fired before calling bpf_map_delete_elem().  The
hrtimer is therefore inactive at delete time, so timer_fired cannot
change during the second sleep regardless of whether the delete path
actually cancels anything.

The existing convention in this directory is to arm a long timer so
it is still pending at delete time - progs/timer_start_delete_race.c
uses bpf_timer_start(&value->timer, 100000000, 0) (100ms) for exactly
this delete-vs-pending-timer scenario, and progs/timer.c uses
1ull << 35 (~34s) as its 'must not fire' expiry.  Would arming with an
expiry longer than the arm-to-delete window make the assertion able to
fail?

[ ... ]


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