BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] bpf: Fix task work round ownership race
@ 2026-09-18  8:00 Yun Lu
  2026-09-18  8:00 ` [PATCH bpf-next 1/2] bpf: Fix task work round ownership during cancellation Yun Lu
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Yun Lu @ 2026-09-18  8:00 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai
  Cc: bpf

From: Yun Lu <luyun@kylinos.cn>

This series fixes a race in the bpf_task_work scheduling kfuncs and
adds a regression test for it.

A task_work callback scheduled for a task running on another CPU can
complete before bpf_task_work_irq() regains control after
task_work_add().  The callback's cleanup releases ctx->task, so a
concurrent map value deletion that publishes the FREED state makes the
resumed irq_work handler call task_work_cancel(NULL, &ctx->work),
dereferencing task_struct::task_works through a NULL task (RIP at
task_work_cancel+0xd, RDI == 0, CR2 at the task_works offset).  The
same cancellation is reached when the deletion wins before the
callback runs and the callback's bailout path drops the last ctx
refcount, clearing ctx->task.

Patch 1 gives each scheduling round an ownership count: the irq_work
scheduler, the published callback and an asynchronous canceller hold a
reference while using this round's task/prog/work, and only the last
user releases them, before the ctx can be reused from STANDBY.  The
count is zero based with the STANDBY -> PENDING transition acting as
the zero-to-one gate, so a finished round cannot be revived: a
canceller either pins a still-active round (ctx->task guaranteed
valid) or declines to cancel.  Separate irq_work objects for
scheduling, cancellation and deferred destruction avoid reinitializing
an irq_work that may still be queued or running.  FREED remains
terminal and the scheduling path stays atomic-only, preserving NMI
safety.

The race window between task_work_add() and the SCHEDULING ->
SCHEDULED cmpxchg is only a few instructions wide, so the fix was
verified with a deterministic reproduction that widens exactly this
window with a debug delay, while scheduling cross-CPU and deleting the
map value concurrently: without the fix the kernel panics reliably;
with it, all interleavings complete.  A 900-round stress test rotating
through ctx reuse, deletion after the callback and deletion right
after scheduling also runs clean, with no leaks reported by kmemleak.

Patch 2 adds a selftest that keeps steady pressure on the
interleaving: it schedules cross-CPU, deletes the map value at
different points of a round, uses READY/DONE handshakes so scheduling
errors cannot be missed, and tags every callback with a generation so
a late callback cannot satisfy a later round's assertions.

Yun Lu (2):
  bpf: Fix task work round ownership during cancellation
  selftests/bpf: Add task work round ownership race test

 kernel/bpf/helpers.c                                  | 123 ++++++++++--
 .../selftests/bpf/prog_tests/test_task_work.c         | 283 +++++++++++++++++++++
 .../selftests/bpf/progs/task_work_race.c              | 125 ++++++++
 3 files changed, 503 insertions(+), 28 deletions(-)

-- 
2.43.0


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

* [PATCH bpf-next 1/2] bpf: Fix task work round ownership during cancellation
  2026-09-18  8:00 [PATCH bpf-next 0/2] bpf: Fix task work round ownership race Yun Lu
@ 2026-09-18  8:00 ` Yun Lu
  2026-09-18 17:54   ` Alexei Starovoitov
  2026-09-18  8:00 ` [PATCH bpf-next 2/2] selftests/bpf: Add task work round ownership race test Yun Lu
  2026-09-18 16:50 ` [PATCH bpf-next 0/2] bpf: Fix task work round ownership race Mykyta Yatsenko
  2 siblings, 1 reply; 9+ messages in thread
From: Yun Lu @ 2026-09-18  8:00 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai
  Cc: bpf

From: Yun Lu <luyun@kylinos.cn>

bpf_task_work_irq() reacts to observing BPF_TW_FREED after a successful
task_work_add() by calling bpf_task_work_cancel(), which dereferences
ctx->task unconditionally.  Nothing currently orders that dereference
against bpf_task_work_ctx_reset(), so it can pass NULL to
task_work_cancel().

The race requires the target task to run concurrently with the irq_work
scheduler, for example on a different CPU.  This is a supported use of
the bpf_task_work_schedule_*() kfuncs.  One concrete interleaving is:

  1. CPU 0 executes bpf_task_work_irq() and publishes ctx->work with
     task_work_add(), but has not yet attempted the
     SCHEDULING-to-SCHEDULED transition.

  2. The target task on CPU 1 returns to userspace and runs the callback.
     It changes SCHEDULING to RUNNING, executes the BPF subprogram, then
     bpf_task_work_ctx_reset() releases ctx->task and sets it to NULL.
     The callback changes RUNNING to STANDBY and drops its ctx reference.

  3. CPU 2 deletes the map value.  bpf_task_work_cancel_and_free()
     changes STANDBY to FREED.  Since the old state is not SCHEDULED, it
     does not queue cancellation and drops the map's ctx reference.

  4. CPU 0 resumes.  Its SCHEDULING-to-SCHEDULED cmpxchg observes FREED,
     so bpf_task_work_cancel() calls
     task_work_cancel(NULL, &ctx->work).

If deletion changes the state to FREED before the callback runs, the
callback instead takes its FREED exit and can release the last ctx
reference.  The original destroy path then resets ctx->task, producing
the same NULL dereference when CPU 0 resumes.

A controlled reproducer that delays CPU 0 between task_work_add() and
the state cmpxchg triggered the following fault on a v7.3-rc3 based
x86-64 KVM guest:

  BUG: kernel NULL pointer dereference, address: 0000000000000890
  #PF: supervisor read access in kernel mode
  RIP: task_work_cancel+0xd/0xa0
  RDI: 0000000000000000
  CR2: 0000000000000890
  Call Trace:
   <IRQ>
   bpf_task_work_irq+0x95/0x100
   irq_work_run_list+0x4f/0x90
   irq_work_run+0x18/0x50
   __sysvec_irq_work+0x18/0xb0
   sysvec_irq_work+0x66/0x80
   </IRQ>

0x890 is the task_struct::task_works offset in that build.  It is read
by task_work_pending() through the NULL task argument.

The rcu_read_lock() in bpf_task_work_irq() only keeps ctx memory alive;
it does not retain ctx->task.  A NULL check is insufficient as the task
reference could be released immediately after the check.  Delaying ctx
memory reclamation alone does not preserve the value of ctx->task
either.

Fix the ownership of a scheduling round.  The irq_work scheduler, the
published callback and an asynchronous canceller each hold round_refs
while they can use the round's task, prog or work.  The
STANDBY-to-PENDING transition is the exclusive zero-to-one gate for a
new round.  Its winner initializes the scheduler reference only after
the previous round has reset the context and published STANDBY.

Take the callback reference before task_work_add() publishes the work.
If task_work_add() fails, the scheduler releases the untransferred
callback reference.  The last round user resets task/prog before making
the context reusable.

Map-value deletion publishes FREED first.  If the previous state was
SCHEDULED, refcount_inc_not_zero() either pins the active round for
cancellation or loses to the last put after cleanup has started.  A
successful task_work_cancel() releases exactly one callback round and
ctx reference on its behalf.  If cancellation fails, the actual
callback releases them.

Use separate irq_work objects for scheduling, cancellation and deferred
destruction.  This avoids reinitializing an irq_work that can still be
queued or running.  The deletion path remains atomic-only for NMI
callers, and FREED remains terminal.

Fixes: 38aa7003e369 ("bpf: task work scheduling kfuncs")
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
 kernel/bpf/helpers.c | 123 +++++++++++++++++++++++++++++++++----------
 1 file changed, 95 insertions(+), 28 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b3cc5c8fc875..ff66a30cd04a 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -4380,8 +4380,15 @@ enum bpf_task_work_state {
 struct bpf_task_work_ctx {
 	enum bpf_task_work_state state;
 	refcount_t refcnt;
+	/*
+	 * References to task/prog/work in the current scheduling round.  The
+	 * STANDBY -> PENDING transition serializes initialization from zero.
+	 */
+	refcount_t round_refs;
 	struct callback_head work;
 	struct irq_work irq_work;
+	struct irq_work cancel_irq_work;
+	struct irq_work destroy_irq_work;
 	/* bpf_prog that schedules task work */
 	struct bpf_prog *prog;
 	/* task for which callback is scheduled */
@@ -4416,11 +4423,55 @@ static bool bpf_task_work_ctx_tryget(struct bpf_task_work_ctx *ctx)
 	return refcount_inc_not_zero(&ctx->refcnt);
 }
 
-static void bpf_task_work_destroy(struct irq_work *irq_work)
+static void bpf_task_work_round_get(struct bpf_task_work_ctx *ctx)
 {
-	struct bpf_task_work_ctx *ctx = container_of(irq_work, struct bpf_task_work_ctx, irq_work);
+	refcount_inc(&ctx->round_refs);
+}
+
+/*
+ * Pin an active round for cancellation without reviving a round whose last
+ * user has already started resetting the context.
+ */
+static bool bpf_task_work_round_tryget(struct bpf_task_work_ctx *ctx)
+{
+	return refcount_inc_not_zero(&ctx->round_refs);
+}
+
+/*
+ * Once the last reference reaches zero, a canceller cannot join this round.
+ * Reset task/prog before publishing STANDBY, which is the only state from
+ * which a new scheduler can initialize the next round's first reference.
+ */
+static void bpf_task_work_round_put(struct bpf_task_work_ctx *ctx)
+{
+	enum bpf_task_work_state state, old_state;
+
+	if (!refcount_dec_and_test(&ctx->round_refs))
+		return;
 
 	bpf_task_work_ctx_reset(ctx);
+
+	/* FREED is terminal; leave the round permanently closed. */
+	state = READ_ONCE(ctx->state);
+	if (state == BPF_TW_FREED)
+		return;
+	if (WARN_ON_ONCE(state != BPF_TW_PENDING &&
+			 state != BPF_TW_SCHEDULING &&
+			 state != BPF_TW_RUNNING))
+		return;
+
+	old_state = cmpxchg(&ctx->state, state, BPF_TW_STANDBY);
+	if (old_state == BPF_TW_FREED)
+		return;
+	if (WARN_ON_ONCE(old_state != state))
+		return;
+}
+
+static void bpf_task_work_destroy(struct irq_work *irq_work)
+{
+	struct bpf_task_work_ctx *ctx;
+
+	ctx = container_of(irq_work, struct bpf_task_work_ctx, destroy_irq_work);
 	kfree_rcu(ctx, rcu);
 }
 
@@ -4430,23 +4481,24 @@ static void bpf_task_work_ctx_put(struct bpf_task_work_ctx *ctx)
 		return;
 
 	if (irqs_disabled()) {
-		ctx->irq_work = IRQ_WORK_INIT(bpf_task_work_destroy);
-		irq_work_queue(&ctx->irq_work);
+		ctx->destroy_irq_work = IRQ_WORK_INIT(bpf_task_work_destroy);
+		irq_work_queue(&ctx->destroy_irq_work);
 	} else {
-		bpf_task_work_destroy(&ctx->irq_work);
+		bpf_task_work_destroy(&ctx->destroy_irq_work);
 	}
 }
 
 static void bpf_task_work_cancel(struct bpf_task_work_ctx *ctx)
 {
 	/*
-	 * Scheduled task_work callback holds ctx ref, so if we successfully
-	 * cancelled, we put that ref on callback's behalf. If we couldn't
-	 * cancel, callback will inevitably run or has already completed
-	 * running, and it would have taken care of its ctx ref itself.
+	 * The scheduled callback holds both a ctx ref and a round reference.
+	 * Successful cancellation releases both on its behalf.  If the
+	 * work was already claimed, the callback releases them itself.
 	 */
-	if (task_work_cancel(ctx->task, &ctx->work))
+	if (task_work_cancel(ctx->task, &ctx->work)) {
+		bpf_task_work_round_put(ctx);
 		bpf_task_work_ctx_put(ctx);
+	}
 }
 
 static void bpf_task_work_callback(struct callback_head *cb)
@@ -4466,6 +4518,7 @@ static void bpf_task_work_callback(struct callback_head *cb)
 	if (state == BPF_TW_SCHEDULED)
 		state = cmpxchg(&ctx->state, BPF_TW_SCHEDULED, BPF_TW_RUNNING);
 	if (state == BPF_TW_FREED) {
+		bpf_task_work_round_put(ctx);
 		bpf_task_work_ctx_put(ctx);
 		return;
 	}
@@ -4477,9 +4530,7 @@ static void bpf_task_work_callback(struct callback_head *cb)
 			 (u64)(long)ctx->map_val, 0, 0);
 	migrate_enable();
 
-	bpf_task_work_ctx_reset(ctx);
-	(void)cmpxchg(&ctx->state, BPF_TW_RUNNING, BPF_TW_STANDBY);
-
+	bpf_task_work_round_put(ctx);
 	bpf_task_work_ctx_put(ctx);
 }
 
@@ -4492,18 +4543,17 @@ static void bpf_task_work_irq(struct irq_work *irq_work)
 	guard(rcu)();
 
 	if (cmpxchg(&ctx->state, BPF_TW_PENDING, BPF_TW_SCHEDULING) != BPF_TW_PENDING) {
+		bpf_task_work_round_put(ctx);
 		bpf_task_work_ctx_put(ctx);
 		return;
 	}
 
+	/* Publish the callback's reference before task_work_add(). */
+	bpf_task_work_round_get(ctx);
 	err = task_work_add(ctx->task, &ctx->work, ctx->mode);
 	if (err) {
-		bpf_task_work_ctx_reset(ctx);
-		/*
-		 * try to switch back to STANDBY for another task_work reuse, but we might have
-		 * gone to FREED already, which is fine as we already cleaned up after ourselves
-		 */
-		(void)cmpxchg(&ctx->state, BPF_TW_SCHEDULING, BPF_TW_STANDBY);
+		bpf_task_work_round_put(ctx);
+		bpf_task_work_round_put(ctx);
 		bpf_task_work_ctx_put(ctx);
 		return;
 	}
@@ -4518,6 +4568,8 @@ static void bpf_task_work_irq(struct irq_work *irq_work)
 	state = cmpxchg(&ctx->state, BPF_TW_SCHEDULING, BPF_TW_SCHEDULED);
 	if (state == BPF_TW_FREED)
 		bpf_task_work_cancel(ctx); /* clean up if we switched into FREED state */
+
+	bpf_task_work_round_put(ctx);
 }
 
 static struct bpf_task_work_ctx *bpf_task_work_fetch_ctx(struct bpf_task_work *tw,
@@ -4536,6 +4588,7 @@ static struct bpf_task_work_ctx *bpf_task_work_fetch_ctx(struct bpf_task_work *t
 
 	memset(ctx, 0, sizeof(*ctx));
 	refcount_set(&ctx->refcnt, 1); /* map's own ref */
+	refcount_set(&ctx->round_refs, 0);
 	ctx->state = BPF_TW_STANDBY;
 
 	old_ctx = cmpxchg(&twk->ctx, NULL, ctx);
@@ -4575,10 +4628,11 @@ static struct bpf_task_work_ctx *bpf_task_work_acquire_ctx(struct bpf_task_work
 	}
 
 	if (cmpxchg(&ctx->state, BPF_TW_STANDBY, BPF_TW_PENDING) != BPF_TW_STANDBY) {
-		/* lost acquiring race or map_release_uref() stole it from us, put ref and bail */
+		/* Lost the acquiring race or map deletion made FREED terminal. */
 		bpf_task_work_ctx_put(ctx);
 		return ERR_PTR(-EBUSY);
 	}
+	refcount_set(&ctx->round_refs, 1); /* scheduler's reference */
 
 	/*
 	 * If no process or bpffs is holding a reference to the map, no new callbacks should be
@@ -4586,6 +4640,7 @@ static struct bpf_task_work_ctx *bpf_task_work_acquire_ctx(struct bpf_task_work
 	 * choice: dropping user references should stop everything.
 	 */
 	if (!atomic64_read(&map->usercnt)) {
+		bpf_task_work_round_put(ctx);
 		/* drop ref we just got for task_work callback itself */
 		bpf_task_work_ctx_put(ctx);
 		/* transfer map's ref into cancel_and_free() */
@@ -4773,12 +4828,14 @@ __bpf_kfunc int bpf_timer_cancel_async(struct bpf_timer *timer)
 
 __bpf_kfunc_end_defs();
 
-static void bpf_task_work_cancel_scheduled(struct irq_work *irq_work)
+static void bpf_task_work_cancel_freed(struct irq_work *irq_work)
 {
-	struct bpf_task_work_ctx *ctx = container_of(irq_work, struct bpf_task_work_ctx, irq_work);
+	struct bpf_task_work_ctx *ctx;
 
-	bpf_task_work_cancel(ctx); /* this might put task_work callback's ref */
-	bpf_task_work_ctx_put(ctx); /* and here we put map's own ref that was transferred to us */
+	ctx = container_of(irq_work, struct bpf_task_work_ctx, cancel_irq_work);
+	bpf_task_work_cancel(ctx);
+	bpf_task_work_round_put(ctx);
+	bpf_task_work_ctx_put(ctx); /* put the map's ref transferred to us */
 }
 
 void bpf_task_work_cancel_and_free(void *val)
@@ -4792,10 +4849,20 @@ void bpf_task_work_cancel_and_free(void *val)
 		return;
 
 	state = xchg(&ctx->state, BPF_TW_FREED);
-	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);
-		irq_work_queue(&ctx->irq_work);
+	/*
+	 * SCHEDULED guarantees a callback round user.  After publishing FREED,
+	 * either add the cancellation user before the callback leaves, or lose
+	 * to the last put after the callback has already claimed/completed work.
+	 */
+	if (state == BPF_TW_SCHEDULED &&
+	    bpf_task_work_round_tryget(ctx)) {
+		/*
+		 * A separate irq_work keeps the NMI deletion path atomic-only and
+		 * avoids reinitializing the scheduler's irq_work while it is queued
+		 * or running.  Holding the map ref keeps ctx alive until this runs.
+		 */
+		init_irq_work(&ctx->cancel_irq_work, bpf_task_work_cancel_freed);
+		WARN_ON_ONCE(!irq_work_queue(&ctx->cancel_irq_work));
 		return;
 	}
 
-- 
2.43.0


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

* [PATCH bpf-next 2/2] selftests/bpf: Add task work round ownership race test
  2026-09-18  8:00 [PATCH bpf-next 0/2] bpf: Fix task work round ownership race Yun Lu
  2026-09-18  8:00 ` [PATCH bpf-next 1/2] bpf: Fix task work round ownership during cancellation Yun Lu
@ 2026-09-18  8:00 ` Yun Lu
  2026-09-18  8:13   ` sashiko-bot
  2026-09-18  9:12   ` bot+bpf-ci
  2026-09-18 16:50 ` [PATCH bpf-next 0/2] bpf: Fix task work round ownership race Mykyta Yatsenko
  2 siblings, 2 replies; 9+ messages in thread
From: Yun Lu @ 2026-09-18  8:00 UTC (permalink / raw)
  To: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai
  Cc: bpf

From: Yun Lu <luyun@kylinos.cn>

Exercise task-work scheduling, callback completion and map-value deletion
on separate execution contexts.  Rotate through ctx reuse, deletion
after the callback body, and deletion immediately before the scheduling
kfunc.  The latter two variants stress both the original
SCHEDULING/SCHEDULED cancellation window and the final-user/delete race.

Use READY and DONE sequence handshakes so deletion cannot race map-value
setup and scheduling errors cannot be missed.  Give every callback a
unique generation and wait for that exact generation, preventing a late
callback from an early-delete round from satisfying a later assertion.
Retry transient -EBUSY results while a previous callback wrapper is
finishing.

Select two CPUs from the process affinity mask and use atomic
thread-start/stop handshakes, so the cross-CPU prerequisite is either
established or the test is skipped.

Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
 .../selftests/bpf/prog_tests/test_task_work.c | 283 ++++++++++++++++++
 .../selftests/bpf/progs/task_work_race.c      | 125 ++++++++
 2 files changed, 408 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/task_work_race.c

diff --git a/tools/testing/selftests/bpf/prog_tests/test_task_work.c b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
index 774b31a5f6ca..6d5e1e97aa3a 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_task_work.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
@@ -5,10 +5,14 @@
 #include <stdio.h>
 #include "task_work.skel.h"
 #include "task_work_fail.skel.h"
+#include "task_work_race.skel.h"
 #include <linux/bpf.h>
 #include <linux/perf_event.h>
 #include <sys/syscall.h>
 #include <time.h>
+#include <pthread.h>
+#include <sched.h>
+#include <unistd.h>
 
 static int perf_event_open(__u32 type, __u64 config, int pid)
 {
@@ -155,3 +159,282 @@ void test_task_work(void)
 
 	RUN_TESTS(task_work_fail);
 }
+
+#define TASK_WORK_RACE_ROUNDS	1500
+
+/* Must match progs/task_work_race.c. */
+enum task_work_race_status {
+	RACE_ARM_SEQ,
+	RACE_READY_SEQ,
+	RACE_DONE_SEQ,
+	RACE_SCHED_ERR,
+};
+
+struct task_work_race_value {
+	__u32 seq;
+	char data[60];
+	struct bpf_task_work tw;
+};
+
+struct task_work_race_ctx {
+	int stop;
+	int setup_err;
+	int trigger_tid;
+	int target_tid;
+	int trigger_cpu;
+	int target_cpu;
+};
+
+static int task_work_race_pin_cpu(int cpu)
+{
+	cpu_set_t set;
+
+	CPU_ZERO(&set);
+	CPU_SET(cpu, &set);
+	return pthread_setaffinity_np(pthread_self(), sizeof(set), &set);
+}
+
+static void *task_work_race_trigger(void *arg)
+{
+	struct task_work_race_ctx *ctx = arg;
+	int err;
+
+	err = task_work_race_pin_cpu(ctx->trigger_cpu);
+	if (err)
+		__atomic_store_n(&ctx->setup_err, err, __ATOMIC_RELEASE);
+	__atomic_store_n(&ctx->trigger_tid, syscall(__NR_gettid),
+			 __ATOMIC_RELEASE);
+	while (!__atomic_load_n(&ctx->stop, __ATOMIC_ACQUIRE))
+		getppid();
+	return NULL;
+}
+
+static void *task_work_race_target(void *arg)
+{
+	struct task_work_race_ctx *ctx = arg;
+	int err;
+
+	err = task_work_race_pin_cpu(ctx->target_cpu);
+	if (err)
+		__atomic_store_n(&ctx->setup_err, err, __ATOMIC_RELEASE);
+	__atomic_store_n(&ctx->target_tid, syscall(__NR_gettid),
+			 __ATOMIC_RELEASE);
+	while (!__atomic_load_n(&ctx->stop, __ATOMIC_ACQUIRE))
+		getppid();
+	return NULL;
+}
+
+static int task_work_race_status(struct task_work_race *skel, int idx,
+				 __s64 *value)
+{
+	return bpf_map_lookup_elem(bpf_map__fd(skel->maps.status), &idx,
+				   value);
+}
+
+static int task_work_race_set_status(struct task_work_race *skel, int idx,
+				     __s64 value)
+{
+	return bpf_map_update_elem(bpf_map__fd(skel->maps.status), &idx,
+				   &value, BPF_ANY);
+}
+
+static int task_work_race_wait_seq(struct task_work_race *skel, int idx,
+				   __u32 seq, int timeout_us)
+{
+	int i;
+
+	for (i = 0; i < timeout_us / 100; i++) {
+		__s64 value = 0;
+
+		if (!task_work_race_status(skel, idx, &value) && value == seq)
+			return 0;
+		usleep(100);
+	}
+	return -ETIMEDOUT;
+}
+
+static int task_work_race_wait_ready(struct task_work_race *skel, __u32 seq)
+{
+	int i;
+
+	for (i = 0; i < 2000000 / 100; i++) {
+		__s64 value = 0;
+
+		if (!task_work_race_status(skel, RACE_READY_SEQ, &value) &&
+		    value == seq)
+			return 0;
+		if (!task_work_race_status(skel, RACE_DONE_SEQ, &value) &&
+		    value == seq)
+			return -EIO;
+		usleep(100);
+	}
+	return -ETIMEDOUT;
+}
+
+static int task_work_race_prepare_elem(struct task_work_race *skel)
+{
+	struct task_work_race_value value = {};
+	int key = 0;
+
+	if (!bpf_map_lookup_elem(bpf_map__fd(skel->maps.hmap), &key, &value))
+		return 0;
+	if (errno != ENOENT)
+		return -errno;
+	if (bpf_map_update_elem(bpf_map__fd(skel->maps.hmap), &key, &value,
+				BPF_NOEXIST))
+		return -errno;
+	return 0;
+}
+
+static int task_work_race_arm(struct task_work_race *skel, __u32 seq)
+{
+	__u64 completed = 0;
+	int key = seq;
+
+	if (bpf_map_update_elem(bpf_map__fd(skel->maps.completed), &key,
+				&completed, BPF_ANY))
+		return -errno;
+	if (task_work_race_set_status(skel, RACE_SCHED_ERR, 0) ||
+	    task_work_race_set_status(skel, RACE_ARM_SEQ, seq))
+		return -errno;
+	return task_work_race_wait_ready(skel, seq);
+}
+
+static int task_work_race_check_done(struct task_work_race *skel, __u32 seq)
+{
+	__s64 err = 0;
+	int ret;
+
+	ret = task_work_race_wait_seq(skel, RACE_DONE_SEQ, seq, 2000000);
+	if (ret)
+		return ret;
+	if (task_work_race_status(skel, RACE_SCHED_ERR, &err))
+		return -errno;
+	return err;
+}
+
+static int task_work_race_wait_callback(struct task_work_race *skel,
+					__u32 seq)
+{
+	int i, key = seq;
+
+	for (i = 0; i < 2000000 / 100; i++) {
+		__u64 completed = 0;
+
+		if (!bpf_map_lookup_elem(bpf_map__fd(skel->maps.completed),
+					 &key, &completed) && completed)
+			return 0;
+		usleep(100);
+	}
+	return -ETIMEDOUT;
+}
+
+void serial_test_task_work_race(void)
+{
+	struct task_work_race_ctx ctx = {};
+	struct task_work_race *skel;
+	pthread_t trigger, target;
+	cpu_set_t allowed;
+	int cpu, cpu_count = 0;
+	bool target_started = false, trigger_started = false;
+	int err, i, key = 0;
+
+	if (sched_getaffinity(0, sizeof(allowed), &allowed)) {
+		ASSERT_OK(-errno, "sched_getaffinity");
+		return;
+	}
+	for (cpu = 0; cpu < CPU_SETSIZE && cpu_count < 2; cpu++) {
+		if (!CPU_ISSET(cpu, &allowed))
+			continue;
+		if (!cpu_count)
+			ctx.trigger_cpu = cpu;
+		else
+			ctx.target_cpu = cpu;
+		cpu_count++;
+	}
+	if (cpu_count < 2) {
+		printf("%s:SKIP:need two CPUs in the process affinity mask\n",
+		       __func__);
+		test__skip();
+		return;
+	}
+
+	skel = task_work_race__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+	if (!ASSERT_OK(task_work_race__attach(skel), "attach"))
+		goto cleanup;
+
+	err = pthread_create(&target, NULL, task_work_race_target, &ctx);
+	if (!ASSERT_OK(err, "pthread_create target"))
+		goto cleanup;
+	target_started = true;
+	err = pthread_create(&trigger, NULL, task_work_race_trigger, &ctx);
+	if (!ASSERT_OK(err, "pthread_create trigger"))
+		goto stop;
+	trigger_started = true;
+
+	while (!__atomic_load_n(&ctx.trigger_tid, __ATOMIC_ACQUIRE) ||
+	       !__atomic_load_n(&ctx.target_tid, __ATOMIC_ACQUIRE))
+		usleep(100);
+	if (!ASSERT_OK(__atomic_load_n(&ctx.setup_err, __ATOMIC_ACQUIRE),
+		       "thread affinity"))
+		goto stop;
+
+	skel->bss->trigger_tid = __atomic_load_n(&ctx.trigger_tid, __ATOMIC_ACQUIRE);
+	skel->bss->target_tid = __atomic_load_n(&ctx.target_tid, __ATOMIC_ACQUIRE);
+
+	for (i = 0; i < TASK_WORK_RACE_ROUNDS; i++) {
+		__u32 seq = i + 1;
+		int variant = i % 3;
+
+		if (!ASSERT_OK(task_work_race_prepare_elem(skel), "prepare elem") ||
+		    !ASSERT_OK(task_work_race_arm(skel, seq), "round ready"))
+			goto stop;
+
+		if (variant == 2 &&
+		    !ASSERT_OK(bpf_map_delete_elem(bpf_map__fd(skel->maps.hmap),
+						   &key), "early delete"))
+			goto stop;
+
+		err = task_work_race_check_done(skel, seq);
+		if (!ASSERT_OK(err, "schedule")) {
+			printf("round %d schedule failed: %d\n", i, err);
+			goto stop;
+		}
+
+		if (variant != 2 &&
+		    !ASSERT_OK(task_work_race_wait_callback(skel, seq),
+			       "callback"))
+			goto stop;
+
+		if (variant == 1 &&
+		    !ASSERT_OK(bpf_map_delete_elem(bpf_map__fd(skel->maps.hmap),
+						   &key), "late delete"))
+			goto stop;
+	}
+
+	/* A distinct completion generation prevents an old callback satisfying this. */
+	if (!ASSERT_OK(task_work_race_prepare_elem(skel), "final prepare") ||
+	    !ASSERT_OK(task_work_race_arm(skel, TASK_WORK_RACE_ROUNDS + 1),
+		       "final ready") ||
+	    !ASSERT_OK(task_work_race_check_done(skel,
+					TASK_WORK_RACE_ROUNDS + 1),
+		       "final schedule") ||
+	    !ASSERT_OK(task_work_race_wait_callback(skel,
+					     TASK_WORK_RACE_ROUNDS + 1),
+		       "final callback"))
+		goto stop;
+
+	printf("%s: %d race rounds completed\n", __func__,
+	       TASK_WORK_RACE_ROUNDS);
+
+stop:
+	__atomic_store_n(&ctx.stop, 1, __ATOMIC_RELEASE);
+	if (trigger_started)
+		pthread_join(trigger, NULL);
+	if (target_started)
+		pthread_join(target, NULL);
+cleanup:
+	task_work_race__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/task_work_race.c b/tools/testing/selftests/bpf/progs/task_work_race.c
new file mode 100644
index 000000000000..8d3f31fb075f
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/task_work_race.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Corporation. */
+#include <vmlinux.h>
+#include <stdbool.h>
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+#define TASK_WORK_RACE_MAX_SEQ	2048
+
+enum task_work_race_status {
+	RACE_ARM_SEQ,
+	RACE_READY_SEQ,
+	RACE_DONE_SEQ,
+	RACE_SCHED_ERR,
+	RACE_STATUS_MAX,
+};
+
+int trigger_tid;
+int target_tid;
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, RACE_STATUS_MAX);
+	__type(key, int);
+	__type(value, __s64);
+} status SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, TASK_WORK_RACE_MAX_SEQ);
+	__type(key, int);
+	__type(value, __u64);
+} completed SEC(".maps");
+
+struct task_work_race_value {
+	__u32 seq;
+	char data[60];
+	struct bpf_task_work tw;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__uint(max_entries, 4);
+	__type(key, int);
+	__type(value, struct task_work_race_value);
+} hmap SEC(".maps");
+
+static __always_inline void set_status(int key, __s64 value)
+{
+	__s64 *slot;
+
+	slot = bpf_map_lookup_elem(&status, &key);
+	if (slot)
+		*slot = value;
+}
+
+static int process_work(struct bpf_map *map, void *key, void *value)
+{
+	struct task_work_race_value *work = value;
+	__u64 *done;
+	int seq = work->seq;
+
+	if (seq <= 0 || seq >= TASK_WORK_RACE_MAX_SEQ)
+		return 0;
+	done = bpf_map_lookup_elem(&completed, &seq);
+	if (done)
+		*done = 1;
+	return 0;
+}
+
+SEC("tracepoint/syscalls/sys_enter_getppid")
+int race_sched_work(void *ctx)
+{
+	struct task_work_race_value *work;
+	struct task_struct *task;
+	__s64 *arm, *done;
+	__u32 tid = (__u32)bpf_get_current_pid_tgid();
+	int key = 0, err;
+	__u32 seq;
+
+	if (tid != trigger_tid)
+		return 0;
+
+	key = RACE_ARM_SEQ;
+	arm = bpf_map_lookup_elem(&status, &key);
+	key = RACE_DONE_SEQ;
+	done = bpf_map_lookup_elem(&status, &key);
+	if (!arm || !done || *arm <= 0 || *done == *arm)
+		return 0;
+	seq = *arm;
+
+	task = bpf_task_from_pid(target_tid);
+	if (!task) {
+		err = -ESRCH;
+		goto out_done;
+	}
+
+	key = 0;
+	work = bpf_map_lookup_elem(&hmap, &key);
+	if (!work) {
+		err = -ENOENT;
+		goto out_task;
+	}
+
+	work->seq = seq;
+	set_status(RACE_READY_SEQ, seq);
+	err = bpf_task_work_schedule_signal(task, &work->tw, &hmap,
+					    process_work);
+	if (err == -EBUSY) {
+		bpf_task_release(task);
+		return 0;
+	}
+
+out_task:
+	bpf_task_release(task);
+out_done:
+	set_status(RACE_SCHED_ERR, err);
+	set_status(RACE_DONE_SEQ, seq);
+	return 0;
+}
-- 
2.43.0


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

* Re: [PATCH bpf-next 2/2] selftests/bpf: Add task work round ownership race test
  2026-09-18  8:00 ` [PATCH bpf-next 2/2] selftests/bpf: Add task work round ownership race test Yun Lu
@ 2026-09-18  8:13   ` sashiko-bot
  2026-09-18  9:12   ` bot+bpf-ci
  1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-18  8:13 UTC (permalink / raw)
  To: Yun Lu; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] bpf: spurious -ENOENT test failure due to unhandled early delete race
- [Medium] bpf: missing memory barriers cause stale error state reads in race test
- [Medium] bpf: TOCTOU race in task_work_race_wait_ready() causes false test failures
--

commit 6a4b075b19b6a65e88e8c41b961bca6ee7533e52
Author: Yun Lu <luyun@kylinos.cn>

selftests/bpf: Add task work round ownership race test

Exercise task-work scheduling, callback completion and map-value deletion
on separate execution contexts.  Rotate through ctx reuse, deletion
after the callback body, and deletion immediately before the scheduling
kfunc.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_task_work.c b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
> index 774b31a5f6ca1..6d5e1e97aa3af 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_task_work.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
[ ... ]
> +static int task_work_race_wait_ready(struct task_work_race *skel, __u32 seq)
> +{
> +	int i;
> +
> +	for (i = 0; i < 2000000 / 100; i++) {
> +		__s64 value = 0;
> +
> +		if (!task_work_race_status(skel, RACE_READY_SEQ, &value) &&
> +		    value == seq)
> +			return 0;
> +		if (!task_work_race_status(skel, RACE_DONE_SEQ, &value) &&
> +		    value == seq)
> +			return -EIO;

[Severity: Medium]
Could this sequential checking lead to a false negative?

If the concurrent BPF program finishes execution exactly between these two
syscalls, the thread will see RACE_READY_SEQ as false from the first read
and RACE_DONE_SEQ as true from the second read, returning -EIO. 

If the BPF program had finished before the first check, it would have read
RACE_READY_SEQ == seq and correctly returned 0. Can this time-of-check to
time-of-use interleaving incorrectly penalize a fast BPF execution as an
error?

> +		usleep(100);
> +	}
> +	return -ETIMEDOUT;
> +}
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/task_work_race.c b/tools/testing/selftests/bpf/progs/task_work_race.c
> new file mode 100644
> index 0000000000000..8d3f31fb075f8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/task_work_race.c
[ ... ]
> +SEC("tracepoint/syscalls/sys_enter_getppid")
> +int race_sched_work(void *ctx)
> +{
[ ... ]
> +	key = 0;
> +	work = bpf_map_lookup_elem(&hmap, &key);
> +	if (!work) {
> +		err = -ENOENT;
> +		goto out_task;
> +	}
> +
> +	work->seq = seq;
> +	set_status(RACE_READY_SEQ, seq);
> +	err = bpf_task_work_schedule_signal(task, &work->tw, &hmap,
> +					    process_work);
> +	if (err == -EBUSY) {
> +		bpf_task_release(task);
> +		return 0;
> +	}

[Severity: Medium]
When variant 2 is tested, userspace deliberately races to delete the map
element early. If this early delete wins the race against the scheduling
kfunc above, bpf_task_work_schedule_signal() returns -EBUSY. 

This causes the BPF program to return 0, triggering a retry. On the next
tracepoint invocation, looking up the now-deleted element fails with -ENOENT. 

Because userspace expects a successful schedule (err == 0) in
task_work_race_check_done(), the test will fail when it observes -ENOENT.
Should the test be updated to handle this expected -ENOENT outcome, or
should the BPF program avoid retrying when the element is permanently
deleted?

> +
> +out_task:
> +	bpf_task_release(task);
> +out_done:
> +	set_status(RACE_SCHED_ERR, err);
> +	set_status(RACE_DONE_SEQ, seq);

[Severity: Medium]
Does this code sequence need a write memory barrier?

On weakly-ordered architectures like ARM64, the CPU can reorder these
independent stores because the BPF JIT does not emit barrier instructions
for array map stores.

If userspace polls RACE_DONE_SEQ and observes the update before
RACE_SCHED_ERR is globally visible, it might read a stale error code 
(e.g., 0 instead of the actual error) and potentially mask failures.

> +	return 0;
> +}

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

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

* Re: [PATCH bpf-next 2/2] selftests/bpf: Add task work round ownership race test
  2026-09-18  8:00 ` [PATCH bpf-next 2/2] selftests/bpf: Add task work round ownership race test Yun Lu
  2026-09-18  8:13   ` sashiko-bot
@ 2026-09-18  9:12   ` bot+bpf-ci
  1 sibling, 0 replies; 9+ messages in thread
From: bot+bpf-ci @ 2026-09-18  9:12 UTC (permalink / raw)
  To: luyun_611, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai
  Cc: bpf, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song,
	mason, ihor.solodrai

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_task_work.c b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
> index 774b31a5f6ca1..6d5e1e97aa3af 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_task_work.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_task_work.c

[ ... ]

> +static int task_work_race_wait_ready(struct task_work_race *skel, __u32 seq)
> +{
> +	int i;
> +
> +	for (i = 0; i < 2000000 / 100; i++) {
> +		__s64 value = 0;
> +
> +		if (!task_work_race_status(skel, RACE_READY_SEQ, &value) &&
> +		    value == seq)
> +			return 0;
> +		if (!task_work_race_status(skel, RACE_DONE_SEQ, &value) &&
> +		    value == seq)
> +			return -EIO;
> +		usleep(100);
> +	}
> +	return -ETIMEDOUT;
> +}

[ ... ]

> +void serial_test_task_work_race(void)
> +{
> +	struct task_work_race_ctx ctx = {};
> +	struct task_work_race *skel;
> +	pthread_t trigger, target;
> +	cpu_set_t allowed;
> +	int cpu, cpu_count = 0;
> +	bool target_started = false, trigger_started = false;
> +	int err, i, key = 0;
> +
> +	if (sched_getaffinity(0, sizeof(allowed), &allowed)) {
> +		ASSERT_OK(-errno, "sched_getaffinity");
> +		return;
> +	}
> +	for (cpu = 0; cpu < CPU_SETSIZE && cpu_count < 2; cpu++) {
> +		if (!CPU_ISSET(cpu, &allowed))
> +			continue;
> +		if (!cpu_count)
> +			ctx.trigger_cpu = cpu;
> +		else
> +			ctx.target_cpu = cpu;
> +		cpu_count++;
> +	}
> +	if (cpu_count < 2) {
> +		printf("%s:SKIP:need two CPUs in the process affinity mask\n",
> +		       __func__);
> +		test__skip();
> +		return;
> +	}
> +
> +	skel = task_work_race__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open_and_load"))
> +		return;
> +	if (!ASSERT_OK(task_work_race__attach(skel), "attach"))
> +		goto cleanup;
> +
> +	err = pthread_create(&target, NULL, task_work_race_target, &ctx);
> +	if (!ASSERT_OK(err, "pthread_create target"))
> +		goto cleanup;
> +	target_started = true;
> +	err = pthread_create(&trigger, NULL, task_work_race_trigger, &ctx);
> +	if (!ASSERT_OK(err, "pthread_create trigger"))
> +		goto stop;
> +	trigger_started = true;
> +
> +	while (!__atomic_load_n(&ctx.trigger_tid, __ATOMIC_ACQUIRE) ||
> +	       !__atomic_load_n(&ctx.target_tid, __ATOMIC_ACQUIRE))
> +		usleep(100);

If neither thread publishes its tid, does this poll terminate?  The
cpu_count < 2 case skips with test__skip(), but if pthread_create()
succeeds yet the thread fails to run or to reach its __atomic_store_n(),
this loop has no iteration cap or timeout so the test would hang rather
than report a failure.

> +	if (!ASSERT_OK(__atomic_load_n(&ctx.setup_err, __ATOMIC_ACQUIRE),
> +		       "thread affinity"))
> +		goto stop;
> +
> +	skel->bss->trigger_tid = __atomic_load_n(&ctx.trigger_tid, __ATOMIC_ACQUIRE);
> +	skel->bss->target_tid = __atomic_load_n(&ctx.target_tid, __ATOMIC_ACQUIRE);

[ ... ]

> +
> +	printf("%s: %d race rounds completed\n", __func__,
> +	       TASK_WORK_RACE_ROUNDS);

This isn't a bug, but could the printf() on the success path and the
'round %d schedule failed: %d' diagnostic on the failure path be removed?
The ASSERT_* calls already record pass/fail state, and the failure
diagnostic writes to stdout while the neighbouring task_work_run()
diagnostics use fprintf(stderr, ...).

> +
> +stop:
> +	__atomic_store_n(&ctx.stop, 1, __ATOMIC_RELEASE);

[ ... ]


The commit message says:

>     Select two CPUs from the process affinity mask and use atomic
>     thread-start/stop handshakes, so the cross-CPU prerequisite is either
>     established or the test is skipped.

Is this description accurate?  When cpu_count < 2 the test does skip with
test__skip() and a reason string.  But when pthread_setaffinity_np() or
pthread_create() fails the test reports a hard FAILURE via ASSERT_OK()
rather than skipping - the affinity-setup prerequisite is not being met,
yet the test is not skipped.

Could either the commit message drop the 'or the test is skipped' claim,
or the affinity/thread-creation failures route to test__skip() with a
reason string like the CPU-count check does?


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

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

* Re: [PATCH bpf-next 0/2] bpf: Fix task work round ownership race
  2026-09-18  8:00 [PATCH bpf-next 0/2] bpf: Fix task work round ownership race Yun Lu
  2026-09-18  8:00 ` [PATCH bpf-next 1/2] bpf: Fix task work round ownership during cancellation Yun Lu
  2026-09-18  8:00 ` [PATCH bpf-next 2/2] selftests/bpf: Add task work round ownership race test Yun Lu
@ 2026-09-18 16:50 ` Mykyta Yatsenko
  2026-09-18 17:17   ` Mykyta Yatsenko
  2 siblings, 1 reply; 9+ messages in thread
From: Mykyta Yatsenko @ 2026-09-18 16:50 UTC (permalink / raw)
  To: Yun Lu, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai
  Cc: bpf



On 9/18/26 9:00 AM, Yun Lu wrote:
> From: Yun Lu <luyun@kylinos.cn>
> 
> This series fixes a race in the bpf_task_work scheduling kfuncs and
> adds a regression test for it.
> 
> A task_work callback scheduled for a task running on another CPU can
> complete before bpf_task_work_irq() regains control after
> task_work_add().  The callback's cleanup releases ctx->task, so a
> concurrent map value deletion that publishes the FREED state makes the
> resumed irq_work handler call task_work_cancel(NULL, &ctx->work),
> dereferencing task_struct::task_works through a NULL task (RIP at
> task_work_cancel+0xd, RDI == 0, CR2 at the task_works offset).  The
> same cancellation is reached when the deletion wins before the
> callback runs and the callback's bailout path drops the last ctx
> refcount, clearing ctx->task.
> 
> Patch 1 gives each scheduling round an ownership count: the irq_work
> scheduler, the published callback and an asynchronous canceller hold a
> reference while using this round's task/prog/work, and only the last
> user releases them, before the ctx can be reused from STANDBY.  The
> count is zero based with the STANDBY -> PENDING transition acting as
> the zero-to-one gate, so a finished round cannot be revived: a
> canceller either pins a still-active round (ctx->task guaranteed
> valid) or declines to cancel.  Separate irq_work objects for
> scheduling, cancellation and deferred destruction avoid reinitializing
> an irq_work that may still be queued or running.  FREED remains
> terminal and the scheduling path stays atomic-only, preserving NMI
> safety.
> 
> The race window between task_work_add() and the SCHEDULING ->
> SCHEDULED cmpxchg is only a few instructions wide, so the fix was
> verified with a deterministic reproduction that widens exactly this
> window with a debug delay, while scheduling cross-CPU and deleting the
> map value concurrently: without the fix the kernel panics reliably;
> with it, all interleavings complete.  A 900-round stress test rotating
> through ctx reuse, deletion after the callback and deletion right
> after scheduling also runs clean, with no leaks reported by kmemleak.
> 
> Patch 2 adds a selftest that keeps steady pressure on the
> interleaving: it schedules cross-CPU, deletes the map value at
> different points of a round, uses READY/DONE handshakes so scheduling
> errors cannot be missed, and tags every callback with a generation so
> a late callback cannot satisfy a later round's assertions.
> 

I could not reproduce the kernel crash on my computer, the test is also
very flaky:

./test_progs -t task_work_race
serial_test_task_work_race:FAIL:round ready unexpected error: -5 (errno 2)
#513     task_work_race:FAIL

Could you please share a bit more on how to repro the crash, feel free to
share your config, arch, any details on how you run it.

> Yun Lu (2):
>   bpf: Fix task work round ownership during cancellation
>   selftests/bpf: Add task work round ownership race test
> 
>  kernel/bpf/helpers.c                                  | 123 ++++++++++--
>  .../selftests/bpf/prog_tests/test_task_work.c         | 283 +++++++++++++++++++++
>  .../selftests/bpf/progs/task_work_race.c              | 125 ++++++++
>  3 files changed, 503 insertions(+), 28 deletions(-)
> 


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

* Re: [PATCH bpf-next 0/2] bpf: Fix task work round ownership race
  2026-09-18 16:50 ` [PATCH bpf-next 0/2] bpf: Fix task work round ownership race Mykyta Yatsenko
@ 2026-09-18 17:17   ` Mykyta Yatsenko
  0 siblings, 0 replies; 9+ messages in thread
From: Mykyta Yatsenko @ 2026-09-18 17:17 UTC (permalink / raw)
  To: Yun Lu, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai
  Cc: bpf

On 9/18/26 5:50 PM, Mykyta Yatsenko wrote:
> 
> 
> On 9/18/26 9:00 AM, Yun Lu wrote:
>> From: Yun Lu <luyun@kylinos.cn>
>>
>> This series fixes a race in the bpf_task_work scheduling kfuncs and
>> adds a regression test for it.
>>
>> A task_work callback scheduled for a task running on another CPU can
>> complete before bpf_task_work_irq() regains control after
>> task_work_add().  The callback's cleanup releases ctx->task, so a
>> concurrent map value deletion that publishes the FREED state makes the
>> resumed irq_work handler call task_work_cancel(NULL, &ctx->work),
>> dereferencing task_struct::task_works through a NULL task (RIP at
>> task_work_cancel+0xd, RDI == 0, CR2 at the task_works offset).  The
>> same cancellation is reached when the deletion wins before the
>> callback runs and the callback's bailout path drops the last ctx
>> refcount, clearing ctx->task.
>>
>> Patch 1 gives each scheduling round an ownership count: the irq_work
>> scheduler, the published callback and an asynchronous canceller hold a
>> reference while using this round's task/prog/work, and only the last
>> user releases them, before the ctx can be reused from STANDBY.  The
>> count is zero based with the STANDBY -> PENDING transition acting as
>> the zero-to-one gate, so a finished round cannot be revived: a
>> canceller either pins a still-active round (ctx->task guaranteed
>> valid) or declines to cancel.  Separate irq_work objects for
>> scheduling, cancellation and deferred destruction avoid reinitializing
>> an irq_work that may still be queued or running.  FREED remains
>> terminal and the scheduling path stays atomic-only, preserving NMI
>> safety.
>>
>> The race window between task_work_add() and the SCHEDULING ->
>> SCHEDULED cmpxchg is only a few instructions wide, so the fix was
>> verified with a deterministic reproduction that widens exactly this
>> window with a debug delay, while scheduling cross-CPU and deleting the
>> map value concurrently: without the fix the kernel panics reliably;
>> with it, all interleavings complete.  A 900-round stress test rotating
>> through ctx reuse, deletion after the callback and deletion right
>> after scheduling also runs clean, with no leaks reported by kmemleak.
>>
>> Patch 2 adds a selftest that keeps steady pressure on the
>> interleaving: it schedules cross-CPU, deletes the map value at
>> different points of a round, uses READY/DONE handshakes so scheduling
>> errors cannot be missed, and tags every callback with a generation so
>> a late callback cannot satisfy a later round's assertions.
>>
> 
> I could not reproduce the kernel crash on my computer, the test is also
> very flaky:
> 
> ./test_progs -t task_work_race
> serial_test_task_work_race:FAIL:round ready unexpected error: -5 (errno 2)
> #513     task_work_race:FAIL
> 
> Could you please share a bit more on how to repro the crash, feel free to
> share your config, arch, any details on how you run it.
> 

I've got a repro:

[  139.522974] BUG: kernel NULL pointer dereference, address: 0000000000000758
[  139.523087] #PF: supervisor read access in kernel mode
[  139.523155] #PF: error_code(0x0000) - not-present page
[  139.523207] PGD 101bdf067 P4D 0
[  139.523254] Oops: Oops: 0000 [#1] SMP
[  139.523302] CPU: 0 UID: 0 PID: 784 Comm: test_progs Tainted: G           OE       7.3.0-rc2-g961768eff0dc #3 PREEMPT(full)
[  139.523418] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
[  139.523492] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[  139.523630] RIP: 0010:task_work_cancel+0xe/0xa0
[  139.523814] Code: 48 89 df e8 b4 e6 b8 00 4c 89 f8 5b 41 5e 41 5f c3 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 d6 0f 1f 44 00 00 41 57 41 56 53 <48> 83 bf 58 07 00 00 00 75 0f 45 31 ff 49 39 f7 0f 94 c0 5b 41 5e
[  139.524037] RSP: 0018:ff4aee5680003f80 EFLAGS: 00010046
[  139.524106] RAX: 0000000000000005 RBX: ff140371450c0d00 RCX: 0000000000000003
[  139.524199] RDX: ffffffff950bd6bb RSI: ff140371450c0d08 RDI: 0000000000000000
[  139.524293] RBP: 0000000000000022 R08: 0000000000000000 R09: 0000000000000000
[  139.524383] R10: 0000000000000000 R11: ff4aee5680003ff8 R12: 0000000000000000
[  139.524475] R13: 0000000000000000 R14: ff140371450c0d18 R15: ff140371450c0d08
[  139.524572] FS:  00007f7ce77be640(0000) GS:ff140371e457b000(0000) knlGS:0000000000000000
[  139.524672] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  139.524746] CR2: 0000000000000758 CR3: 0000000101862003 CR4: 0000000000771ef0
[  139.524847] PKRU: 55555554
[  139.524894] Call Trace:
[  139.524926]  <IRQ>
[  139.524956]  bpf_task_work_irq+0xaf/0xc0
[  139.525056]  irq_work_run+0x91/0x110
[  139.525137]  __sysvec_irq_work+0x1d/0xa0
[  139.525219]  sysvec_irq_work+0x64/0x80
[  139.525277]  </IRQ>
[  139.525310]  <TASK>
[  139.525378]  asm_sysvec_irq_work+0x1a/0x20
[  139.525436] RIP: 0010:default_send_IPI_self+0x39/0x50
[  139.525574] Code: 04 25 00 d3 5f ff a9 00 10 00 00 74 10 f3 90 8b 04 25 00 d3 5f ff a9 00 10 00 00 75 f0 81 cf 00 00 04 00 89 3c 25 00 d3 5f ff <c3> e8 21 fc ff ff bf 00 04 00 00 eb e6 cc cc cc cc cc cc cc cc cc
[  139.525785] RSP: 0018:ff4aee56802bfc80 EFLAGS: 00000206
[  139.525844] RAX: 00000000000000fb RBX: 0000000000000000 RCX: 0000000000000000
[  139.525931] RDX: 0000000000000023 RSI: ff140371e457b000 RDI: 00000000000400f6
[  139.526020] RBP: ffffffffc0200988 R08: ffffffff976aa170 R09: 0000000000000002
[  139.526107] R10: 0000000000000000 R11: 0000000000000000 R12: ff14037140800700
[  139.526189] R13: ff140371416bcc00 R14: ff140371450c0d00 R15: ff140371428b42c0
[  139.526271]  ? 0xffffffffc0200988
[  139.526320]  arch_irq_work_raise+0x21/0x30
[  139.526369]  irq_work_queue+0x28/0x60
[  139.526405]  bpf_task_work_schedule+0x2bf/0x2e0
[  139.526467]  bpf_prog_971652e63b55a6ac_race_sched_work+0x1a1/0x257
[  139.526539]  trace_call_bpf_faultable+0x15c/0x2e0
[  139.526602]  perf_syscall_enter+0x16e/0x2f0
[  139.526652]  ? trace_syscall_enter+0x5d/0x90
[  139.526792]  trace_syscall_enter+0x5d/0x90
[  139.526841]  do_syscall_64+0x1cb/0x290
[  139.526892]  entry_SYSCALL_64_after_hwframe+0x4b/0x53
[  139.526953] RIP: 0033:0x7f7ce80a0cfb
[  139.526997] Code: 0f 1e fa 31 c9 e9 a5 fc ff ff 0f 1f 44 00 00 f3 0f 1e fa b8 27 00 00 00 0f 05 c3 0f 1f 40 00 f3 0f 1e fa b8 6e 00 00 00 0f 05 <c3> 0f 1f 40 00 f3 0f 1e fa b8 66 00 00 00 0f 05 c3 0f 1f 40 00 f3
[  139.527210] RSP: 002b:00007f7ce77bddd8 EFLAGS: 00000202 ORIG_RAX: 000000000000006e
[  139.527304] RAX: ffffffffffffffda RBX: 00007f7ce77be640 RCX: 00007f7ce80a0cfb
[  139.527395] RDX: 00007f7ce8056ea1 RSI: 0000000000000000 RDI: 0000000000000080
[  139.527485] RBP: 00007f7ce77bde10 R08: 00007ffe01a1c0bf R09: 0000000000000000
[  139.527585] R10: 00007f7ce7fd7f38 R11: 0000000000000202 R12: 00007f7ce77be640
[  139.527678] R13: 0000000000000016 R14: 00007f7ce8050230 R15: 0000000000000000
[  139.527766]  </TASK>
[  139.527791] Modules linked in: bpf_testmod(OE) [last unloaded: bpf_testmod(OE)]
[  139.527932] CR2: 0000000000000758
[  139.527989] ---[ end trace 0000000000000000 ]---
[  139.528059] RIP: 0010:task_work_cancel+0xe/0xa0
[  139.528121] Code: 48 89 df e8 b4 e6 b8 00 4c 89 f8 5b 41 5e 41 5f c3 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 d6 0f 1f 44 00 00 41 57 41 56 53 <48> 83 bf 58 07 00 00 00 75 0f 45 31 ff 49 39 f7 0f 94 c0 5b 41 5e
[  139.528306] RSP: 0018:ff4aee5680003f80 EFLAGS: 00010046
[  139.528356] RAX: 0000000000000005 RBX: ff140371450c0d00 RCX: 0000000000000003
[  139.528449] RDX: ffffffff950bd6bb RSI: ff140371450c0d08 RDI: 0000000000000000
[  139.528519] RBP: 0000000000000022 R08: 0000000000000000 R09: 0000000000000000
[  139.528580] R10: 0000000000000000 R11: ff4aee5680003ff8 R12: 0000000000000000
[  139.528649] R13: 0000000000000000 R14: ff140371450c0d18 R15: ff140371450c0d08
[  139.528753] FS:  00007f7ce77be640(0000) GS:ff140371e457b000(0000) knlGS:0000000000000000
[  139.528849] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  139.528922] CR2: 0000000000000758 CR3: 0000000101862003 CR4: 0000000000771ef0
[  139.529006] PKRU: 55555554
[  139.529043] Kernel panic - not syncing: Fatal exception in interrupt
[  139.529755] Kernel Offset: 0x14000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[  139.529938] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---

>> Yun Lu (2):
>>   bpf: Fix task work round ownership during cancellation
>>   selftests/bpf: Add task work round ownership race test
>>
>>  kernel/bpf/helpers.c                                  | 123 ++++++++++--
>>  .../selftests/bpf/prog_tests/test_task_work.c         | 283 +++++++++++++++++++++
>>  .../selftests/bpf/progs/task_work_race.c              | 125 ++++++++
>>  3 files changed, 503 insertions(+), 28 deletions(-)
>>
> 


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

* Re: [PATCH bpf-next 1/2] bpf: Fix task work round ownership during cancellation
  2026-09-18  8:00 ` [PATCH bpf-next 1/2] bpf: Fix task work round ownership during cancellation Yun Lu
@ 2026-09-18 17:54   ` Alexei Starovoitov
  2026-09-21 10:22     ` luyun
  0 siblings, 1 reply; 9+ messages in thread
From: Alexei Starovoitov @ 2026-09-18 17:54 UTC (permalink / raw)
  To: Yun Lu, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai
  Cc: bpf

On Fri Sep 18, 2026 at 8:00 AM UTC, Yun Lu wrote:
>  struct bpf_task_work_ctx {
>  	enum bpf_task_work_state state;
>  	refcount_t refcnt;
> +	/*
> +	 * References to task/prog/work in the current scheduling round.  The
> +	 * STANDBY -> PENDING transition serializes initialization from zero.
> +	 */
> +	refcount_t round_refs;
>  	struct callback_head work;
>  	struct irq_work irq_work;
> +	struct irq_work cancel_irq_work;
> +	struct irq_work destroy_irq_work;

This is overkill. Find a different way to fix it.

pw-bot: cr

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

* Re: [PATCH bpf-next 1/2] bpf: Fix task work round ownership during cancellation
  2026-09-18 17:54   ` Alexei Starovoitov
@ 2026-09-21 10:22     ` luyun
  0 siblings, 0 replies; 9+ messages in thread
From: luyun @ 2026-09-21 10:22 UTC (permalink / raw)
  To: Alexei Starovoitov, ast, daniel, andrii, eddyz87, memxor,
	martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai
  Cc: bpf


在 2026/9/19 01:54, Alexei Starovoitov 写道:
> On Fri Sep 18, 2026 at 8:00 AM UTC, Yun Lu wrote:
>>   struct bpf_task_work_ctx {
>>   	enum bpf_task_work_state state;
>>   	refcount_t refcnt;
>> +	/*
>> +	 * References to task/prog/work in the current scheduling round.  The
>> +	 * STANDBY -> PENDING transition serializes initialization from zero.
>> +	 */
>> +	refcount_t round_refs;
>>   	struct callback_head work;
>>   	struct irq_work irq_work;
>> +	struct irq_work cancel_irq_work;
>> +	struct irq_work destroy_irq_work;
> This is overkill. Find a different way to fix it.


Hi, Alexei

Thanks for reviewing, this has been dropped in v2.

Instead of the extra refcount and irq_work objects, v2 adds a
completion edge: the callback claims RUNNING and waits for the
scheduling irq_work invocation with irq_work_sync() before it resets
task/prog or publishes STANDBY.  One temporary reference from the
existing ctx refcount covers the window where the callback bails out
on FREED before the handler's cancellation attempt, and a BUSY check
in acquire covers the add-failure path.  No fields or states are added
to the context, and the cancel/destroy paths are unchanged.

v2: https://lore.kernel.org/all/20260921101453.69273-2-luyun_611@163.com/


---

Thanks,

Yun Lu



>
> pw-bot: cr


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

end of thread, other threads:[~2026-09-21 10:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-18  8:00 [PATCH bpf-next 0/2] bpf: Fix task work round ownership race Yun Lu
2026-09-18  8:00 ` [PATCH bpf-next 1/2] bpf: Fix task work round ownership during cancellation Yun Lu
2026-09-18 17:54   ` Alexei Starovoitov
2026-09-21 10:22     ` luyun
2026-09-18  8:00 ` [PATCH bpf-next 2/2] selftests/bpf: Add task work round ownership race test Yun Lu
2026-09-18  8:13   ` sashiko-bot
2026-09-18  9:12   ` bot+bpf-ci
2026-09-18 16:50 ` [PATCH bpf-next 0/2] bpf: Fix task work round ownership race Mykyta Yatsenko
2026-09-18 17:17   ` Mykyta Yatsenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox