* [PATCH bpf-next v2 0/4] bpf: Cancel special fields in resizable hashtab on recycle
[not found] <DKM6K95EN9OF.3O9XNYWVLYHDE@gmail.com>
@ 2026-08-24 14:36 ` chenyuan_fl
2026-08-24 14:36 ` [PATCH 1/4] " chenyuan_fl
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: chenyuan_fl @ 2026-08-24 14:36 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle
2026-08-24 14:36 ` [PATCH bpf-next v2 0/4] bpf: Cancel special fields in resizable hashtab on recycle chenyuan_fl
@ 2026-08-24 14:36 ` chenyuan_fl
2026-08-24 15:42 ` bot+bpf-ci
2026-08-24 16:15 ` Mykyta Yatsenko
2026-08-24 14:36 ` [PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor chenyuan_fl
` (2 subsequent siblings)
3 siblings, 2 replies; 20+ messages in thread
From: chenyuan_fl @ 2026-08-24 14:36 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor
2026-08-24 14:36 ` [PATCH bpf-next v2 0/4] bpf: Cancel special fields in resizable hashtab on recycle chenyuan_fl
2026-08-24 14:36 ` [PATCH 1/4] " chenyuan_fl
@ 2026-08-24 14:36 ` chenyuan_fl
2026-08-24 15:42 ` bot+bpf-ci
2026-08-24 14:36 ` [PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context chenyuan_fl
2026-08-24 14:36 ` [PATCH 4/4] selftests/bpf: Test rhtab special-field combinations chenyuan_fl
3 siblings, 1 reply; 20+ messages in thread
From: chenyuan_fl @ 2026-08-24 14:36 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context
2026-08-24 14:36 ` [PATCH bpf-next v2 0/4] bpf: Cancel special fields in resizable hashtab on recycle chenyuan_fl
2026-08-24 14:36 ` [PATCH 1/4] " chenyuan_fl
2026-08-24 14:36 ` [PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor chenyuan_fl
@ 2026-08-24 14:36 ` chenyuan_fl
2026-08-24 15:42 ` bot+bpf-ci
2026-08-24 14:36 ` [PATCH 4/4] selftests/bpf: Test rhtab special-field combinations chenyuan_fl
3 siblings, 1 reply; 20+ messages in thread
From: chenyuan_fl @ 2026-08-24 14:36 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/4] selftests/bpf: Test rhtab special-field combinations
2026-08-24 14:36 ` [PATCH bpf-next v2 0/4] bpf: Cancel special fields in resizable hashtab on recycle chenyuan_fl
` (2 preceding siblings ...)
2026-08-24 14:36 ` [PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context chenyuan_fl
@ 2026-08-24 14:36 ` chenyuan_fl
2026-08-24 15:42 ` bot+bpf-ci
3 siblings, 1 reply; 20+ messages in thread
From: chenyuan_fl @ 2026-08-24 14:36 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor
2026-08-24 14:36 ` [PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor chenyuan_fl
@ 2026-08-24 15:42 ` bot+bpf-ci
0 siblings, 0 replies; 20+ messages in thread
From: bot+bpf-ci @ 2026-08-24 15:42 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, ast, daniel, andrii, eddyz87,
memxor, martin.lau, song, yonghong.song, jolsa, emil,
ihor.solodrai, shuah, gnq25, chenyuan, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- 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, ©->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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle
2026-08-24 14:36 ` [PATCH 1/4] " chenyuan_fl
@ 2026-08-24 15:42 ` bot+bpf-ci
2026-08-24 16:15 ` Mykyta Yatsenko
1 sibling, 0 replies; 20+ messages in thread
From: bot+bpf-ci @ 2026-08-24 15:42 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, ast, daniel, andrii, eddyz87,
memxor, martin.lau, song, yonghong.song, jolsa, emil,
ihor.solodrai, shuah, gnq25, chenyuan, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context
2026-08-24 14:36 ` [PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context chenyuan_fl
@ 2026-08-24 15:42 ` bot+bpf-ci
0 siblings, 0 replies; 20+ messages in thread
From: bot+bpf-ci @ 2026-08-24 15:42 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, ast, daniel, andrii, eddyz87,
memxor, martin.lau, song, yonghong.song, jolsa, emil,
ihor.solodrai, shuah, gnq25, chenyuan, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] selftests/bpf: Test rhtab special-field combinations
2026-08-24 14:36 ` [PATCH 4/4] selftests/bpf: Test rhtab special-field combinations chenyuan_fl
@ 2026-08-24 15:42 ` bot+bpf-ci
0 siblings, 0 replies; 20+ messages in thread
From: bot+bpf-ci @ 2026-08-24 15:42 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, ast, daniel, andrii, eddyz87,
memxor, martin.lau, song, yonghong.song, jolsa, emil,
ihor.solodrai, shuah, gnq25, chenyuan, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle
2026-08-24 14:36 ` [PATCH 1/4] " chenyuan_fl
2026-08-24 15:42 ` bot+bpf-ci
@ 2026-08-24 16:15 ` Mykyta Yatsenko
2026-09-01 6:28 ` [PATCH bpf-next v3 0/4] " chenyuan_fl
1 sibling, 1 reply; 20+ messages in thread
From: Mykyta Yatsenko @ 2026-08-24 16:15 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
Shuah Khan, Nuoqi Gui, Yuan Chen
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();
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH bpf-next v3 0/4] bpf: Cancel special fields in resizable hashtab on recycle
2026-08-24 16:15 ` Mykyta Yatsenko
@ 2026-09-01 6:28 ` chenyuan_fl
2026-09-01 6:28 ` [PATCH bpf-next v3 1/4] " chenyuan_fl
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: chenyuan_fl @ 2026-09-01 6:28 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Mykyta Yatsenko,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
Emil Tsalapatis, Ihor Solodrai, Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH bpf-next v3 1/4] bpf: Cancel special fields in resizable hashtab on recycle
2026-09-01 6:28 ` [PATCH bpf-next v3 0/4] " chenyuan_fl
@ 2026-09-01 6:28 ` chenyuan_fl
2026-09-01 7:37 ` bot+bpf-ci
2026-09-01 16:57 ` Mykyta Yatsenko
2026-09-01 6:28 ` [PATCH bpf-next v3 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor chenyuan_fl
` (2 subsequent siblings)
3 siblings, 2 replies; 20+ messages in thread
From: chenyuan_fl @ 2026-09-01 6:28 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Mykyta Yatsenko,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
Emil Tsalapatis, Ihor Solodrai, Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH bpf-next v3 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor
2026-09-01 6:28 ` [PATCH bpf-next v3 0/4] " chenyuan_fl
2026-09-01 6:28 ` [PATCH bpf-next v3 1/4] " chenyuan_fl
@ 2026-09-01 6:28 ` chenyuan_fl
2026-09-01 17:10 ` Mykyta Yatsenko
2026-09-01 6:28 ` [PATCH bpf-next v3 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context chenyuan_fl
2026-09-01 6:28 ` [PATCH bpf-next v3 4/4] selftests/bpf: Test rhtab special-field combinations chenyuan_fl
3 siblings, 1 reply; 20+ messages in thread
From: chenyuan_fl @ 2026-09-01 6:28 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Mykyta Yatsenko,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
Emil Tsalapatis, Ihor Solodrai, Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH bpf-next v3 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context
2026-09-01 6:28 ` [PATCH bpf-next v3 0/4] " chenyuan_fl
2026-09-01 6:28 ` [PATCH bpf-next v3 1/4] " chenyuan_fl
2026-09-01 6:28 ` [PATCH bpf-next v3 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor chenyuan_fl
@ 2026-09-01 6:28 ` chenyuan_fl
2026-09-01 7:37 ` bot+bpf-ci
2026-09-01 6:28 ` [PATCH bpf-next v3 4/4] selftests/bpf: Test rhtab special-field combinations chenyuan_fl
3 siblings, 1 reply; 20+ messages in thread
From: chenyuan_fl @ 2026-09-01 6:28 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Mykyta Yatsenko,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
Emil Tsalapatis, Ihor Solodrai, Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH bpf-next v3 4/4] selftests/bpf: Test rhtab special-field combinations
2026-09-01 6:28 ` [PATCH bpf-next v3 0/4] " chenyuan_fl
` (2 preceding siblings ...)
2026-09-01 6:28 ` [PATCH bpf-next v3 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context chenyuan_fl
@ 2026-09-01 6:28 ` chenyuan_fl
2026-09-01 7:37 ` bot+bpf-ci
3 siblings, 1 reply; 20+ messages in thread
From: chenyuan_fl @ 2026-09-01 6:28 UTC (permalink / raw)
To: bpf
Cc: linux-kernel, linux-kselftest, Mykyta Yatsenko,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
Emil Tsalapatis, Ihor Solodrai, Shuah Khan, Nuoqi Gui, Yuan Chen
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH bpf-next v3 1/4] bpf: Cancel special fields in resizable hashtab on recycle
2026-09-01 6:28 ` [PATCH bpf-next v3 1/4] " chenyuan_fl
@ 2026-09-01 7:37 ` bot+bpf-ci
2026-09-01 16:57 ` Mykyta Yatsenko
1 sibling, 0 replies; 20+ messages in thread
From: bot+bpf-ci @ 2026-09-01 7:37 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, mykyta.yatsenko5, ast, daniel,
andrii, eddyz87, memxor, martin.lau, emil, ihor.solodrai, shuah,
gnq25, chenyuan, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH bpf-next v3 4/4] selftests/bpf: Test rhtab special-field combinations
2026-09-01 6:28 ` [PATCH bpf-next v3 4/4] selftests/bpf: Test rhtab special-field combinations chenyuan_fl
@ 2026-09-01 7:37 ` bot+bpf-ci
0 siblings, 0 replies; 20+ messages in thread
From: bot+bpf-ci @ 2026-09-01 7:37 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, mykyta.yatsenko5, ast, daniel,
andrii, eddyz87, memxor, martin.lau, emil, ihor.solodrai, shuah,
gnq25, chenyuan, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH bpf-next v3 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context
2026-09-01 6:28 ` [PATCH bpf-next v3 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context chenyuan_fl
@ 2026-09-01 7:37 ` bot+bpf-ci
0 siblings, 0 replies; 20+ messages in thread
From: bot+bpf-ci @ 2026-09-01 7:37 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, mykyta.yatsenko5, ast, daniel,
andrii, eddyz87, memxor, martin.lau, emil, ihor.solodrai, shuah,
gnq25, chenyuan, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH bpf-next v3 1/4] bpf: Cancel special fields in resizable hashtab on recycle
2026-09-01 6:28 ` [PATCH bpf-next v3 1/4] " chenyuan_fl
2026-09-01 7:37 ` bot+bpf-ci
@ 2026-09-01 16:57 ` Mykyta Yatsenko
1 sibling, 0 replies; 20+ messages in thread
From: Mykyta Yatsenko @ 2026-09-01 16:57 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Emil Tsalapatis,
Ihor Solodrai, Shuah Khan, Nuoqi Gui, Yuan Chen
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();
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH bpf-next v3 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor
2026-09-01 6:28 ` [PATCH bpf-next v3 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor chenyuan_fl
@ 2026-09-01 17:10 ` Mykyta Yatsenko
0 siblings, 0 replies; 20+ messages in thread
From: Mykyta Yatsenko @ 2026-09-01 17:10 UTC (permalink / raw)
To: chenyuan_fl, bpf
Cc: linux-kernel, linux-kselftest, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Martin KaFai Lau, Emil Tsalapatis,
Ihor Solodrai, Shuah Khan, Nuoqi Gui, Yuan Chen
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;
> }
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-09-01 17:10 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <DKM6K95EN9OF.3O9XNYWVLYHDE@gmail.com>
2026-08-24 14:36 ` [PATCH bpf-next v2 0/4] bpf: Cancel special fields in resizable hashtab on recycle chenyuan_fl
2026-08-24 14:36 ` [PATCH 1/4] " chenyuan_fl
2026-08-24 15:42 ` bot+bpf-ci
2026-08-24 16:15 ` Mykyta Yatsenko
2026-09-01 6:28 ` [PATCH bpf-next v3 0/4] " chenyuan_fl
2026-09-01 6:28 ` [PATCH bpf-next v3 1/4] " chenyuan_fl
2026-09-01 7:37 ` bot+bpf-ci
2026-09-01 16:57 ` Mykyta Yatsenko
2026-09-01 6:28 ` [PATCH bpf-next v3 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor chenyuan_fl
2026-09-01 17:10 ` Mykyta Yatsenko
2026-09-01 6:28 ` [PATCH bpf-next v3 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context chenyuan_fl
2026-09-01 7:37 ` bot+bpf-ci
2026-09-01 6:28 ` [PATCH bpf-next v3 4/4] selftests/bpf: Test rhtab special-field combinations chenyuan_fl
2026-09-01 7:37 ` bot+bpf-ci
2026-08-24 14:36 ` [PATCH 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor chenyuan_fl
2026-08-24 15:42 ` bot+bpf-ci
2026-08-24 14:36 ` [PATCH 3/4] selftests/bpf: Test rhtab kptr recycle from NMI context chenyuan_fl
2026-08-24 15:42 ` bot+bpf-ci
2026-08-24 14:36 ` [PATCH 4/4] selftests/bpf: Test rhtab special-field combinations chenyuan_fl
2026-08-24 15:42 ` bot+bpf-ci
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox