* [PATCH bpf-next v2 1/8] bpf: Mark task work FREED before detaching it
2026-11-06 9:57 [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Yun Lu
@ 2026-11-06 9:57 ` Yun Lu
2026-11-06 9:57 ` [PATCH bpf-next v2 2/8] bpf: Check task work value membership before scheduling Yun Lu
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Yun Lu @ 2026-11-06 9:57 UTC (permalink / raw)
To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
From: Yun Lu <luyun@kylinos.cn>
bpf_task_work_cancel_and_free() clears the value's context slot before
marking the context FREED. A concurrent deletion can observe the empty
slot and return while the detached context can still enter RUNNING:
uref cleanup delete target task
xchg(slot, NULL) gets C
unlink E
cancel sees NULL
return
C -> RUNNING
xchg(C->state, FREED)
A preallocated HASH uref walk racing with deletion allows this missed
cancellation. It does not itself free the element, but deletion fails to
prevent the old context from starting a callback.
Mark FREED before conditionally clearing the slot. A callback that has
not already entered RUNNING can no longer start once C is detached, and
the compare-and-exchange does not remove a replacement context.
Unlike xchg(), reading the slot does not acquire exclusive ownership of
C. Hold regular RCU while competing cancellers can detach C and drop its
reference. Preserve the old xchg()'s unlink-to-slot ordering with a full
barrier, including when the slot is NULL. Only the first FREED transition
drops or transfers the map-owned reference; later cancellers may help
detach C without disposing of that reference again.
Fixes: 38aa7003e369 ("bpf: task work scheduling kfuncs")
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
kernel/bpf/helpers.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 6788489a2608..281fedabf7b0 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -5089,11 +5089,20 @@ void bpf_task_work_cancel_and_free(void *val)
struct bpf_task_work_ctx *ctx;
enum bpf_task_work_state state;
- ctx = xchg(&twk->ctx, NULL);
+ /* Another canceller can release the map-owned context reference. */
+ guard(rcu)();
+ /* Preserve unlink-to-slot ordering, including the NULL case. */
+ smp_mb();
+ ctx = READ_ONCE(twk->ctx);
if (!ctx)
return;
+ /* Stop callbacks before making the context invisible to deletion. */
state = xchg(&ctx->state, BPF_TW_FREED);
+ (void)cmpxchg(&twk->ctx, ctx, NULL);
+ /* Only the first FREED transition owns the map's reference. */
+ if (state == BPF_TW_FREED)
+ return;
if (state == BPF_TW_SCHEDULED) {
/* run in irq_work to avoid locks in NMI */
init_irq_work(&ctx->irq_work, bpf_task_work_cancel_scheduled);
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH bpf-next v2 2/8] bpf: Check task work value membership before scheduling
2026-11-06 9:57 [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Yun Lu
2026-11-06 9:57 ` [PATCH bpf-next v2 1/8] bpf: Mark task work FREED before detaching it Yun Lu
@ 2026-11-06 9:57 ` Yun Lu
2026-11-06 9:57 ` [PATCH bpf-next v2 3/8] selftests/bpf: Check task work value membership Yun Lu
` (6 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Yun Lu @ 2026-11-06 9:57 UTC (permalink / raw)
To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
From: Yun Lu <luyun@kylinos.cn>
Cancellation clears the task-work slot, but a BPF program can still hold
the deleted element's value pointer in its read-side section. Scheduling
through that pointer can publish a new context. Its reference keeps the
context alive, not the element referenced by ctx->map_val:
1. A BPF program looks up E and retains its value pointer.
2. Another task deletes E, cancels its task work and clears the slot.
3. The original reader publishes a new context C, schedules task work
and leaves its trace read-side section.
4. C enters RUNNING outside the grace period's waiting set for E,
before the final destructor marks C FREED.
5. The destructor frees E without waiting for C. The callback then
accesses E through C->map_val.
A controlled reproducer on the unmodified kernel at commit 7c3188f84b54
("selftests/bpf: Time tc_edt from its first packet with data") reported:
BUG: KASAN: slab-use-after-free in array_map_update_elem+0x4b8/0x7f0
Read of size 8 at addr ffff88800e61be98 by task uaf_repro/91
Call Trace:
__asan_memcpy+0x23/0x60
array_map_update_elem+0x4b8/0x7f0
bpf_prog_976f3fa6d36fbef6_cb_probe_step+0xaf/0xe5
bpf_prog_ac7215eae4609044_process_work_cb+0x12c/0x234
bpf_task_work_callback+0x351/0x970
task_work_run+0x116/0x200
Allocated by task 90:
alloc_bulk+0x238/0x360
bpf_mem_alloc_init+0x2fd/0x930
rhtab_map_alloc+0x352/0x430
Freed by task 9:
kfree+0x1b8/0x550
free_all.isra.0+0x5b/0x110
__free_rcu+0x87/0x1b0
srcu_invoke_callbacks+0x360/0x5c0
After publishing C and claiming PENDING, look up the key and require the
returned pointer to match the retained value. This is before irq_work is
queued: either lookup rejects the removed value or deletion finds the
published context and marks it FREED. Take regular RCU around the lookup
because sleepable callers hold only a trace read-side section.
On rejection, conditionally roll PENDING back to STANDBY so a concurrent
FREED is preserved, and drop this attempt's callback reference. The idle
context remains available for normal cancellation and reclamation.
Check membership before the usercnt policy, so a removed value returns
-ENOENT when this check is reached even if usercnt is zero. Compare value
addresses as well as keys: replacement must reject the old value, while
reinsertion at the same address is the current member. Live in-place
updates retain their existing behavior.
Fixes: 38aa7003e369 ("bpf: task work scheduling kfuncs")
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
kernel/bpf/helpers.c | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 281fedabf7b0..a2c199aed893 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1148,6 +1148,18 @@ static void *map_key_from_value(struct bpf_map *map, void *value, u32 *arr_idx)
return (void *)value - round_up(map->key_size, 8);
}
+/* Does lookup still return this value under its key? */
+static bool bpf_map_value_is_linked(struct bpf_map *map, void *value)
+{
+ u32 idx;
+ void *key;
+
+ /* Sleepable callers need regular RCU for the map lookup itself. */
+ guard(rcu)();
+ key = map_key_from_value(map, value, &idx);
+ return map->ops->map_lookup_elem(map, key) == value;
+}
+
enum bpf_async_type {
BPF_ASYNC_TYPE_TIMER = 0,
BPF_ASYNC_TYPE_WQ,
@@ -4774,6 +4786,16 @@ static struct bpf_task_work_ctx *bpf_task_work_acquire_ctx(struct bpf_task_work
return ERR_PTR(-EBUSY);
}
+ /*
+ * ctx is published and PENDING, with no irq_work queued. Either
+ * lookup rejects the value or deletion finds ctx.
+ */
+ if (!bpf_map_value_is_linked(map, (void *)tw - map->record->task_work_off)) {
+ (void)cmpxchg(&ctx->state, BPF_TW_PENDING, BPF_TW_STANDBY);
+ bpf_task_work_ctx_put(ctx);
+ return ERR_PTR(-ENOENT);
+ }
+
/*
* If no process or bpffs is holding a reference to the map, no new callbacks should be
* scheduled. This does not address any race or correctness issue, but rather is a policy
@@ -4842,7 +4864,12 @@ release_prog:
* @callback: pointer to BPF subprogram to call
* @aux: pointer to bpf_prog_aux of the caller BPF program, implicitly set by the verifier
*
- * Return: 0 if task work has been scheduled successfully, negative error code otherwise
+ * A value reinserted at the same address is the current member. Success
+ * queues irq_work but does not guarantee callback delivery.
+ *
+ * Return: 0 if the scheduling irq_work was queued; -ENOENT if the membership
+ * lookup does not return this value; -EBUSY for a busy context or no user
+ * references; another negative error code on failure.
*/
__bpf_kfunc int bpf_task_work_schedule_signal(struct task_struct *task, struct bpf_task_work *tw,
void *map__const_map, bpf_task_work_callback_t callback,
@@ -4860,7 +4887,7 @@ __bpf_kfunc int bpf_task_work_schedule_signal(struct task_struct *task, struct b
* @callback: pointer to BPF subprogram to call
* @aux: pointer to bpf_prog_aux of the caller BPF program, implicitly set by the verifier
*
- * Return: 0 if task work has been scheduled successfully, negative error code otherwise
+ * Return: As for bpf_task_work_schedule_signal().
*/
__bpf_kfunc int bpf_task_work_schedule_resume(struct task_struct *task, struct bpf_task_work *tw,
void *map__const_map, bpf_task_work_callback_t callback,
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH bpf-next v2 3/8] selftests/bpf: Check task work value membership
2026-11-06 9:57 [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Yun Lu
2026-11-06 9:57 ` [PATCH bpf-next v2 1/8] bpf: Mark task work FREED before detaching it Yun Lu
2026-11-06 9:57 ` [PATCH bpf-next v2 2/8] bpf: Check task work value membership before scheduling Yun Lu
@ 2026-11-06 9:57 ` Yun Lu
2026-11-06 10:31 ` bot+bpf-ci
2026-11-06 10:56 ` sashiko-bot
2026-11-06 9:57 ` [PATCH bpf-next v2 4/8] bpf: Order async callback admission and cancellation Yun Lu
` (5 subsequent siblings)
8 siblings, 2 replies; 18+ messages in thread
From: Yun Lu @ 2026-11-06 9:57 UTC (permalink / raw)
To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
From: Yun Lu <luyun@kylinos.cn>
Exercise scheduling through value pointers retained across deletion,
replacement and same-key reinsertion. The lookup fix must reject a value
that is no longer the current member, including preallocated values that
previously accepted a new context after cancellation.
Add 46 subtests for both notification modes across HASH with and without
preallocation, RHASH, LRU HASH and ARRAY. Require -ENOENT for removed
values; record old/new addresses to distinguish replacement from a
same-address reinsertion, which is the current member. Also cover live
in-place updates, BPF_F_LOCK updates and last-user-reference cleanup
followed by reopening the map by ID.
Repeat removal after a successful live schedule to exercise a populated
slot as well as the initial empty one. The callback counts deliveries
without dereferencing key or value, so the negative comparison does not
deliberately access removed storage from the BPF callback.
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
.../bpf/prog_tests/task_work_membership.c | 392 ++++++++++++++++++
.../bpf/progs/task_work_membership.c | 158 +++++++
2 files changed, 550 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/task_work_membership.c
create mode 100644 tools/testing/selftests/bpf/progs/task_work_membership.c
diff --git a/tools/testing/selftests/bpf/prog_tests/task_work_membership.c b/tools/testing/selftests/bpf/prog_tests/task_work_membership.c
new file mode 100644
index 000000000000..0f5cdef3a24c
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/task_work_membership.c
@@ -0,0 +1,392 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Corporation. */
+#include <test_progs.h>
+#include <fcntl.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+#include "task_work_membership.skel.h"
+
+#define REUSE_ROUNDS 8
+#define COMPLETION_WAIT_NS 2000000000ULL
+#define COMPLETION_WAIT_ITERS 2000
+
+struct membership_value {
+ __u64 data;
+ struct bpf_task_work tw;
+};
+
+struct lock_value {
+ __u64 data;
+ struct bpf_spin_lock lock;
+ struct bpf_task_work tw;
+};
+
+static bool run_prog(struct bpf_program *prog, int expected)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, opts);
+ int err;
+
+ err = bpf_prog_test_run_opts(bpf_program__fd(prog), &opts);
+ return ASSERT_OK(err, "test_run") &&
+ ASSERT_EQ((__s32)opts.retval, expected, "retval");
+}
+
+static int run_prog_ret(struct bpf_program *prog, bool *ok)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, opts);
+ int err;
+
+ err = bpf_prog_test_run_opts(bpf_program__fd(prog), &opts);
+ *ok = ASSERT_OK(err, "test_run");
+ return *ok ? (__s32)opts.retval : 0;
+}
+
+static __u64 completed(struct task_work_membership *skel)
+{
+ return __atomic_load_n(&skel->bss->completed, __ATOMIC_ACQUIRE);
+}
+
+static bool wait_completed(struct task_work_membership *skel, __u64 expected)
+{
+ __u64 deadline = get_time_ns() + COMPLETION_WAIT_NS;
+ __u64 count;
+ int i;
+
+ for (i = 0; i < COMPLETION_WAIT_ITERS; i++) {
+ /* irq_work may enqueue task work after test_run returned. */
+ syscall(__NR_gettid); /* Give TWA_RESUME another return to user mode. */
+ count = completed(skel);
+ if (count >= expected)
+ return ASSERT_EQ(count, expected, "completed");
+ if (get_time_ns() >= deadline)
+ break;
+ usleep(1000);
+ }
+ return ASSERT_EQ(completed(skel), expected, "callback_timeout");
+}
+
+static struct task_work_membership *open_test(enum bpf_map_type type, __u32 flags,
+ bool use_signal)
+{
+ struct task_work_membership *skel;
+ int err;
+
+ skel = task_work_membership__open();
+ if (!ASSERT_OK_PTR(skel, "open"))
+ return NULL;
+ err = bpf_map__set_type(skel->maps.values, type);
+ if (!ASSERT_OK(err, "map_type"))
+ goto fail;
+ err = bpf_map__set_map_flags(skel->maps.values, flags);
+ if (!ASSERT_OK(err, "map_flags"))
+ goto fail;
+ /* A full LRU map can evict the target while obtaining a candidate. */
+ if (type == BPF_MAP_TYPE_LRU_HASH) {
+ err = bpf_map__set_max_entries(skel->maps.values, 2);
+ if (!ASSERT_OK(err, "map_max_entries"))
+ goto fail;
+ }
+ if (!ASSERT_OK(task_work_membership__load(skel), "load"))
+ goto fail;
+ skel->bss->use_signal = use_signal;
+ return skel;
+fail:
+ task_work_membership__destroy(skel);
+ return NULL;
+}
+
+static bool insert_value(struct task_work_membership *skel)
+{
+ struct membership_value value = {};
+ int key = 0;
+
+ return ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.values),
+ &key, &value, BPF_ANY), "insert");
+}
+
+static bool insert_lock_value(struct task_work_membership *skel)
+{
+ struct lock_value value = {};
+ int key = 0;
+
+ return ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.lock_values),
+ &key, &value, BPF_ANY), "insert_lock");
+}
+
+static void retired_value(enum bpf_map_type type, __u32 flags, bool use_signal,
+ bool replace)
+{
+ struct task_work_membership *skel;
+ struct bpf_program *mutate;
+ bool ok;
+ int i;
+
+ skel = open_test(type, flags, use_signal);
+ if (!skel)
+ return;
+ mutate = replace ? skel->progs.update_then_schedule :
+ skel->progs.delete_then_schedule;
+ if (!insert_value(skel))
+ goto out;
+
+ for (i = 0; i < REUSE_ROUNDS; i++) {
+ /* Later rounds start with a ctx left by the live schedule below. */
+ ok = run_prog(mutate, -ENOENT);
+ if (replace)
+ fprintf(stderr,
+ "replace address map_type=%d old=%#llx new=%#llx\n",
+ (int)type,
+ (unsigned long long)skel->bss->old_value,
+ (unsigned long long)skel->bss->new_value);
+ if (!ok ||
+ !ASSERT_OK(skel->bss->mutation_err, "mutation") ||
+ !ASSERT_EQ(completed(skel), i, "retired_not_called"))
+ goto out;
+ if (replace)
+ ASSERT_NEQ(skel->bss->old_value, skel->bss->new_value,
+ "replace_address");
+ if (!replace && !insert_value(skel))
+ goto out;
+ if (!run_prog(skel->progs.schedule_live, 0) ||
+ !wait_completed(skel, i + 1))
+ goto out;
+ }
+out:
+ task_work_membership__destroy(skel);
+}
+
+static void live_update(enum bpf_map_type type, __u32 flags, bool use_signal)
+{
+ struct task_work_membership *skel;
+
+ skel = open_test(type, flags, use_signal);
+ if (!skel)
+ return;
+ if (!insert_value(skel) || !run_prog(skel->progs.schedule_live, 0) ||
+ !wait_completed(skel, 1))
+ goto out;
+ if (!run_prog(skel->progs.update_then_schedule, 0) ||
+ !ASSERT_OK(skel->bss->mutation_err, "update"))
+ goto out;
+ wait_completed(skel, 2);
+out:
+ task_work_membership__destroy(skel);
+}
+
+static void reinsert(enum bpf_map_type type, __u32 flags, bool use_signal)
+{
+ struct task_work_membership *skel;
+ bool ok;
+ int ret;
+
+ skel = open_test(type, flags, use_signal);
+ if (!skel)
+ return;
+ if (!insert_value(skel))
+ goto out;
+ ret = run_prog_ret(skel->progs.reinsert_then_schedule, &ok);
+ fprintf(stderr, "%s address map_type=%d old=%#llx new=%#llx same=%d\n",
+ __func__, (int)type, (unsigned long long)skel->bss->old_value,
+ (unsigned long long)skel->bss->new_value,
+ skel->bss->old_value == skel->bss->new_value);
+ if (!ok || !ASSERT_OK(skel->bss->mutation_err, "mutation"))
+ goto out;
+
+ if (skel->bss->old_value == skel->bss->new_value) {
+ /* Same-address reinsertion is the current member. */
+ if (!ASSERT_EQ(ret, 0, "same_address_retval"))
+ goto out;
+ wait_completed(skel, 1);
+ } else {
+ ASSERT_EQ(ret, -ENOENT, "different_address_retval");
+ ASSERT_EQ(completed(skel), 0, "different_address_not_called");
+ }
+out:
+ task_work_membership__destroy(skel);
+}
+
+static void lock_update(__u32 flags, bool use_signal)
+{
+ struct task_work_membership *skel;
+ int err;
+
+ skel = task_work_membership__open();
+ if (!ASSERT_OK_PTR(skel, "open"))
+ return;
+ err = bpf_map__set_map_flags(skel->maps.lock_values, flags);
+ if (!ASSERT_OK(err, "lock_map_flags"))
+ goto out;
+ if (!ASSERT_OK(task_work_membership__load(skel), "load"))
+ goto out;
+ skel->bss->use_signal = use_signal;
+ if (!insert_lock_value(skel) ||
+ !run_prog(skel->progs.lock_schedule_live, 0) ||
+ !wait_completed(skel, 1))
+ goto out;
+ if (!run_prog(skel->progs.lock_update_then_schedule, 0) ||
+ !ASSERT_OK(skel->bss->mutation_err, "lock_update"))
+ goto out;
+ wait_completed(skel, 2);
+out:
+ task_work_membership__destroy(skel);
+}
+
+static void uref_reopen(enum bpf_map_type type, __u32 flags, bool used)
+{
+ struct bpf_map_info info = {};
+ __u32 info_len = sizeof(info);
+ struct task_work_membership *skel;
+ int map_fd, null_fd = -1, reopened_fd = -1;
+ int err;
+
+ skel = open_test(type, flags, false);
+ if (!skel)
+ return;
+ if (!insert_value(skel))
+ goto out;
+ if (used && (!run_prog(skel->progs.schedule_live, 0) ||
+ !wait_completed(skel, 1)))
+ goto out;
+ map_fd = bpf_map__fd(skel->maps.values);
+ err = bpf_map_get_info_by_fd(map_fd, &info, &info_len);
+ if (!ASSERT_OK(err, "map_info"))
+ goto out;
+ null_fd = open("/dev/null", O_RDONLY | O_CLOEXEC);
+ if (!ASSERT_GE(null_fd, 0, "open_dev_null"))
+ goto out;
+ /*
+ * Drop the sole map FD, retaining only program references. Reserve
+ * its number so skeleton cleanup cannot close an unrelated new FD.
+ */
+ if (!ASSERT_EQ(dup3(null_fd, map_fd, O_CLOEXEC), map_fd, "drop_uref"))
+ goto out;
+ if (!run_prog(skel->progs.schedule_live, -EBUSY))
+ goto out;
+ reopened_fd = bpf_map_get_fd_by_id(info.id);
+ if (!ASSERT_GE(reopened_fd, 0, "reopen_by_id"))
+ goto out;
+ if (!ASSERT_EQ(dup3(reopened_fd, map_fd, O_CLOEXEC), map_fd, "restore_map_fd"))
+ goto out;
+ if (!run_prog(skel->progs.schedule_live, 0))
+ goto out;
+ wait_completed(skel, used ? 2 : 1);
+out:
+ if (reopened_fd >= 0)
+ close(reopened_fd);
+ if (null_fd >= 0)
+ close(null_fd);
+ task_work_membership__destroy(skel);
+}
+
+static void usercnt_zero_deleted(enum bpf_map_type type, __u32 flags, bool use_signal)
+{
+ struct task_work_membership *skel;
+ int map_fd, null_fd = -1;
+
+ skel = open_test(type, flags, use_signal);
+ if (!skel)
+ return;
+ if (!insert_value(skel))
+ goto out;
+ map_fd = bpf_map__fd(skel->maps.values);
+ null_fd = open("/dev/null", O_RDONLY | O_CLOEXEC);
+ if (!ASSERT_GE(null_fd, 0, "open_dev_null"))
+ goto out;
+ if (!ASSERT_EQ(dup3(null_fd, map_fd, O_CLOEXEC), map_fd, "drop_uref"))
+ goto out;
+ /*
+ * Membership is checked before the usercnt policy: a deleted value
+ * returns -ENOENT even after the last user reference is gone.
+ */
+ if (!run_prog(skel->progs.delete_then_schedule, -ENOENT))
+ goto out;
+ ASSERT_OK(skel->bss->mutation_err, "delete");
+out:
+ if (null_fd >= 0)
+ close(null_fd);
+ task_work_membership__destroy(skel);
+}
+
+void test_task_work_membership(void)
+{
+ static const struct {
+ const char *name;
+ enum bpf_map_type type;
+ __u32 flags;
+ } maps[] = {
+ { "hash", BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC },
+ { "rhash", BPF_MAP_TYPE_RHASH, BPF_F_NO_PREALLOC },
+ { "array", BPF_MAP_TYPE_ARRAY, 0 },
+ { "prealloc_hash", BPF_MAP_TYPE_HASH, 0 },
+ { "lru", BPF_MAP_TYPE_LRU_HASH, 0 },
+ };
+ char name[96];
+ int signal, i, used;
+
+ for (signal = 0; signal < 2; signal++) {
+ const char *mode = signal ? "signal" : "resume";
+
+ snprintf(name, sizeof(name), "hash_delete_%s", mode);
+ if (test__start_subtest(name))
+ retired_value(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC, signal, false);
+ snprintf(name, sizeof(name), "prealloc_hash_delete_%s", mode);
+ if (test__start_subtest(name))
+ retired_value(BPF_MAP_TYPE_HASH, 0, signal, false);
+ snprintf(name, sizeof(name), "rhash_delete_%s", mode);
+ if (test__start_subtest(name))
+ retired_value(BPF_MAP_TYPE_RHASH, BPF_F_NO_PREALLOC, signal, false);
+ snprintf(name, sizeof(name), "lru_delete_%s", mode);
+ if (test__start_subtest(name))
+ retired_value(BPF_MAP_TYPE_LRU_HASH, 0, signal, false);
+ snprintf(name, sizeof(name), "hash_replace_%s", mode);
+ if (test__start_subtest(name))
+ retired_value(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC, signal, true);
+ snprintf(name, sizeof(name), "prealloc_hash_replace_%s", mode);
+ if (test__start_subtest(name))
+ retired_value(BPF_MAP_TYPE_HASH, 0, signal, true);
+ snprintf(name, sizeof(name), "lru_replace_%s", mode);
+ if (test__start_subtest(name))
+ retired_value(BPF_MAP_TYPE_LRU_HASH, 0, signal, true);
+ snprintf(name, sizeof(name), "hash_reinsert_%s", mode);
+ if (test__start_subtest(name))
+ reinsert(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC, signal);
+ snprintf(name, sizeof(name), "prealloc_hash_reinsert_%s", mode);
+ if (test__start_subtest(name))
+ reinsert(BPF_MAP_TYPE_HASH, 0, signal);
+ snprintf(name, sizeof(name), "lru_reinsert_%s", mode);
+ if (test__start_subtest(name))
+ reinsert(BPF_MAP_TYPE_LRU_HASH, 0, signal);
+ snprintf(name, sizeof(name), "rhash_update_%s", mode);
+ if (test__start_subtest(name))
+ live_update(BPF_MAP_TYPE_RHASH, BPF_F_NO_PREALLOC, signal);
+ snprintf(name, sizeof(name), "array_update_%s", mode);
+ if (test__start_subtest(name))
+ live_update(BPF_MAP_TYPE_ARRAY, 0, signal);
+ snprintf(name, sizeof(name), "lock_hash_update_%s", mode);
+ if (test__start_subtest(name))
+ lock_update(BPF_F_NO_PREALLOC, signal);
+ snprintf(name, sizeof(name), "lock_hash_prealloc_update_%s", mode);
+ if (test__start_subtest(name))
+ lock_update(0, signal);
+ snprintf(name, sizeof(name), "usercnt_zero_deleted_hash_%s", mode);
+ if (test__start_subtest(name))
+ usercnt_zero_deleted(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC, signal);
+ snprintf(name, sizeof(name), "usercnt_zero_deleted_prealloc_hash_%s", mode);
+ if (test__start_subtest(name))
+ usercnt_zero_deleted(BPF_MAP_TYPE_HASH, 0, signal);
+ snprintf(name, sizeof(name), "usercnt_zero_deleted_rhash_%s", mode);
+ if (test__start_subtest(name))
+ usercnt_zero_deleted(BPF_MAP_TYPE_RHASH, BPF_F_NO_PREALLOC, signal);
+ snprintf(name, sizeof(name), "usercnt_zero_deleted_lru_%s", mode);
+ if (test__start_subtest(name))
+ usercnt_zero_deleted(BPF_MAP_TYPE_LRU_HASH, 0, signal);
+ }
+ for (i = 0; i < ARRAY_SIZE(maps); i++) {
+ for (used = 0; used < 2; used++) {
+ snprintf(name, sizeof(name), "%s_uref_reopen_%s", maps[i].name,
+ used ? "used" : "unused");
+ if (test__start_subtest(name))
+ uref_reopen(maps[i].type, maps[i].flags, used);
+ }
+ }
+}
diff --git a/tools/testing/selftests/bpf/progs/task_work_membership.c b/tools/testing/selftests/bpf/progs/task_work_membership.c
new file mode 100644
index 000000000000..025f8c1bfb70
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/task_work_membership.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Corporation. */
+#include <vmlinux.h>
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+struct elem {
+ __u64 data;
+ struct bpf_task_work tw;
+};
+
+struct lock_elem {
+ __u64 data;
+ struct bpf_spin_lock lock;
+ struct bpf_task_work tw;
+};
+
+struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __uint(map_flags, BPF_F_NO_PREALLOC);
+ __uint(max_entries, 1);
+ __type(key, int);
+ __type(value, struct elem);
+} values SEC(".maps");
+
+struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __uint(map_flags, BPF_F_NO_PREALLOC);
+ __uint(max_entries, 1);
+ __type(key, int);
+ __type(value, struct lock_elem);
+} lock_values SEC(".maps");
+
+bool use_signal;
+int mutation_err;
+__u64 old_value;
+__u64 new_value;
+__u64 completed;
+
+static int callback(struct bpf_map *map, void *key, void *value)
+{
+ /* Count deliveries without dereferencing a possibly removed value. */
+ __sync_fetch_and_add(&completed, 1);
+ return 0;
+}
+
+static __always_inline int schedule_value(struct elem *value)
+{
+ struct task_struct *task = bpf_get_current_task_btf();
+
+ if (use_signal)
+ return bpf_task_work_schedule_signal(task, &value->tw, &values, callback);
+ return bpf_task_work_schedule_resume(task, &value->tw, &values, callback);
+}
+
+static __always_inline int schedule_lock_value(struct lock_elem *value)
+{
+ struct task_struct *task = bpf_get_current_task_btf();
+
+ if (use_signal)
+ return bpf_task_work_schedule_signal(task, &value->tw, &lock_values, callback);
+ return bpf_task_work_schedule_resume(task, &value->tw, &lock_values, callback);
+}
+
+SEC("syscall")
+int schedule_live(void *ctx)
+{
+ int key = 0;
+ struct elem *value = bpf_map_lookup_elem(&values, &key);
+
+ return value ? schedule_value(value) : -EINVAL;
+}
+
+SEC("syscall")
+int delete_then_schedule(void *ctx)
+{
+ int key = 0;
+ struct elem *value = bpf_map_lookup_elem(&values, &key);
+
+ if (!value)
+ return -EINVAL;
+ mutation_err = bpf_map_delete_elem(&values, &key);
+ if (mutation_err)
+ return mutation_err;
+
+ /* test_run holds the trace read-side section across both operations. */
+ return schedule_value(value);
+}
+
+SEC("syscall")
+int update_then_schedule(void *ctx)
+{
+ struct elem replacement = {};
+ int key = 0;
+ struct elem *value = bpf_map_lookup_elem(&values, &key);
+
+ if (!value)
+ return -EINVAL;
+ old_value = (u64)(long)value;
+ mutation_err = bpf_map_update_elem(&values, &key, &replacement, BPF_EXIST);
+ if (mutation_err)
+ return mutation_err;
+ new_value = (u64)(long)bpf_map_lookup_elem(&values, &key);
+
+ /* HASH replaces the element; RHASH and ARRAY update it in place. */
+ return schedule_value(value);
+}
+
+SEC("syscall")
+int reinsert_then_schedule(void *ctx)
+{
+ struct elem replacement = {};
+ int key = 0;
+ struct elem *value = bpf_map_lookup_elem(&values, &key);
+
+ if (!value)
+ return -EINVAL;
+ mutation_err = bpf_map_delete_elem(&values, &key);
+ if (mutation_err)
+ return mutation_err;
+ mutation_err = bpf_map_update_elem(&values, &key, &replacement, BPF_ANY);
+ if (mutation_err)
+ return mutation_err;
+
+ old_value = (u64)(long)value;
+ new_value = (u64)(long)bpf_map_lookup_elem(&values, &key);
+ return schedule_value(value);
+}
+
+SEC("syscall")
+int lock_schedule_live(void *ctx)
+{
+ int key = 0;
+ struct lock_elem *value = bpf_map_lookup_elem(&lock_values, &key);
+
+ return value ? schedule_lock_value(value) : -EINVAL;
+}
+
+SEC("syscall")
+int lock_update_then_schedule(void *ctx)
+{
+ struct lock_elem replacement = {};
+ int key = 0;
+ struct lock_elem *value = bpf_map_lookup_elem(&lock_values, &key);
+
+ if (!value)
+ return -EINVAL;
+ mutation_err = bpf_map_update_elem(&lock_values, &key, &replacement,
+ BPF_EXIST | BPF_F_LOCK);
+ if (mutation_err)
+ return mutation_err;
+
+ return schedule_lock_value(value);
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH bpf-next v2 3/8] selftests/bpf: Check task work value membership
2026-11-06 9:57 ` [PATCH bpf-next v2 3/8] selftests/bpf: Check task work value membership Yun Lu
@ 2026-11-06 10:31 ` bot+bpf-ci
2026-11-06 10:56 ` sashiko-bot
1 sibling, 0 replies; 18+ messages in thread
From: bot+bpf-ci @ 2026-11-06 10:31 UTC (permalink / raw)
To: luyun_611, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2772 bytes --]
> selftests/bpf: Check task work value membership
TL;DR: Test issue: LRU subtests with max_entries=2 and no CPU pinning may
spuriously fail (-ENOENT) if the thread migrates and the LRU evicts the live
key, with no kernel bug.
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_work_membership.c b/tools/testing/selftests/bpf/prog_tests/task_work_membership.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/task_work_membership.c
[ ... ]
> +static struct task_work_membership *open_test(enum bpf_map_type type, __u32 flags,
> + bool use_signal)
> +{
[ ... ]
> + /* A full LRU map can evict the target while obtaining a candidate. */
> + if (type == BPF_MAP_TYPE_LRU_HASH) {
> + err = bpf_map__set_max_entries(skel->maps.values, 2);
> + if (!ASSERT_OK(err, "map_max_entries"))
> + goto fail;
> + }
Is max_entries = 2 enough to keep the common LRU from evicting the live
key 0 in the lru_replace_* subtests (retired_value(BPF_MAP_TYPE_LRU_HASH,
0, signal, true), REUSE_ROUNDS = 8)?
htab_lru_map_update_elem() calls prealloc_lru_pop() before it looks up
l_old. With 2 elements, bpf_common_lru_populate() sets target_free to 1,
and a node freed while pending on CPU P goes back to P's private
free_llist, not to the global free list.
If the test thread migrates between rounds, this sequence seems possible:
insert_value() on CPU 0:
node A becomes key 0, pending on CPU 0; node B stays on the global
free list.
thread moves to CPU 1, round 0 of update_then_schedule:
takes B from the global free list (B pending on CPU 1);
htab_lru_push_free(A) puts A on CPU 0's free_llist.
round 1, still on CPU 1:
prealloc_lru_pop() -> bpf_common_lru_pop_free() finds CPU 1's
free_llist empty and calls bpf_lru_list_pop_free_to_local(), which
flushes B to the global active list, finds the global free list empty
and calls __bpf_lru_list_shrink(). That force-shrinks the active
list and unlinks B, the live key 0, via htab_lru_map_delete_node().
Back in htab_lru_map_update_elem(), lookup_elem_raw() then returns NULL and
check_flags() returns -ENOENT for BPF_EXIST. The program stores
mutation_err = -ENOENT, so ASSERT_OK(skel->bss->mutation_err, "mutation")
fails without any kernel bug.
The test does not pin itself to a CPU, whereas test_lru_map.c uses
sched_setaffinity() for this reason. Could the LRU subtests pin the thread
to one CPU, or size the map so the global free list cannot run empty within
REUSE_ROUNDS rounds?
---
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/42200504781
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH bpf-next v2 3/8] selftests/bpf: Check task work value membership
2026-11-06 9:57 ` [PATCH bpf-next v2 3/8] selftests/bpf: Check task work value membership Yun Lu
2026-11-06 10:31 ` bot+bpf-ci
@ 2026-11-06 10:56 ` sashiko-bot
1 sibling, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-11-06 10:56 UTC (permalink / raw)
To: Yun Lu; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] selftests/bpf: LRU eviction on CPU migration in open_test()
--
commit a48c230ac453ac2d2db53442d38749169b051723
Author: Yun Lu <luyun@kylinos.cn>
selftests/bpf: Check task work value membership
This commit adds BPF selftests verifying map value membership checks when
scheduling task_work across deletion, replacement, reinsertion, in-place
updates, and map user-reference drops.
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_work_membership.c b/tools/testing/selftests/bpf/prog_tests/task_work_membership.c
> new file mode 100644
> index 0000000000000..0f5cdef3a24c7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/task_work_membership.c
> @@ -0,0 +1,392 @@
[ ... ]
> +static struct task_work_membership *open_test(enum bpf_map_type type, __u32 flags,
> + bool use_signal)
> +{
> + struct task_work_membership *skel;
> + int err;
> +
> + skel = task_work_membership__open();
> + if (!ASSERT_OK_PTR(skel, "open"))
> + return NULL;
> + err = bpf_map__set_type(skel->maps.values, type);
> + if (!ASSERT_OK(err, "map_type"))
> + goto fail;
> + err = bpf_map__set_map_flags(skel->maps.values, flags);
> + if (!ASSERT_OK(err, "map_flags"))
> + goto fail;
> + /* A full LRU map can evict the target while obtaining a candidate. */
> + if (type == BPF_MAP_TYPE_LRU_HASH) {
> + err = bpf_map__set_max_entries(skel->maps.values, 2);
> + if (!ASSERT_OK(err, "map_max_entries"))
> + goto fail;
> + }
[ ... ]
> +static void retired_value(enum bpf_map_type type, __u32 flags, bool use_signal,
> + bool replace)
> +{
> + struct task_work_membership *skel;
> + struct bpf_program *mutate;
> + bool ok;
> + int i;
> +
> + skel = open_test(type, flags, use_signal);
> + if (!skel)
> + return;
> + mutate = replace ? skel->progs.update_then_schedule :
> + skel->progs.delete_then_schedule;
> + if (!insert_value(skel))
> + goto out;
> +
> + for (i = 0; i < REUSE_ROUNDS; i++) {
> + /* Later rounds start with a ctx left by the live schedule below. */
> + ok = run_prog(mutate, -ENOENT);
> + if (replace)
> + fprintf(stderr,
> + "replace address map_type=%d old=%#llx new=%#llx\n",
> + (int)type,
> + (unsigned long long)skel->bss->old_value,
> + (unsigned long long)skel->bss->new_value);
> + if (!ok ||
> + !ASSERT_OK(skel->bss->mutation_err, "mutation") ||
> + !ASSERT_EQ(completed(skel), i, "retired_not_called"))
> + goto out;
[Severity: Medium]
Can setting max_entries to 2 for BPF_MAP_TYPE_LRU_HASH still evict the live
entry in update_then_schedule() if the test thread migrates between CPUs
across iterations of retired_value()?
With max_entries = 2, bpf_common_lru_populate() allocates only two nodes (N1
and N2) on the global free list. When N1 is inserted on CPU A, it is placed
on CPU A's per-CPU pending_list with N1->cpu = A. If the unpinned test
thread sleeps in wait_completed() and migrates to CPU B for a replacement
round, prealloc_lru_pop() pops N2 from the global free list onto CPU B's
pending_list, and htab_lru_push_free() moves the replaced N1 from CPU A's
pending_list onto CPU A's per-CPU free_llist.
On the next round of update_then_schedule() on CPU B, CPU B's local
free_llist and the global free list are both empty. Before stealing from CPU
A's free_llist, bpf_common_lru_pop_free() calls
bpf_lru_list_pop_free_to_local(), which flushes the live N2 from CPU B's
pending_list and shrinks the global list, evicting key 0 from the hash table.
htab_lru_map_update_elem() then fails its BPF_EXIST check with -ENOENT,
setting mutation_err = -ENOENT and failing the "mutation" assertion in
lru_replace_resume and lru_replace_signal.
Using BPF_F_NO_COMMON_LRU with a sufficient max_entries, or pinning the test
thread to a single CPU, would avoid stranding the spare node on a remote CPU.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261106095747.471718-1-luyun_611@163.com?part=3
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH bpf-next v2 4/8] bpf: Order async callback admission and cancellation
2026-11-06 9:57 [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Yun Lu
` (2 preceding siblings ...)
2026-11-06 9:57 ` [PATCH bpf-next v2 3/8] selftests/bpf: Check task work value membership Yun Lu
@ 2026-11-06 9:57 ` Yun Lu
2026-11-06 9:57 ` [PATCH bpf-next v2 5/8] bpf: Check bpf_wq value membership before use Yun Lu
` (4 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Yun Lu @ 2026-11-06 9:57 UTC (permalink / raw)
To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
From: Yun Lu <luyun@kylinos.cn>
The shared wq/timer cancellation detaches cb before clearing cb->map.
Another canceller can observe the empty slot and return while that
callback object still allows the BPF callback to start:
canceller A canceller B callback
xchg(slot, NULL) gets C
reads NULL, returns
reads C->map
calls BPF program
WRITE_ONCE(C->map, NULL)
Clear cb->map before conditionally detaching cb. Only the successful
detacher clears fn/prog and consumes the map-owned reference. Keep cb
alive under regular RCU while the cancellers compete. Order unlink or
the last-user-reference drop before the slot read, including NULL, and
use an acquire load to observe the invalidation preceding another
canceller's detach.
Init failure currently rereads the slot to cancel it. Another canceller
may already have removed that init's cb and a new init may have installed
a replacement. Cancel the local cb instead, so failed cleanup cannot
terminate the replacement or consume its reference.
Publication also precedes init's usercnt check. A setter on another CPU
can otherwise configure cb before init rejects the last-user-reference
drop. Check cb->map and usercnt in the setter, returning -EINVAL for an
invalidated cb and -EPERM for zero usercnt. A full barrier orders these
checks after the publication observed by the setter, even when init ran
on a third CPU.
A setter that passed the checks can still race with cancellation. It
cannot restore cb->map, so the existing callback NULL check and read-side
protection continue to handle late fn/prog updates. Membership checks
for deleted values follow separately.
Fixes: 1bfbc267ec91 ("bpf: Enable bpf_timer and bpf_wq in any context")
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
kernel/bpf/helpers.c | 77 +++++++++++++++++++++++++++++++++++---------
1 file changed, 61 insertions(+), 16 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a2c199aed893..1227debd3e8a 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1374,7 +1374,8 @@ static void bpf_async_refcount_put(struct bpf_async_cb *cb)
}
}
-static void bpf_async_cancel_and_free(struct bpf_async_kern *async);
+static void bpf_async_cancel_cb(struct bpf_async_kern *async,
+ struct bpf_async_cb *cb);
static void bpf_async_irq_worker(struct irq_work *work);
static int __bpf_async_init(struct bpf_async_kern *async, struct bpf_map *map, u64 flags,
@@ -1436,17 +1437,15 @@ static int __bpf_async_init(struct bpf_async_kern *async, struct bpf_map *map, u
kfree_nolock(cb);
return -EBUSY;
}
- /* Guarantee the order between async->cb and map->usercnt. So
- * when there are concurrent uref release and bpf timer init, either
- * bpf_timer_cancel_and_free() called by uref release reads a no-NULL
- * timer or atomic64_read() below returns a zero usercnt.
+ /*
+ * Pair with uref cancellation: it sees cb or init sees usercnt=0.
*/
smp_mb();
if (!atomic64_read(&map->usercnt)) {
/* maps with timers must be either held by user space
* or pinned in bpffs.
*/
- bpf_async_cancel_and_free(async);
+ bpf_async_cancel_cb(async, cb);
return -EPERM;
}
@@ -1553,10 +1552,21 @@ static int __bpf_async_set_callback(struct bpf_async_kern *async, void *callback
struct bpf_prog *prog)
{
struct bpf_async_cb *cb;
+ struct bpf_map *map;
cb = READ_ONCE(async->cb);
if (!cb)
return -EINVAL;
+ /*
+ * Pair with cancellation even if another CPU published cb and has
+ * not finished its init checks.
+ */
+ smp_mb();
+ map = READ_ONCE(cb->map);
+ if (!map)
+ return -EINVAL;
+ if (!atomic64_read(&map->usercnt))
+ return -EPERM;
return bpf_async_update_prog_callback(cb, prog, callback_fn);
}
@@ -1744,19 +1754,16 @@ static void bpf_async_irq_worker(struct irq_work *work)
this_cpu_write(async_cb_running, NULL);
}
-static void bpf_async_cancel_and_free(struct bpf_async_kern *async)
+/* The caller's read-side section must keep cb alive. */
+static void bpf_async_cancel_cb(struct bpf_async_kern *async,
+ struct bpf_async_cb *cb)
{
- struct bpf_async_cb *cb;
-
- if (!READ_ONCE(async->cb))
- return;
-
- cb = xchg(&async->cb, NULL);
- if (!cb)
+ /* Make cb unusable before detaching it. */
+ WRITE_ONCE(cb->map, NULL);
+ /* Only the successful detacher consumes the slot's reference. */
+ if (cmpxchg(&async->cb, cb, NULL) != cb)
return;
- /* Callbacks that run after this see NULL and return. */
- WRITE_ONCE(cb->map, NULL);
bpf_async_update_prog_callback(cb, NULL, NULL);
/*
* No refcount_inc_not_zero(&cb->refcnt) here. Dropping the last
@@ -1776,6 +1783,19 @@ static void bpf_async_cancel_and_free(struct bpf_async_kern *async)
}
}
+static void bpf_async_cancel_and_free(struct bpf_async_kern *async)
+{
+ struct bpf_async_cb *cb;
+
+ guard(rcu)();
+ /* Order unlink/uref release before the slot read, including NULL. */
+ smp_mb();
+ /* Pair with the detach following cb->map = NULL. */
+ cb = smp_load_acquire(&async->cb);
+ if (cb)
+ bpf_async_cancel_cb(async, cb);
+}
+
/*
* This function is called by map_delete/update_elem for individual element and
* by ops->map_release_uref when the user space reference to a map reaches zero.
@@ -3490,6 +3510,18 @@ __bpf_kfunc void bpf_throw(u64 cookie)
WARN(1, "A call to BPF exception callback should never return\n");
}
+/**
+ * bpf_wq_init - Initialize a bpf_wq inside a map value.
+ * @wq: pointer to the bpf_wq field
+ * @p__const_map: the map containing @wq
+ * @flags: must be zero
+ *
+ * Success does not prevent concurrent cancellation.
+ *
+ * Return: 0 on success; -EINVAL for invalid flags; -EBUSY if already
+ * initialized; -ENOMEM on allocation failure; -EPERM if the map has no user
+ * reference.
+ */
__bpf_kfunc int bpf_wq_init(struct bpf_wq *wq, void *p__const_map, unsigned int flags)
{
struct bpf_async_kern *async = (struct bpf_async_kern *)wq;
@@ -3528,6 +3560,19 @@ __bpf_kfunc int bpf_wq_start(struct bpf_wq *wq, unsigned int flags)
}
}
+/**
+ * bpf_wq_set_callback - Configure the callback of a bpf_wq.
+ * @wq: pointer to the bpf_wq field
+ * @callback_fn: callback function
+ * @flags: must be zero
+ * @aux: implicit program argument
+ *
+ * Success does not prevent concurrent cancellation.
+ *
+ * Return: 0 on success; -EINVAL for invalid flags, if @wq was not initialized,
+ * or if its containing value was already invalidated; -EPERM if the map has
+ * no user references.
+ */
__bpf_kfunc int bpf_wq_set_callback(struct bpf_wq *wq,
int (callback_fn)(void *map, int *key, void *value),
unsigned int flags,
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH bpf-next v2 5/8] bpf: Check bpf_wq value membership before use
2026-11-06 9:57 [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Yun Lu
` (3 preceding siblings ...)
2026-11-06 9:57 ` [PATCH bpf-next v2 4/8] bpf: Order async callback admission and cancellation Yun Lu
@ 2026-11-06 9:57 ` Yun Lu
2026-11-06 9:57 ` [PATCH bpf-next v2 6/8] selftests/bpf: Check bpf_wq value membership Yun Lu
` (3 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Yun Lu @ 2026-11-06 9:57 UTC (permalink / raw)
To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
From: Yun Lu <luyun@kylinos.cn>
bpf_wq_init() checks the map's usercnt but not whether its value is still
linked. A program retaining a value pointer across deletion can publish
and configure a new cb after deletion has already cancelled the element:
reader delete / reclamation worker
retain value E
unlink E, cancel work
publish cb, set and start
leave read-side section
finish E's grace period
enter callback
cancel_work(), free E
access E
The callback's trace read-side section is outside that grace period's
waiting set. Keeping cb alive does not keep E alive, and the destructor's
cancel_work() does not wait for the running callback.
After publication and the existing usercnt check, look up the key and
require the result to equal cb->value. Either lookup rejects the removed
value or deletion finds the published cb. On rejection, cancel only this
init's cb and return -ENOENT.
Repeat membership checking in the setter and return -EINVAL on mismatch.
Another CPU can use the published cb before init completes its checks,
so checking only init would leave that configuration window open. Use
pointer identity to reject an old value after same-key replacement.
Enable these checks only for WQ here; timer follows separately.
Fixes: eb48f6cd41a0 ("bpf: wq: add bpf_wq_init")
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
kernel/bpf/helpers.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 1227debd3e8a..7bcb66c92abd 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1448,6 +1448,11 @@ static int __bpf_async_init(struct bpf_async_kern *async, struct bpf_map *map, u
bpf_async_cancel_cb(async, cb);
return -EPERM;
}
+ if (type == BPF_ASYNC_TYPE_WQ &&
+ !bpf_map_value_is_linked(map, cb->value)) {
+ bpf_async_cancel_cb(async, cb);
+ return -ENOENT;
+ }
return 0;
}
@@ -1567,6 +1572,9 @@ static int __bpf_async_set_callback(struct bpf_async_kern *async, void *callback
return -EINVAL;
if (!atomic64_read(&map->usercnt))
return -EPERM;
+ if (cb->type == BPF_ASYNC_TYPE_WQ &&
+ !bpf_map_value_is_linked(map, cb->value))
+ return -EINVAL;
return bpf_async_update_prog_callback(cb, prog, callback_fn);
}
@@ -3520,7 +3528,8 @@ __bpf_kfunc void bpf_throw(u64 cookie)
*
* Return: 0 on success; -EINVAL for invalid flags; -EBUSY if already
* initialized; -ENOMEM on allocation failure; -EPERM if the map has no user
- * reference.
+ * reference; -ENOENT if the current map lookup does not return this value
+ * when the membership check is reached.
*/
__bpf_kfunc int bpf_wq_init(struct bpf_wq *wq, void *p__const_map, unsigned int flags)
{
@@ -3570,8 +3579,8 @@ __bpf_kfunc int bpf_wq_start(struct bpf_wq *wq, unsigned int flags)
* Success does not prevent concurrent cancellation.
*
* Return: 0 on success; -EINVAL for invalid flags, if @wq was not initialized,
- * or if its containing value was already invalidated; -EPERM if the map has
- * no user references.
+ * if its containing value was already invalidated, or if the current map
+ * lookup does not return that value; -EPERM if the map has no user references.
*/
__bpf_kfunc int bpf_wq_set_callback(struct bpf_wq *wq,
int (callback_fn)(void *map, int *key, void *value),
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH bpf-next v2 6/8] selftests/bpf: Check bpf_wq value membership
2026-11-06 9:57 [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Yun Lu
` (4 preceding siblings ...)
2026-11-06 9:57 ` [PATCH bpf-next v2 5/8] bpf: Check bpf_wq value membership before use Yun Lu
@ 2026-11-06 9:57 ` Yun Lu
2026-11-06 10:31 ` bot+bpf-ci
2026-11-06 11:26 ` sashiko-bot
2026-11-06 9:57 ` [PATCH bpf-next v2 7/8] bpf: Check bpf_timer value membership before use Yun Lu
` (2 subsequent siblings)
8 siblings, 2 replies; 18+ messages in thread
From: Yun Lu @ 2026-11-06 9:57 UTC (permalink / raw)
To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
From: Yun Lu <luyun@kylinos.cn>
Add seven subtests to distinguish removed-value rejection from normal
bpf_wq operation: delete from HASH with and without preallocation, RHASH
and LRU HASH; replace a HASH element; and run live HASH/ARRAY callbacks.
Delete or replace the element while retaining its old value pointer in
the same read-side section, then require init through that pointer to
return -ENOENT. Stop after recording the stale init result even if an
unfixed kernel returns success; do not configure or start that object.
For replacement, assert that the old and new addresses differ and verify
a callback from the new member. The two live cases also initialize, set
and start work, then wait for an actual callback. This checks that the
new rejection path leaves linked values usable.
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
.../bpf/prog_tests/bpf_wq_membership.c | 199 ++++++++++++++++++
.../selftests/bpf/progs/bpf_wq_membership.c | 156 ++++++++++++++
2 files changed, 355 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_wq_membership.c
create mode 100644 tools/testing/selftests/bpf/progs/bpf_wq_membership.c
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_wq_membership.c b/tools/testing/selftests/bpf/prog_tests/bpf_wq_membership.c
new file mode 100644
index 000000000000..7206502a4873
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_wq_membership.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Corporation. */
+
+/* Five stale-init cases and two live callback cases. */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <unistd.h>
+
+#include <test_progs.h>
+#include "bpf_wq_membership.skel.h"
+
+#define CB_SLOTS 64
+#define CB_WAIT_ITERS 3000
+
+enum {
+ CMD_INIT_START = 1,
+ CMD_DELETE_INIT,
+ CMD_REPLACE_INIT,
+};
+
+struct wq_case {
+ struct bpf_wq_membership *skel;
+ struct bpf_map *map;
+ struct bpf_program *prog;
+ bool array;
+ int fd;
+};
+
+static struct wq_case open_case(enum bpf_map_type type, __u32 flags)
+{
+ struct wq_case c = {};
+ int err;
+
+ c.skel = bpf_wq_membership__open();
+ if (!ASSERT_OK_PTR(c.skel, "open"))
+ return c;
+ c.map = c.skel->maps.values;
+ c.prog = c.skel->progs.values_run;
+ c.array = type == BPF_MAP_TYPE_ARRAY;
+ err = bpf_map__set_type(c.map, type);
+ if (!ASSERT_OK(err, "set_type"))
+ goto fail;
+ err = bpf_map__set_map_flags(c.map, flags);
+ if (!ASSERT_OK(err, "set_flags"))
+ goto fail;
+ err = bpf_wq_membership__load(c.skel);
+ if (!ASSERT_OK(err, "load"))
+ goto fail;
+ c.fd = bpf_map__fd(c.map);
+ if (!ASSERT_GE(c.fd, 0, "map_fd"))
+ goto fail;
+ return c;
+fail:
+ bpf_wq_membership__destroy(c.skel);
+ c.skel = NULL;
+ return c;
+}
+
+static int run_cmd(struct wq_case *c, u32 cmd, u64 cookie)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, opts);
+ int err;
+
+ c->skel->bss->cmd = cmd;
+ c->skel->bss->cookie = cookie;
+ err = bpf_prog_test_run_opts(bpf_program__fd(c->prog), &opts);
+ if (!ASSERT_OK(err, "test_run"))
+ return err;
+ if (!ASSERT_OK((int)opts.retval, "prog_retval"))
+ return -EINVAL;
+ return 0;
+}
+
+static u64 cb_count(struct wq_case *c, u64 cookie)
+{
+ return __atomic_load_n(&c->skel->bss->cb_count[cookie & (CB_SLOTS - 1)],
+ __ATOMIC_ACQUIRE);
+}
+
+static bool wait_cb(struct wq_case *c, u64 cookie, u64 want)
+{
+ int i;
+
+ for (i = 0; i < CB_WAIT_ITERS; i++) {
+ if (cb_count(c, cookie) >= want)
+ return ASSERT_EQ(cb_count(c, cookie), want, "callback_count");
+ usleep(1000);
+ }
+ return ASSERT_EQ(cb_count(c, cookie), want, "callback_timeout");
+}
+
+static bool seed(struct wq_case *c)
+{
+ size_t size = bpf_map__value_size(c->map);
+ void *val = calloc(1, size);
+ int key = 0, err;
+
+ if (!ASSERT_OK_PTR(val, "calloc"))
+ return false;
+ err = bpf_map_update_elem(c->fd, &key, val, BPF_ANY);
+ free(val);
+ return ASSERT_OK(err, "insert");
+}
+
+static void drop_elem(struct wq_case *c)
+{
+ int key = 0;
+
+ if (c->array)
+ return;
+ bpf_map_delete_elem(c->fd, &key);
+}
+
+static bool check_init_start(struct wq_case *c)
+{
+ return ASSERT_OK(c->skel->bss->res_lookup_err, "lookup_err") &&
+ ASSERT_OK(c->skel->bss->res_init_err, "init_err") &&
+ ASSERT_OK(c->skel->bss->res_set_err, "set_err") &&
+ ASSERT_OK(c->skel->bss->res_start_err, "start_err");
+}
+
+static void subtest_live(enum bpf_map_type type, __u32 flags)
+{
+ struct wq_case c = open_case(type, flags);
+
+ if (!c.skel)
+ return;
+ if (!seed(&c))
+ goto out;
+ if (run_cmd(&c, CMD_INIT_START, 1))
+ goto out;
+ if (!check_init_start(&c))
+ goto out;
+ wait_cb(&c, 1, 1);
+out:
+ drop_elem(&c);
+ bpf_wq_membership__destroy(c.skel);
+}
+
+static void subtest_delete(enum bpf_map_type type, __u32 flags)
+{
+ struct wq_case c = open_case(type, flags);
+
+ if (!c.skel)
+ return;
+ if (!seed(&c))
+ goto out;
+ if (run_cmd(&c, CMD_DELETE_INIT, 1))
+ goto out;
+ ASSERT_OK(c.skel->bss->res_lookup_err, "lookup_err");
+ ASSERT_OK(c.skel->bss->res_delete_err, "delete_err");
+ ASSERT_EQ(c.skel->bss->res_init_err, -ENOENT, "init_err");
+ ASSERT_EQ(cb_count(&c, 1), 0, "no_callback");
+out:
+ bpf_wq_membership__destroy(c.skel);
+}
+
+static void subtest_replace(void)
+{
+ struct wq_case c = open_case(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC);
+
+ if (!c.skel)
+ return;
+ if (!seed(&c))
+ goto out;
+ if (run_cmd(&c, CMD_REPLACE_INIT, 1))
+ goto out;
+ ASSERT_OK(c.skel->bss->res_lookup_err, "lookup_err");
+ ASSERT_OK(c.skel->bss->res_update_err, "update_err");
+ ASSERT_NEQ(c.skel->bss->res_old_addr, c.skel->bss->res_new_addr,
+ "distinct_address");
+ ASSERT_EQ(c.skel->bss->res_init_err, -ENOENT, "init_err");
+ ASSERT_OK(c.skel->bss->res_new_init_err, "new_init_err");
+ ASSERT_OK(c.skel->bss->res_set_err, "set_err");
+ ASSERT_OK(c.skel->bss->res_start_err, "start_err");
+ wait_cb(&c, 1, 1);
+out:
+ drop_elem(&c);
+ bpf_wq_membership__destroy(c.skel);
+}
+
+void test_bpf_wq_membership(void)
+{
+ if (test__start_subtest("live_hash_np"))
+ subtest_live(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC);
+ if (test__start_subtest("live_array"))
+ subtest_live(BPF_MAP_TYPE_ARRAY, 0);
+ if (test__start_subtest("delete_hash_np"))
+ subtest_delete(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC);
+ if (test__start_subtest("delete_hash_pre"))
+ subtest_delete(BPF_MAP_TYPE_HASH, 0);
+ if (test__start_subtest("delete_rhash"))
+ subtest_delete(BPF_MAP_TYPE_RHASH, BPF_F_NO_PREALLOC);
+ if (test__start_subtest("delete_lru"))
+ subtest_delete(BPF_MAP_TYPE_LRU_HASH, 0);
+ if (test__start_subtest("replace_hash_np"))
+ subtest_replace();
+}
diff --git a/tools/testing/selftests/bpf/progs/bpf_wq_membership.c b/tools/testing/selftests/bpf/progs/bpf_wq_membership.c
new file mode 100644
index 000000000000..b78b4f1422b3
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpf_wq_membership.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Corporation. */
+
+/* Run one command per test_run; stale-pointer commands only record init. */
+
+#include <vmlinux.h>
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+
+#define CB_SLOTS 64
+
+struct wq_value {
+ struct bpf_wq wq;
+ __u64 cookie;
+};
+
+enum wq_cmd {
+ CMD_INIT_START = 1,
+ CMD_DELETE_INIT,
+ CMD_REPLACE_INIT,
+};
+
+/* Results reset before each command. */
+int res_lookup_err;
+int res_delete_err;
+int res_update_err;
+int res_init_err;
+int res_new_init_err;
+int res_set_err;
+int res_start_err;
+__u64 res_old_addr;
+__u64 res_new_addr;
+u32 cmd;
+u64 cookie;
+u64 cb_count[CB_SLOTS];
+
+static __always_inline void reset_result(void)
+{
+ res_lookup_err = 0;
+ res_delete_err = 0;
+ res_update_err = 0;
+ res_init_err = 0;
+ res_new_init_err = 0;
+ res_set_err = 0;
+ res_start_err = 0;
+ res_old_addr = 0;
+ res_new_addr = 0;
+}
+
+/* Configure and start only a current map member. */
+#define WQ_INIT_START(v, map_sym, cb) \
+ ({ \
+ long __e; \
+ (v)->cookie = cookie; \
+ __e = bpf_wq_init(&(v)->wq, &map_sym, 0); \
+ if (!__e) { \
+ res_set_err = bpf_wq_set_callback(&(v)->wq, cb, 0); \
+ if (!res_set_err) \
+ res_start_err = bpf_wq_start(&(v)->wq, 0); \
+ } \
+ __e; \
+ })
+
+/* Delete and initialize through the retained pointer in one RCU section.
+ * Never configure or start work through that pointer, even if init succeeds.
+ */
+#define WQ_DELETE_INIT(map_sym, v) \
+ ({ \
+ long __e; \
+ bpf_rcu_read_lock(); \
+ res_delete_err = bpf_map_delete_elem(&map_sym, &key); \
+ __e = res_delete_err; \
+ if (!__e) \
+ res_init_err = bpf_wq_init(&(v)->wq, &map_sym, 0); \
+ bpf_rcu_read_unlock(); \
+ __e; \
+ })
+
+#define DEFINE_WQ_MAP(map_sym, value_type) \
+static int map_sym##_cb(void *map, int *key, void *value) \
+{ \
+ value_type *v = value; \
+ \
+ __sync_fetch_and_add(&cb_count[v->cookie & (CB_SLOTS - 1)], 1); \
+ return 0; \
+} \
+ \
+SEC("syscall") \
+int map_sym##_run(void *ctx) \
+{ \
+ value_type *v, *nv, zero = {}; \
+ int key = 0; \
+ \
+ reset_result(); \
+ \
+ switch (cmd) { \
+ case CMD_INIT_START: \
+ v = bpf_map_lookup_elem(&map_sym, &key); \
+ if (!v) { \
+ res_lookup_err = -ENOENT; \
+ break; \
+ } \
+ res_old_addr = (__u64)v; \
+ res_init_err = WQ_INIT_START(v, map_sym, map_sym##_cb); \
+ break; \
+ case CMD_DELETE_INIT: \
+ v = bpf_map_lookup_elem(&map_sym, &key); \
+ if (!v) { \
+ res_lookup_err = -ENOENT; \
+ break; \
+ } \
+ res_old_addr = (__u64)v; \
+ WQ_DELETE_INIT(map_sym, v); \
+ break; \
+ case CMD_REPLACE_INIT: \
+ v = bpf_map_lookup_elem(&map_sym, &key); \
+ if (!v) { \
+ res_lookup_err = -ENOENT; \
+ break; \
+ } \
+ res_old_addr = (__u64)v; \
+ bpf_rcu_read_lock(); \
+ res_update_err = bpf_map_update_elem(&map_sym, &key, &zero, 0); \
+ if (res_update_err) { \
+ bpf_rcu_read_unlock(); \
+ break; \
+ } \
+ nv = bpf_map_lookup_elem(&map_sym, &key); \
+ if (!nv) { \
+ res_lookup_err = -ENOENT; \
+ bpf_rcu_read_unlock(); \
+ break; \
+ } \
+ res_new_addr = (__u64)nv; \
+ res_init_err = bpf_wq_init(&v->wq, &map_sym, 0); \
+ bpf_rcu_read_unlock(); \
+ res_new_init_err = WQ_INIT_START(nv, map_sym, map_sym##_cb); \
+ break; \
+ default: \
+ res_lookup_err = -EINVAL; \
+ break; \
+ } \
+ return 0; \
+}
+
+struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __uint(map_flags, BPF_F_NO_PREALLOC);
+ __uint(max_entries, 4);
+ __type(key, int);
+ __type(value, struct wq_value);
+} values SEC(".maps");
+
+DEFINE_WQ_MAP(values, struct wq_value)
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH bpf-next v2 6/8] selftests/bpf: Check bpf_wq value membership
2026-11-06 9:57 ` [PATCH bpf-next v2 6/8] selftests/bpf: Check bpf_wq value membership Yun Lu
@ 2026-11-06 10:31 ` bot+bpf-ci
2026-11-06 11:26 ` sashiko-bot
1 sibling, 0 replies; 18+ messages in thread
From: bot+bpf-ci @ 2026-11-06 10:31 UTC (permalink / raw)
To: luyun_611, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]
> selftests/bpf: Check bpf_wq value membership
TL;DR: Nit only, in the bpf_wq membership selftest: DEFINE_WQ_MAP() is
instantiated just once, so plain functions with a single callback counter
would be simpler and give clearer verifier logs.
> diff --git a/tools/testing/selftests/bpf/progs/bpf_wq_membership.c b/tools/testing/selftests/bpf/progs/bpf_wq_membership.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bpf_wq_membership.c
[ ... ]
> +#define DEFINE_WQ_MAP(map_sym, value_type) \
> +static int map_sym##_cb(void *map, int *key, void *value) \
> +{ \
> + value_type *v = value; \
> + \
> + __sync_fetch_and_add(&cb_count[v->cookie & (CB_SLOTS - 1)], 1); \
> + return 0; \
> +} \
> + \
> +SEC("syscall") \
> [ ... ]
> +DEFINE_WQ_MAP(values, struct wq_value)
This isn't a bug, but DEFINE_WQ_MAP() is expanded only once, as
DEFINE_WQ_MAP(values, struct wq_value), and every run_cmd() call in
prog_tests/bpf_wq_membership.c passes cookie 1.
Since the macro is only ever instantiated for the one values map, could
values_run() and values_cb() be written as ordinary functions with a single
callback counter, the way progs/task_work_membership.c does?
That would also make verifier logs point at the failing statement instead of
at the DEFINE_WQ_MAP(values, struct wq_value) line.
---
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/42200504781
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH bpf-next v2 6/8] selftests/bpf: Check bpf_wq value membership
2026-11-06 9:57 ` [PATCH bpf-next v2 6/8] selftests/bpf: Check bpf_wq value membership Yun Lu
2026-11-06 10:31 ` bot+bpf-ci
@ 2026-11-06 11:26 ` sashiko-bot
1 sibling, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-11-06 11:26 UTC (permalink / raw)
To: Yun Lu; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] selftests/bpf: multi-line comment style violation in
bpf_wq_membership.c
- [Low] selftests/bpf: single-use macros DEFINE_WQ_MAP and
WQ_DELETE_INIT
--
commit 67a78af8389db430f474e1214b9e087d1c068104
Author: Yun Lu <luyun@kylinos.cn>
selftests/bpf: Check bpf_wq value membership
This commit adds BPF selftests covering bpf_wq initialization on deleted or
replaced map values as well as live HASH and ARRAY map callbacks.
> diff --git a/tools/testing/selftests/bpf/progs/bpf_wq_membership.c b/tools/testing/selftests/bpf/progs/bpf_wq_membership.c
> new file mode 100644
> index 0000000000000..b78b4f1422b3d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bpf_wq_membership.c
[ ... ]
> +/* Delete and initialize through the retained pointer in one RCU section.
> + * Never configure or start work through that pointer, even if init succeeds.
> + */
[Severity: Low]
This isn't a bug, but BPF coding style places the opening /* of a
multi-line comment on its own line.
> +#define WQ_DELETE_INIT(map_sym, v) \
> + ({ \
> + long __e; \
> + bpf_rcu_read_lock(); \
> + res_delete_err = bpf_map_delete_elem(&map_sym, &key); \
> + __e = res_delete_err; \
> + if (!__e) \
> + res_init_err = bpf_wq_init(&(v)->wq, &map_sym, 0); \
> + bpf_rcu_read_unlock(); \
> + __e; \
> + })
> +
> +#define DEFINE_WQ_MAP(map_sym, value_type) \
> +static int map_sym##_cb(void *map, int *key, void *value) \
> +{ \
> + value_type *v = value; \
> + \
> + __sync_fetch_and_add(&cb_count[v->cookie & (CB_SLOTS - 1)], 1); \
> + return 0; \
> +} \
> + \
> +SEC("syscall") \
> +int map_sym##_run(void *ctx) \
> +{ \
[ ... ]
> +DEFINE_WQ_MAP(values, struct wq_value)
[Severity: Low]
This isn't a bug, but DEFINE_WQ_MAP() and WQ_DELETE_INIT() are only
expanded once, and WQ_DELETE_INIT() implicitly captures the caller's local
key variable while returning an unused value. Could values_cb() and
values_run() be written directly as plain functions with WQ_DELETE_INIT()
inlined, and could these subtests be added to
tools/testing/selftests/bpf/prog_tests/wq.c instead of a separate test
file?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261106095747.471718-1-luyun_611@163.com?part=6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH bpf-next v2 7/8] bpf: Check bpf_timer value membership before use
2026-11-06 9:57 [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Yun Lu
` (5 preceding siblings ...)
2026-11-06 9:57 ` [PATCH bpf-next v2 6/8] selftests/bpf: Check bpf_wq value membership Yun Lu
@ 2026-11-06 9:57 ` Yun Lu
2026-11-06 10:31 ` bot+bpf-ci
2026-11-06 9:57 ` [PATCH bpf-next v2 8/8] selftests/bpf: Check bpf_timer value membership Yun Lu
2026-11-06 17:16 ` [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Alexei Starovoitov
8 siblings, 1 reply; 18+ messages in thread
From: Yun Lu @ 2026-11-06 9:57 UTC (permalink / raw)
To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
From: Yun Lu <luyun@kylinos.cn>
bpf_timer_init() can publish a new timer on a value retained across
deletion. The map can still have user references, so its usercnt check
does not reject the removed element. A later callback can outlive it:
reader delete / reclamation softirq
retain value E
unlink E, cancel timer
publish cb, set and arm
leave read-side section
finish E's grace period
enter callback
cancel without waiting
free E
access E
The timer callback runs outside the waiting set of E's reclamation grace
period. Final cancellation does not wait for the running callback, and
keeping the timer's cb alive does not keep E alive.
Extend the shared init and setter membership checks to timers by removing
the WQ-only conditions. Look up the key after publication and compare the
returned pointer with cb->value: either deletion finds cb or lookup
rejects the removed value. Init cancels its local cb and returns -ENOENT
on mismatch. The setter returns -EINVAL, closing the window where another
CPU configures cb before init completes its checks.
Document the errors and update rhash_timer in the same commit. Its old
deleted-value init depended on allocator field preservation; require
-ENOENT instead and retain a live timer roundtrip. Normal cancellation
and restart of a linked timer are unchanged.
Fixes: 1bfbc267ec91 ("bpf: Enable bpf_timer and bpf_wq in any context")
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
include/uapi/linux/bpf.h | 5 +-
kernel/bpf/helpers.c | 6 +-
tools/include/uapi/linux/bpf.h | 5 +-
.../selftests/bpf/prog_tests/rhash_timer.c | 98 ++++++++-----------
.../testing/selftests/bpf/progs/rhash_timer.c | 94 +++++++++++-------
5 files changed, 106 insertions(+), 102 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 62de0919db72..46a102fc0834 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5468,6 +5468,8 @@ union bpf_attr {
* **-EBUSY** if *timer* is already initialized.
* **-EINVAL** if invalid *flags* are passed.
* **-EPERM** if *timer* is in a map that doesn't have any user references.
+ * **-ENOENT** if the current map lookup does not return this value
+ * when the membership check is reached.
* The user space should either hold a file descriptor to a map with timers
* or pin such map in bpffs. When map is unpinned or file descriptor is
* closed all timers in the map will be cancelled and freed.
@@ -5477,7 +5479,8 @@ union bpf_attr {
* Configure the timer to call *callback_fn* static function.
* Return
* 0 on success.
- * **-EINVAL** if *timer* was not initialized with bpf_timer_init() earlier.
+ * **-EINVAL** if *timer* was not initialized with bpf_timer_init() earlier,
+ * or if the value containing it is no longer linked in the map.
* **-EPERM** if *timer* is in a map that doesn't have any user references.
* The user space should either hold a file descriptor to a map with timers
* or pin such map in bpffs. When map is unpinned or file descriptor is
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 7bcb66c92abd..7e91e8502a2e 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1448,8 +1448,7 @@ static int __bpf_async_init(struct bpf_async_kern *async, struct bpf_map *map, u
bpf_async_cancel_cb(async, cb);
return -EPERM;
}
- if (type == BPF_ASYNC_TYPE_WQ &&
- !bpf_map_value_is_linked(map, cb->value)) {
+ if (!bpf_map_value_is_linked(map, cb->value)) {
bpf_async_cancel_cb(async, cb);
return -ENOENT;
}
@@ -1572,8 +1571,7 @@ static int __bpf_async_set_callback(struct bpf_async_kern *async, void *callback
return -EINVAL;
if (!atomic64_read(&map->usercnt))
return -EPERM;
- if (cb->type == BPF_ASYNC_TYPE_WQ &&
- !bpf_map_value_is_linked(map, cb->value))
+ if (!bpf_map_value_is_linked(map, cb->value))
return -EINVAL;
return bpf_async_update_prog_callback(cb, prog, callback_fn);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 62de0919db72..46a102fc0834 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5468,6 +5468,8 @@ union bpf_attr {
* **-EBUSY** if *timer* is already initialized.
* **-EINVAL** if invalid *flags* are passed.
* **-EPERM** if *timer* is in a map that doesn't have any user references.
+ * **-ENOENT** if the current map lookup does not return this value
+ * when the membership check is reached.
* The user space should either hold a file descriptor to a map with timers
* or pin such map in bpffs. When map is unpinned or file descriptor is
* closed all timers in the map will be cancelled and freed.
@@ -5477,7 +5479,8 @@ union bpf_attr {
* Configure the timer to call *callback_fn* static function.
* Return
* 0 on success.
- * **-EINVAL** if *timer* was not initialized with bpf_timer_init() earlier.
+ * **-EINVAL** if *timer* was not initialized with bpf_timer_init() earlier,
+ * or if the value containing it is no longer linked in the map.
* **-EPERM** if *timer* is in a map that doesn't have any user references.
* The user space should either hold a file descriptor to a map with timers
* or pin such map in bpffs. When map is unpinned or file descriptor is
diff --git a/tools/testing/selftests/bpf/prog_tests/rhash_timer.c b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c
index 3aad9fc02e06..a53275bc1175 100644
--- a/tools/testing/selftests/bpf/prog_tests/rhash_timer.c
+++ b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c
@@ -6,9 +6,6 @@
#include <test_progs.h>
#include "rhash_timer.skel.h"
-#define MAX_ATTEMPTS 256
-#define RCU_SYNC_INTERVAL 64
-
static int pin_to_first_cpu(cpu_set_t *old_mask)
{
cpu_set_t new_mask;
@@ -52,10 +49,10 @@ void test_rhash_timer(void)
LIBBPF_OPTS(bpf_test_run_opts, opts);
struct rhash_timer *skel = NULL;
cpu_set_t old_mask;
- int map_fd = -1, arm_fd, cancel_fd;
+ int map_fd = -1, arm_del_fd, arm_live_fd, cancel_live_fd;
bool affinity_set = false;
- __u64 key = 1;
- int attempt, err;
+ __u64 key = 0, deadline;
+ int err;
err = pin_to_first_cpu(&old_mask);
if (!ASSERT_OK(err, "pin_to_first_cpu"))
@@ -69,69 +66,52 @@ void test_rhash_timer(void)
map_fd = bpf_map__fd(skel->maps.timer_map);
if (!ASSERT_GE(map_fd, 0, "timer_map fd"))
goto out;
- arm_fd = bpf_program__fd(skel->progs.arm_deleted_timer);
- if (!ASSERT_GE(arm_fd, 0, "arm_deleted_timer fd"))
+ arm_del_fd = bpf_program__fd(skel->progs.arm_deleted_timer);
+ if (!ASSERT_GE(arm_del_fd, 0, "arm_deleted_timer fd"))
goto out;
- cancel_fd = bpf_program__fd(skel->progs.cancel_recycled_timer);
- if (!ASSERT_GE(cancel_fd, 0, "cancel_recycled_timer fd"))
+ arm_live_fd = bpf_program__fd(skel->progs.live_arm_timer);
+ if (!ASSERT_GE(arm_live_fd, 0, "live_arm_timer fd"))
goto out;
-
- err = update_timer_map(map_fd, key);
- if (!ASSERT_OK(err, "seed_timer_map"))
+ cancel_live_fd = bpf_program__fd(skel->progs.cancel_live_timer);
+ if (!ASSERT_GE(cancel_live_fd, 0, "cancel_live_timer fd"))
goto out;
- for (attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
- err = run_prog(arm_fd, &opts);
- if (err) {
- ASSERT_OK(err, "arm_deleted_timer");
- goto out;
- }
- if (skel->bss->armed != attempt + 1) {
- ASSERT_EQ(skel->bss->armed, attempt + 1, "armed");
- goto out;
- }
- if (skel->bss->timer_init_err) {
- ASSERT_OK(skel->bss->timer_init_err, "timer_init_err");
- goto out;
- }
- if (skel->bss->timer_set_callback_err) {
- ASSERT_OK(skel->bss->timer_set_callback_err,
- "timer_set_callback_err");
+ if (test__start_subtest("deleted_init_rejected")) {
+ err = update_timer_map(map_fd, key);
+ if (!ASSERT_OK(err, "seed_deleted"))
goto out;
- }
- if (skel->bss->timer_start_err) {
- ASSERT_OK(skel->bss->timer_start_err, "timer_start_err");
+ err = run_prog(arm_del_fd, &opts);
+ if (!ASSERT_OK(err, "arm_deleted_timer"))
goto out;
- }
-
- if ((attempt + 1) % RCU_SYNC_INTERVAL == 0) {
- err = kern_sync_rcu();
- if (err) {
- ASSERT_OK(err, "kern_sync_rcu");
- goto out;
- }
- }
-
- err = update_timer_map(map_fd, ++key);
- if (err) {
- ASSERT_OK(err, "replace_timer_map");
- goto out;
- }
+ ASSERT_EQ(skel->bss->timer_init_err, -ENOENT, "deleted_init_errno");
+ }
- err = run_prog(cancel_fd, &opts);
- if (err) {
- ASSERT_OK(err, "cancel_recycled_timer");
+ if (test__start_subtest("live_timer_roundtrip")) {
+ err = update_timer_map(map_fd, key);
+ if (!ASSERT_OK(err, "seed_live"))
goto out;
- }
- if (skel->bss->timer_cancel_err) {
- ASSERT_OK(skel->bss->timer_cancel_err, "timer_cancel_err");
+ err = run_prog(arm_live_fd, &opts);
+ if (!ASSERT_OK(err, "live_arm_timer"))
goto out;
- }
- if (skel->bss->cancelled)
- break;
+ ASSERT_EQ(skel->bss->live_armed, 1, "live_armed");
+ ASSERT_EQ(skel->bss->live_init_err, 0, "live_init_err");
+ ASSERT_EQ(skel->bss->live_set_callback_err, 0,
+ "live_set_callback_err");
+ ASSERT_EQ(skel->bss->live_start_err, 0, "live_start_err");
+
+ deadline = get_time_ns() + 2000000000ULL;
+ while (!__atomic_load_n(&skel->bss->live_cb_cnt, __ATOMIC_ACQUIRE) &&
+ get_time_ns() < deadline)
+ usleep(1000);
+ ASSERT_GT(__atomic_load_n(&skel->bss->live_cb_cnt, __ATOMIC_ACQUIRE),
+ 0, "live_callback");
+
+ err = run_prog(cancel_live_fd, &opts);
+ if (!ASSERT_OK(err, "cancel_live_timer"))
+ goto out;
+ ASSERT_EQ(skel->bss->live_cancel_err, 0, "live_cancel_err");
+ ASSERT_GT(skel->bss->live_cancelled, 0, "live_cancelled");
}
-
- ASSERT_GT(skel->bss->cancelled, 0, "preserved timer");
out:
if (map_fd >= 0)
bpf_map_delete_elem(map_fd, &key);
diff --git a/tools/testing/selftests/bpf/progs/rhash_timer.c b/tools/testing/selftests/bpf/progs/rhash_timer.c
index 2e06a463c605..60793aa8b0fb 100644
--- a/tools/testing/selftests/bpf/progs/rhash_timer.c
+++ b/tools/testing/selftests/bpf/progs/rhash_timer.c
@@ -6,6 +6,7 @@
#define CLOCK_MONOTONIC 1
#define TIMER_NSEC (60ULL * 1000 * 1000 * 1000)
+#define LIVE_TIMER_NSEC 1000ULL
struct timer_value {
struct bpf_timer timer;
@@ -20,18 +21,22 @@ struct {
__type(value, struct timer_value);
} timer_map SEC(".maps");
-u64 armed;
-u64 cancelled;
+u64 live_armed;
+u64 live_cancelled;
+u64 live_cb_cnt;
long timer_init_err;
-long timer_set_callback_err;
-long timer_start_err;
-long timer_cancel_err;
+long live_init_err;
+long live_set_callback_err;
+long live_start_err;
+long live_cancel_err;
static int timer_cb(void *map, u64 *key, struct timer_value *value)
{
+ __sync_fetch_and_add(&live_cb_cnt, 1);
return 0;
}
+/* Initialize through a retained pointer after deleting the element. */
static long arm_timer_cb(struct bpf_map *map, u64 *key,
struct timer_value *value, void *ctx)
{
@@ -43,55 +48,70 @@ static long arm_timer_cb(struct bpf_map *map, u64 *key,
return 1;
err = bpf_timer_init(&value->timer, map, CLOCK_MONOTONIC);
+ timer_init_err = err;
+ /* Do not configure or start the deleted value, even on an unfixed kernel. */
+ return 1;
+}
+
+SEC("syscall")
+int arm_deleted_timer(void *ctx)
+{
+ bpf_for_each_map_elem(&timer_map, arm_timer_cb, NULL, 0);
+ return 0;
+}
+
+/* Arm a live timer; the host waits for its callback before cancellation. */
+SEC("syscall")
+int live_arm_timer(void *ctx)
+{
+ struct timer_value *v;
+ u64 key = 0;
+ long err;
+
+ v = bpf_map_lookup_elem(&timer_map, &key);
+ if (!v)
+ return -EINVAL;
+
+ err = bpf_timer_init(&v->timer, &timer_map, CLOCK_MONOTONIC);
if (err) {
- timer_init_err = err;
- return 1;
+ live_init_err = err;
+ return err;
}
- err = bpf_timer_set_callback(&value->timer, timer_cb);
+ err = bpf_timer_set_callback(&v->timer, timer_cb);
if (err) {
- timer_set_callback_err = err;
- return 1;
+ live_set_callback_err = err;
+ return err;
}
- err = bpf_timer_start(&value->timer, TIMER_NSEC, BPF_F_TIMER_CPU_PIN);
+ err = bpf_timer_start(&v->timer, LIVE_TIMER_NSEC, 0);
if (err) {
- timer_start_err = err;
- return 1;
+ live_start_err = err;
+ return err;
}
- __sync_fetch_and_add(&armed, 1);
- return 1;
+ __sync_fetch_and_add(&live_armed, 1);
+ return 0;
}
-static long cancel_timer_cb(struct bpf_map *map, u64 *key,
- struct timer_value *value, void *ctx)
+SEC("syscall")
+int cancel_live_timer(void *ctx)
{
+ struct timer_value *v;
+ u64 key = 0;
long err;
- err = bpf_timer_cancel(&value->timer);
- if (err == -EINVAL)
- return 1;
+ v = bpf_map_lookup_elem(&timer_map, &key);
+ if (!v)
+ return -EINVAL;
+
+ err = bpf_timer_cancel(&v->timer);
if (err < 0) {
- timer_cancel_err = err;
- return 1;
+ live_cancel_err = err;
+ return err;
}
- __sync_fetch_and_add(&cancelled, 1);
- return 1;
-}
-
-SEC("syscall")
-int arm_deleted_timer(void *ctx)
-{
- bpf_for_each_map_elem(&timer_map, arm_timer_cb, NULL, 0);
- return 0;
-}
-
-SEC("syscall")
-int cancel_recycled_timer(void *ctx)
-{
- bpf_for_each_map_elem(&timer_map, cancel_timer_cb, NULL, 0);
+ __sync_fetch_and_add(&live_cancelled, 1);
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH bpf-next v2 7/8] bpf: Check bpf_timer value membership before use
2026-11-06 9:57 ` [PATCH bpf-next v2 7/8] bpf: Check bpf_timer value membership before use Yun Lu
@ 2026-11-06 10:31 ` bot+bpf-ci
0 siblings, 0 replies; 18+ messages in thread
From: bot+bpf-ci @ 2026-11-06 10:31 UTC (permalink / raw)
To: luyun_611, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3762 bytes --]
> bpf: Check bpf_timer value membership before use
TL;DR:
- Timer membership check may mark LRU hash elements as referenced, so
self-re-arming idle timers could skew eviction
- Nit: new -ENOENT doc line splits the -EPERM text and uses undefined terms
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
[ ... ]
> @@ -5468,6 +5468,8 @@ union bpf_attr {
> * **-EBUSY** if *timer* is already initialized.
> * **-EINVAL** if invalid *flags* are passed.
> * **-EPERM** if *timer* is in a map that doesn't have any user references.
> + * **-ENOENT** if the current map lookup does not return this value
> + * when the membership check is reached.
> * The user space should either hold a file descriptor to a map with timers
> * or pin such map in bpffs. When map is unpinned or file descriptor is
> * closed all timers in the map will be cancelled and freed.
This isn't a bug, but the new -ENOENT entry sits between the -EPERM line and
the paragraph that explains -EPERM ("The user space should either hold a file
descriptor to a map with timers or pin such map in bpffs...").
In the generated bpf-helpers(7) text, that advice now reads as if it explains
-ENOENT.
The wording also relies on terms the helper description never defines ("this
value", "the membership check"). Could the entry be placed before -EPERM and
worded in user terms, for example that the map element containing *timer* was
deleted or replaced?
The same text is copied into tools/include/uapi/linux/bpf.h.
[ ... ]
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
[ ... ]
> @@ -1575,8 +1574,7 @@ static int __bpf_async_set_callback(struct bpf_async_kern *async, void *callback
> return -EINVAL;
> if (!atomic64_read(&map->usercnt))
> return -EPERM;
> - if (cb->type == BPF_ASYNC_TYPE_WQ &&
> - !bpf_map_value_is_linked(map, cb->value))
> + if (!bpf_map_value_is_linked(map, cb->value))
> return -EINVAL;
>
> return bpf_async_update_prog_callback(cb, prog, callback_fn);
With the WQ-only condition removed, every bpf_timer_init() that reaches the
membership check and every bpf_timer_set_callback() now calls
bpf_map_value_is_linked(), which does:
return map->ops->map_lookup_elem(map, key) == value;
For BPF_MAP_TYPE_LRU_HASH, which may hold a BPF_TIMER, map_lookup_elem is
htab_lru_map_lookup_elem() -> __htab_lru_map_lookup_elem(map, key, true),
which calls bpf_lru_node_set_ref() on the element.
Before this patch, bpf_timer_set_callback() had no effect on LRU state.
A timer callback that re-arms itself with bpf_timer_set_callback() +
bpf_timer_start(), as timer_mim.c's timer_cb1/timer_cb2 and timer.c's
rearm() do, would now set the ref bit on its own element every period.
__bpf_lru_list_rotate_inactive() and __bpf_lru_list_shrink_inactive() move
referenced nodes back to the active list instead of evicting them.
Can this make idle entries whose timers re-arm themselves look hot to the
LRU, so that entries that are really in use get evicted first?
The syscall lookup path avoids this side effect by using
map->ops->map_lookup_elem_sys_only (htab_lru_map_lookup_elem_sys, mark=false)
when it is present. Could bpf_map_value_is_linked() use the non-marking
lookup when it exists?
The same helper already serves wq (7b56ce638391) and task work (36948ba24248)
earlier in this series, and no later commit in the series changes this.
---
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/42200504781
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH bpf-next v2 8/8] selftests/bpf: Check bpf_timer value membership
2026-11-06 9:57 [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Yun Lu
` (6 preceding siblings ...)
2026-11-06 9:57 ` [PATCH bpf-next v2 7/8] bpf: Check bpf_timer value membership before use Yun Lu
@ 2026-11-06 9:57 ` Yun Lu
2026-11-06 10:31 ` bot+bpf-ci
2026-11-06 11:51 ` sashiko-bot
2026-11-06 17:16 ` [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Alexei Starovoitov
8 siblings, 2 replies; 18+ messages in thread
From: Yun Lu @ 2026-11-06 9:57 UTC (permalink / raw)
To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
From: Yun Lu <luyun@kylinos.cn>
Add seven subtests to distinguish removed-value rejection from normal
bpf_timer operation: delete from HASH with and without preallocation,
RHASH and LRU HASH; replace a HASH element; and run live HASH/ARRAY timers.
Delete or replace the element while retaining its old value pointer in
the same read-side section, then require init through that pointer to
return -ENOENT. Stop after recording the stale init result even if an
unfixed kernel returns success; do not configure or arm that object.
For replacement, assert that the old and new addresses differ and verify
a callback from the new member. The two live cases also initialize, set
and arm a timer, then wait for an actual callback. This checks that the
new rejection path leaves linked values usable.
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
.../bpf/prog_tests/bpf_timer_membership.c | 199 ++++++++++++++++++
.../bpf/progs/bpf_timer_membership.c | 161 ++++++++++++++
2 files changed, 360 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_timer_membership.c
create mode 100644 tools/testing/selftests/bpf/progs/bpf_timer_membership.c
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_timer_membership.c b/tools/testing/selftests/bpf/prog_tests/bpf_timer_membership.c
new file mode 100644
index 000000000000..b0b880ba71a7
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_timer_membership.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Corporation. */
+
+/* Five stale-init cases and two live callback cases. */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <unistd.h>
+
+#include <test_progs.h>
+#include "bpf_timer_membership.skel.h"
+
+#define CB_SLOTS 64
+#define CB_WAIT_ITERS 3000
+
+enum {
+ CMD_INIT_START = 1,
+ CMD_DELETE_INIT,
+ CMD_REPLACE_INIT,
+};
+
+struct timer_case {
+ struct bpf_timer_membership *skel;
+ struct bpf_map *map;
+ struct bpf_program *prog;
+ bool array;
+ int fd;
+};
+
+static struct timer_case open_case(enum bpf_map_type type, __u32 flags)
+{
+ struct timer_case c = {};
+ int err;
+
+ c.skel = bpf_timer_membership__open();
+ if (!ASSERT_OK_PTR(c.skel, "open"))
+ return c;
+ c.map = c.skel->maps.values;
+ c.prog = c.skel->progs.values_run;
+ c.array = type == BPF_MAP_TYPE_ARRAY;
+ err = bpf_map__set_type(c.map, type);
+ if (!ASSERT_OK(err, "set_type"))
+ goto fail;
+ err = bpf_map__set_map_flags(c.map, flags);
+ if (!ASSERT_OK(err, "set_flags"))
+ goto fail;
+ err = bpf_timer_membership__load(c.skel);
+ if (!ASSERT_OK(err, "load"))
+ goto fail;
+ c.fd = bpf_map__fd(c.map);
+ if (!ASSERT_GE(c.fd, 0, "map_fd"))
+ goto fail;
+ return c;
+fail:
+ bpf_timer_membership__destroy(c.skel);
+ c.skel = NULL;
+ return c;
+}
+
+static int run_cmd(struct timer_case *c, u32 cmd, u64 cookie)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, opts);
+ int err;
+
+ c->skel->bss->cmd = cmd;
+ c->skel->bss->cookie = cookie;
+ err = bpf_prog_test_run_opts(bpf_program__fd(c->prog), &opts);
+ if (!ASSERT_OK(err, "test_run"))
+ return err;
+ if (!ASSERT_OK((int)opts.retval, "prog_retval"))
+ return -EINVAL;
+ return 0;
+}
+
+static u64 cb_count(struct timer_case *c, u64 cookie)
+{
+ return __atomic_load_n(&c->skel->bss->cb_count[cookie & (CB_SLOTS - 1)],
+ __ATOMIC_ACQUIRE);
+}
+
+static bool wait_cb(struct timer_case *c, u64 cookie, u64 want)
+{
+ int i;
+
+ for (i = 0; i < CB_WAIT_ITERS; i++) {
+ if (cb_count(c, cookie) >= want)
+ return ASSERT_EQ(cb_count(c, cookie), want, "callback_count");
+ usleep(1000);
+ }
+ return ASSERT_EQ(cb_count(c, cookie), want, "callback_timeout");
+}
+
+static bool seed(struct timer_case *c)
+{
+ size_t size = bpf_map__value_size(c->map);
+ void *val = calloc(1, size);
+ int key = 0, err;
+
+ if (!ASSERT_OK_PTR(val, "calloc"))
+ return false;
+ err = bpf_map_update_elem(c->fd, &key, val, BPF_ANY);
+ free(val);
+ return ASSERT_OK(err, "insert");
+}
+
+static void drop_elem(struct timer_case *c)
+{
+ int key = 0;
+
+ if (c->array)
+ return;
+ bpf_map_delete_elem(c->fd, &key);
+}
+
+static bool check_init_start(struct timer_case *c)
+{
+ return ASSERT_OK(c->skel->bss->res_lookup_err, "lookup_err") &&
+ ASSERT_OK(c->skel->bss->res_init_err, "init_err") &&
+ ASSERT_OK(c->skel->bss->res_set_err, "set_err") &&
+ ASSERT_OK(c->skel->bss->res_start_err, "start_err");
+}
+
+static void subtest_live(enum bpf_map_type type, __u32 flags)
+{
+ struct timer_case c = open_case(type, flags);
+
+ if (!c.skel)
+ return;
+ if (!seed(&c))
+ goto out;
+ if (run_cmd(&c, CMD_INIT_START, 1))
+ goto out;
+ if (!check_init_start(&c))
+ goto out;
+ wait_cb(&c, 1, 1);
+out:
+ drop_elem(&c);
+ bpf_timer_membership__destroy(c.skel);
+}
+
+static void subtest_delete(enum bpf_map_type type, __u32 flags)
+{
+ struct timer_case c = open_case(type, flags);
+
+ if (!c.skel)
+ return;
+ if (!seed(&c))
+ goto out;
+ if (run_cmd(&c, CMD_DELETE_INIT, 1))
+ goto out;
+ ASSERT_OK(c.skel->bss->res_lookup_err, "lookup_err");
+ ASSERT_OK(c.skel->bss->res_delete_err, "delete_err");
+ ASSERT_EQ(c.skel->bss->res_init_err, -ENOENT, "init_err");
+ ASSERT_EQ(cb_count(&c, 1), 0, "no_callback");
+out:
+ bpf_timer_membership__destroy(c.skel);
+}
+
+static void subtest_replace(void)
+{
+ struct timer_case c = open_case(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC);
+
+ if (!c.skel)
+ return;
+ if (!seed(&c))
+ goto out;
+ if (run_cmd(&c, CMD_REPLACE_INIT, 1))
+ goto out;
+ ASSERT_OK(c.skel->bss->res_lookup_err, "lookup_err");
+ ASSERT_OK(c.skel->bss->res_update_err, "update_err");
+ ASSERT_NEQ(c.skel->bss->res_old_addr, c.skel->bss->res_new_addr,
+ "distinct_address");
+ ASSERT_EQ(c.skel->bss->res_init_err, -ENOENT, "init_err");
+ ASSERT_OK(c.skel->bss->res_new_init_err, "new_init_err");
+ ASSERT_OK(c.skel->bss->res_set_err, "set_err");
+ ASSERT_OK(c.skel->bss->res_start_err, "start_err");
+ wait_cb(&c, 1, 1);
+out:
+ drop_elem(&c);
+ bpf_timer_membership__destroy(c.skel);
+}
+
+void test_bpf_timer_membership(void)
+{
+ if (test__start_subtest("live_hash_np"))
+ subtest_live(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC);
+ if (test__start_subtest("live_array"))
+ subtest_live(BPF_MAP_TYPE_ARRAY, 0);
+ if (test__start_subtest("delete_hash_np"))
+ subtest_delete(BPF_MAP_TYPE_HASH, BPF_F_NO_PREALLOC);
+ if (test__start_subtest("delete_hash_pre"))
+ subtest_delete(BPF_MAP_TYPE_HASH, 0);
+ if (test__start_subtest("delete_rhash"))
+ subtest_delete(BPF_MAP_TYPE_RHASH, BPF_F_NO_PREALLOC);
+ if (test__start_subtest("delete_lru"))
+ subtest_delete(BPF_MAP_TYPE_LRU_HASH, 0);
+ if (test__start_subtest("replace_hash_np"))
+ subtest_replace();
+}
diff --git a/tools/testing/selftests/bpf/progs/bpf_timer_membership.c b/tools/testing/selftests/bpf/progs/bpf_timer_membership.c
new file mode 100644
index 000000000000..b7a10a80f2cf
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpf_timer_membership.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Corporation. */
+
+/* Run one command per test_run; stale-pointer commands only record init. */
+
+#include <vmlinux.h>
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+
+#define CB_SLOTS 64
+#define CLOCK_MONOTONIC 1
+#define TIMER_NS 1000000ULL
+
+struct timer_value {
+ struct bpf_timer timer;
+ __u64 cookie;
+};
+
+enum timer_cmd {
+ CMD_INIT_START = 1,
+ CMD_DELETE_INIT,
+ CMD_REPLACE_INIT,
+};
+
+/* Results reset before each command. */
+int res_lookup_err;
+int res_delete_err;
+int res_update_err;
+int res_init_err;
+int res_new_init_err;
+int res_set_err;
+int res_start_err;
+__u64 res_old_addr;
+__u64 res_new_addr;
+u32 cmd;
+u64 cookie;
+u64 cb_count[CB_SLOTS];
+
+static __always_inline void reset_result(void)
+{
+ res_lookup_err = 0;
+ res_delete_err = 0;
+ res_update_err = 0;
+ res_init_err = 0;
+ res_new_init_err = 0;
+ res_set_err = 0;
+ res_start_err = 0;
+ res_old_addr = 0;
+ res_new_addr = 0;
+}
+
+/* Configure and start only a current map member. */
+#define TIMER_INIT_START(v, map_sym, cb) \
+ ({ \
+ long __e; \
+ (v)->cookie = cookie; \
+ __e = bpf_timer_init(&(v)->timer, &map_sym, CLOCK_MONOTONIC); \
+ if (!__e) { \
+ res_set_err = bpf_timer_set_callback(&(v)->timer, cb); \
+ if (!res_set_err) \
+ res_start_err = bpf_timer_start(&(v)->timer, \
+ TIMER_NS, 0); \
+ } \
+ __e; \
+ })
+
+/* Delete and initialize through the retained pointer in one RCU section.
+ * Never configure or start a timer through that pointer, even if init succeeds.
+ */
+#define TIMER_DELETE_INIT(map_sym, v) \
+ ({ \
+ long __e; \
+ bpf_rcu_read_lock(); \
+ res_delete_err = bpf_map_delete_elem(&map_sym, &key); \
+ __e = res_delete_err; \
+ if (!__e) \
+ res_init_err = bpf_timer_init(&(v)->timer, &map_sym, \
+ CLOCK_MONOTONIC); \
+ bpf_rcu_read_unlock(); \
+ __e; \
+ })
+
+#define DEFINE_TIMER_MAP(map_sym, value_type) \
+static int map_sym##_cb(void *map, int *key, void *value) \
+{ \
+ value_type *v = value; \
+ \
+ __sync_fetch_and_add(&cb_count[v->cookie & (CB_SLOTS - 1)], 1); \
+ return 0; \
+} \
+ \
+SEC("syscall") \
+int map_sym##_run(void *ctx) \
+{ \
+ value_type *v, *nv, zero = {}; \
+ int key = 0; \
+ \
+ reset_result(); \
+ \
+ switch (cmd) { \
+ case CMD_INIT_START: \
+ v = bpf_map_lookup_elem(&map_sym, &key); \
+ if (!v) { \
+ res_lookup_err = -ENOENT; \
+ break; \
+ } \
+ res_old_addr = (__u64)v; \
+ res_init_err = TIMER_INIT_START(v, map_sym, map_sym##_cb); \
+ break; \
+ case CMD_DELETE_INIT: \
+ v = bpf_map_lookup_elem(&map_sym, &key); \
+ if (!v) { \
+ res_lookup_err = -ENOENT; \
+ break; \
+ } \
+ res_old_addr = (__u64)v; \
+ TIMER_DELETE_INIT(map_sym, v); \
+ break; \
+ case CMD_REPLACE_INIT: \
+ v = bpf_map_lookup_elem(&map_sym, &key); \
+ if (!v) { \
+ res_lookup_err = -ENOENT; \
+ break; \
+ } \
+ res_old_addr = (__u64)v; \
+ bpf_rcu_read_lock(); \
+ res_update_err = bpf_map_update_elem(&map_sym, &key, &zero, 0); \
+ if (res_update_err) { \
+ bpf_rcu_read_unlock(); \
+ break; \
+ } \
+ nv = bpf_map_lookup_elem(&map_sym, &key); \
+ if (!nv) { \
+ res_lookup_err = -ENOENT; \
+ bpf_rcu_read_unlock(); \
+ break; \
+ } \
+ res_new_addr = (__u64)nv; \
+ res_init_err = bpf_timer_init(&v->timer, &map_sym, \
+ CLOCK_MONOTONIC); \
+ bpf_rcu_read_unlock(); \
+ res_new_init_err = TIMER_INIT_START(nv, map_sym, map_sym##_cb); \
+ break; \
+ default: \
+ res_lookup_err = -EINVAL; \
+ break; \
+ } \
+ return 0; \
+}
+
+struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __uint(map_flags, BPF_F_NO_PREALLOC);
+ __uint(max_entries, 4);
+ __type(key, int);
+ __type(value, struct timer_value);
+} values SEC(".maps");
+
+DEFINE_TIMER_MAP(values, struct timer_value)
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH bpf-next v2 8/8] selftests/bpf: Check bpf_timer value membership
2026-11-06 9:57 ` [PATCH bpf-next v2 8/8] selftests/bpf: Check bpf_timer value membership Yun Lu
@ 2026-11-06 10:31 ` bot+bpf-ci
2026-11-06 11:51 ` sashiko-bot
1 sibling, 0 replies; 18+ messages in thread
From: bot+bpf-ci @ 2026-11-06 10:31 UTC (permalink / raw)
To: luyun_611, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2887 bytes --]
> selftests/bpf: Check bpf_timer value membership
TL;DR: Test-only nits, no bug found: the new bpf_timer membership selftest
nearly duplicates the bpf_wq one, and its single-use macros could be plain
functions or shared code.
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_timer_membership.c b/tools/testing/selftests/bpf/prog_tests/bpf_timer_membership.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_timer_membership.c
[ ... ]
> +struct timer_case {
> + struct bpf_timer_membership *skel;
> + struct bpf_map *map;
> + struct bpf_program *prog;
> + bool array;
> + int fd;
> +};
> +
> +static struct timer_case open_case(enum bpf_map_type type, __u32 flags)
> +{
> + struct timer_case c = {};
> + int err;
> +
> + c.skel = bpf_timer_membership__open();
This isn't a bug, but this file matches prog_tests/bpf_wq_membership.c
except for the renames (wq_case/timer_case, the skeleton names and the
test entry point). open_case(), run_cmd(), cb_count(), wait_cb(), seed(),
drop_elem(), check_init_start(), subtest_live(), subtest_delete() and
subtest_replace() are the same in both.
Could the two tests share one host-side driver, for example one skeleton
with a timer map and a wq map, or a shared header, instead of two copies?
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/bpf_timer_membership.c b/tools/testing/selftests/bpf/progs/bpf_timer_membership.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bpf_timer_membership.c
[ ... ]
> +#define DEFINE_TIMER_MAP(map_sym, value_type) \
> +static int map_sym##_cb(void *map, int *key, void *value) \
> +{ \
> + value_type *v = value; \
> + \
> + __sync_fetch_and_add(&cb_count[v->cookie & (CB_SLOTS - 1)], 1); \
> + return 0; \
> +} \
> + \
> +SEC("syscall") \
> +int map_sym##_run(void *ctx) \
[ ... ]
This isn't a bug, but DEFINE_TIMER_MAP is expanded only once, as
DEFINE_TIMER_MAP(values, struct timer_value), and TIMER_DELETE_INIT is
also used only once. The userspace side switches the map type with
bpf_map__set_type(), so a single map is all the program needs.
The result is a program body of about 75 lines of backslash-continued
macro text, and TIMER_DELETE_INIT quietly depends on a local named key in
the caller. The file is also nearly line-for-line the same as
progs/bpf_wq_membership.c.
Since DEFINE_TIMER_MAP is expanded only once, could values_cb() and
values_run() be ordinary functions? Alternatively, could the shared
delete/replace logic go in a header used by both this file and
bpf_wq_membership.c?
---
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/42200504781
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH bpf-next v2 8/8] selftests/bpf: Check bpf_timer value membership
2026-11-06 9:57 ` [PATCH bpf-next v2 8/8] selftests/bpf: Check bpf_timer value membership Yun Lu
2026-11-06 10:31 ` bot+bpf-ci
@ 2026-11-06 11:51 ` sashiko-bot
1 sibling, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-11-06 11:51 UTC (permalink / raw)
To: Yun Lu; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] selftests/bpf: non-standard multi-line comment style in
bpf_timer_membership.c
- [Low] selftests/bpf: unnecessary macros and duplication in
bpf_timer_membership
--
commit 62751ca885fba4775d346fbce832c200968a6cd0
Author: Yun Lu <luyun@kylinos.cn>
selftests/bpf: Check bpf_timer value membership
This commit adds BPF selftests verifying that bpf_timer_init() rejects
deleted or replaced map values with -ENOENT across hash, rhash, and LRU
hash maps while keeping live hash and array timers functional.
> diff --git a/tools/testing/selftests/bpf/progs/bpf_timer_membership.c b/tools/testing/selftests/bpf/progs/bpf_timer_membership.c
> new file mode 100644
> index 0000000000000..b7a10a80f2cf5
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bpf_timer_membership.c
> @@ -0,0 +1,161 @@
[ ... ]
> +/* Configure and start only a current map member. */
> +#define TIMER_INIT_START(v, map_sym, cb) \
> + ({ \
> + long __e; \
> + (v)->cookie = cookie; \
> + __e = bpf_timer_init(&(v)->timer, &map_sym, CLOCK_MONOTONIC); \
> + if (!__e) { \
> + res_set_err = bpf_timer_set_callback(&(v)->timer, cb); \
> + if (!res_set_err) \
> + res_start_err = bpf_timer_start(&(v)->timer, \
> + TIMER_NS, 0); \
> + } \
> + __e; \
> + })
> +
> +/* Delete and initialize through the retained pointer in one RCU section.
> + * Never configure or start a timer through that pointer, even if init succeeds.
> + */
[Severity: Low]
This isn't a bug, but BPF coding style places the opening /* of a
multi-line comment on its own line with the comment text starting on the
next line.
> +#define TIMER_DELETE_INIT(map_sym, v) \
> + ({ \
> + long __e; \
> + bpf_rcu_read_lock(); \
> + res_delete_err = bpf_map_delete_elem(&map_sym, &key); \
> + __e = res_delete_err; \
> + if (!__e) \
> + res_init_err = bpf_timer_init(&(v)->timer, &map_sym, \
> + CLOCK_MONOTONIC); \
> + bpf_rcu_read_unlock(); \
> + __e; \
> + })
> +
> +#define DEFINE_TIMER_MAP(map_sym, value_type) \
> +static int map_sym##_cb(void *map, int *key, void *value) \
> +{ \
> + value_type *v = value; \
> + \
> + __sync_fetch_and_add(&cb_count[v->cookie & (CB_SLOTS - 1)], 1); \
> + return 0; \
> +} \
> + \
> +SEC("syscall") \
> +int map_sym##_run(void *ctx) \
> +{ \
> + value_type *v, *nv, zero = {}; \
> + int key = 0; \
[ ... ]
> +struct {
> + __uint(type, BPF_MAP_TYPE_HASH);
> + __uint(map_flags, BPF_F_NO_PREALLOC);
> + __uint(max_entries, 4);
> + __type(key, int);
> + __type(value, struct timer_value);
> +} values SEC(".maps");
> +
> +DEFINE_TIMER_MAP(values, struct timer_value)
[Severity: Low]
This isn't a bug, but DEFINE_TIMER_MAP() is only instantiated once for
values, TIMER_DELETE_INIT() is a single-use macro that implicitly captures
key from its caller and returns an unused value, and TIMER_INIT_START()
could be a static __always_inline function. In addition, cb_count has 64
slots even though every subtest opens a fresh skeleton and passes cookie =
1.
More broadly, prog_tests/bpf_timer_membership.c and
progs/bpf_timer_membership.c duplicate bpf_wq_membership.c from commit
"selftests/bpf: Check bpf_wq value membership" almost verbatim, and the
delete_rhash subtest overlaps with deleted_init_rejected in
prog_tests/rhash_timer.c from commit "bpf: Check bpf_timer value
membership before use". Replacing the single-use macros with regular
functions and consolidating the shared wq and timer membership test logic
would avoid this duplication.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261106095747.471718-1-luyun_611@163.com?part=8
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion
2026-11-06 9:57 [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Yun Lu
` (7 preceding siblings ...)
2026-11-06 9:57 ` [PATCH bpf-next v2 8/8] selftests/bpf: Check bpf_timer value membership Yun Lu
@ 2026-11-06 17:16 ` Alexei Starovoitov
2026-11-09 9:50 ` luyun
8 siblings, 1 reply; 18+ messages in thread
From: Alexei Starovoitov @ 2026-11-06 17:16 UTC (permalink / raw)
To: Yun Lu, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
On Fri, Nov 06, 2026 at 01:57 AM Yun Lu <luyun_611@163.com> wrote:
> Following Alexei's suggestion [2], check whether the map lookup still
> returns the value after publishing the context. For wq and timers, check
> both init and the setter, since another CPU can configure the published
> cb before init finishes its checks.
can we do the check once, in __bpf_async_set_callback() ?
Only the setter installs cb->prog and cb->callback_fn.
bpf_timer_start() and bpf_wq_start() return -EINVAL when cb->prog is NULL.
Nothing runs on a deleted element when the setter rejects it.
The three options for the place of the check:
check in result
--------------- ----------------------------------------------------
init another cpu sets callback_fn before the check
init and setter 2 lookups, bpf_async_cancel_cb() for the failed init
setter 1 lookup, an idle cb stays in the deleted element
The idle cb is like the idle ctx of task work in patch 2.
It stays until the element is reused or freed. That's the case today too.
__bpf_async_init() doesn't change.
rhash_timer keeps its check of the reused element.
The diff replaces patches 5 and 7 and most of patch 4.
bpf_map_value_is_linked() is the one from patch 2 without guard(rcu).
The diff doesn't have the reorder of cb->map = NULL and the detach.
It also lacks the -ENOENT line in the uapi doc and the rhash_timer update.
Something like this (not even compile tested):
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1151,6 +1151,14 @@ static void *map_key_from_value(struct bpf_map *map, void *value, u32 *arr_idx)
return (void *)value - round_up(map->key_size, 8);
}
+/* Is the element that holds this value still in the map? */
+static bool bpf_map_value_is_linked(struct bpf_map *map, void *value)
+{
+ u32 idx;
+
+ return map->ops->map_lookup_elem(map, map_key_from_value(map, value, &idx)) == value;
+}
+
enum bpf_async_type {
BPF_ASYNC_TYPE_TIMER = 0,
BPF_ASYNC_TYPE_WQ,
@@ -1544,11 +1552,25 @@ static int __bpf_async_set_callback(struct bpf_async_kern *async, void *callback
struct bpf_prog *prog)
{
struct bpf_async_cb *cb;
+ struct bpf_map *map;
cb = READ_ONCE(async->cb);
if (!cb)
return -EINVAL;
+ /*
+ * The prog can still hold a value of an element that was deleted from the map.
+ * bpf_async_cancel_and_free() is done with it and nothing stops a new callback
+ * when the element is freed. cb is published at this point: either the lookup
+ * doesn't find the element or bpf_async_cancel_and_free() finds cb.
+ * Nothing runs without callback_fn. An idle cb stays in the element until the
+ * element is reused or freed.
+ */
+ smp_mb();
+ map = READ_ONCE(cb->map);
+ if (!map || !bpf_map_value_is_linked(map, cb->value))
+ return -ENOENT;
+
return bpf_async_update_prog_callback(cb, prog, callback_fn);
}
@@ -1739,9 +1761,7 @@ static void bpf_async_cancel_and_free(struct bpf_async_kern *async)
{
struct bpf_async_cb *cb;
- if (!READ_ONCE(async->cb))
- return;
-
+ /* xchg() orders the unlink of the element before the read of async->cb. */
cb = xchg(&async->cb, NULL);
if (!cb)
return;
[...]
> 11 files changed, 1476 insertions(+), 117 deletions(-)
Way too much for selftest.
Shrink it 10x.
pw-bot: cr
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion
2026-11-06 17:16 ` [PATCH bpf-next v2 0/8] bpf: Fix async callback use-after-free on map deletion Alexei Starovoitov
@ 2026-11-09 9:50 ` luyun
0 siblings, 0 replies; 18+ messages in thread
From: luyun @ 2026-11-09 9:50 UTC (permalink / raw)
To: Alexei Starovoitov, daniel, andrii, eddyz87, memxor, martin.lau,
song, yonghong.song, jolsa, emil, ihor.solodrai
Cc: yatsenko, bpf
Hi Alexei,
Thanks for the review and the example. Sorry for overcomplicating the fix
and adding so many repetitive tests.
I'll rework the next version based on your example: leave __bpf_async_init()
unchanged and check membership only in the shared setter. I'll also reuse
the existing selftests and reduce the added test code.
---
Thanks,
Yun Lu
在 2026/11/7 01:16, Alexei Starovoitov 写道:
> On Fri, Nov 06, 2026 at 01:57 AM Yun Lu <luyun_611@163.com> wrote:
>> Following Alexei's suggestion [2], check whether the map lookup still
>> returns the value after publishing the context. For wq and timers, check
>> both init and the setter, since another CPU can configure the published
>> cb before init finishes its checks.
> can we do the check once, in __bpf_async_set_callback() ?
> Only the setter installs cb->prog and cb->callback_fn.
> bpf_timer_start() and bpf_wq_start() return -EINVAL when cb->prog is NULL.
> Nothing runs on a deleted element when the setter rejects it.
> The three options for the place of the check:
>
> check in result
> --------------- ----------------------------------------------------
> init another cpu sets callback_fn before the check
> init and setter 2 lookups, bpf_async_cancel_cb() for the failed init
> setter 1 lookup, an idle cb stays in the deleted element
>
> The idle cb is like the idle ctx of task work in patch 2.
> It stays until the element is reused or freed. That's the case today too.
> __bpf_async_init() doesn't change.
> rhash_timer keeps its check of the reused element.
>
> The diff replaces patches 5 and 7 and most of patch 4.
> bpf_map_value_is_linked() is the one from patch 2 without guard(rcu).
> The diff doesn't have the reorder of cb->map = NULL and the detach.
> It also lacks the -ENOENT line in the uapi doc and the rhash_timer update.
>
> Something like this (not even compile tested):
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1151,6 +1151,14 @@ static void *map_key_from_value(struct bpf_map *map, void *value, u32 *arr_idx)
> return (void *)value - round_up(map->key_size, 8);
> }
>
> +/* Is the element that holds this value still in the map? */
> +static bool bpf_map_value_is_linked(struct bpf_map *map, void *value)
> +{
> + u32 idx;
> +
> + return map->ops->map_lookup_elem(map, map_key_from_value(map, value, &idx)) == value;
> +}
> +
> enum bpf_async_type {
> BPF_ASYNC_TYPE_TIMER = 0,
> BPF_ASYNC_TYPE_WQ,
> @@ -1544,11 +1552,25 @@ static int __bpf_async_set_callback(struct bpf_async_kern *async, void *callback
> struct bpf_prog *prog)
> {
> struct bpf_async_cb *cb;
> + struct bpf_map *map;
>
> cb = READ_ONCE(async->cb);
> if (!cb)
> return -EINVAL;
>
> + /*
> + * The prog can still hold a value of an element that was deleted from the map.
> + * bpf_async_cancel_and_free() is done with it and nothing stops a new callback
> + * when the element is freed. cb is published at this point: either the lookup
> + * doesn't find the element or bpf_async_cancel_and_free() finds cb.
> + * Nothing runs without callback_fn. An idle cb stays in the element until the
> + * element is reused or freed.
> + */
> + smp_mb();
> + map = READ_ONCE(cb->map);
> + if (!map || !bpf_map_value_is_linked(map, cb->value))
> + return -ENOENT;
> +
> return bpf_async_update_prog_callback(cb, prog, callback_fn);
> }
>
> @@ -1739,9 +1761,7 @@ static void bpf_async_cancel_and_free(struct bpf_async_kern *async)
> {
> struct bpf_async_cb *cb;
>
> - if (!READ_ONCE(async->cb))
> - return;
> -
> + /* xchg() orders the unlink of the element before the read of async->cb. */
> cb = xchg(&async->cb, NULL);
> if (!cb)
> return;
>
> [...]
>
>> 11 files changed, 1476 insertions(+), 117 deletions(-)
> Way too much for selftest.
> Shrink it 10x.
>
> pw-bot: cr
^ permalink raw reply [flat|nested] 18+ messages in thread