BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 0/3] bpf: Fix task work scheduling races
@ 2026-09-29  7:27 Yun Lu
  2026-09-29  7:27 ` [PATCH bpf-next v4 1/3] bpf: Fix NULL task dereference in task work cancellation Yun Lu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Yun Lu @ 2026-09-29  7:27 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>

This series fixes two races in the bpf_task_work scheduling kfuncs and
adds a regression test for them.

Patch 1 fixes a NULL task dereference: the callback can clear ctx->task
before the scheduling irq_work handler attempts cancellation, so the
handler now captures the task pointer before publishing the callback and
relies on its existing RCU read-side section, which covers the captured
task because bpf_task_release() is put_task_struct_rcu_user().

Patch 2 stops a new round from reusing the context while the previous
scheduling irq_work is still BUSY.  Without it, a delayed handler can
consume the new round's SCHEDULING state, leaving the context stuck in a
false SCHEDULED that later routes deletion into task_work_cancel() with
a NULL task.

Patch 3 adds a 1500-round cross-CPU test racing scheduling, callback
completion and map-value deletion.

Changes since v3:

- Drop the get_task_struct()/put_task_struct() pair: capturing the
  pointer before task_work_add() is sufficient, since
  bpf_task_release() is put_task_struct_rcu_user() and the handler's
  RCU read-side section keeps the captured task alive.  Reword the
  RCU explanation.
- Split the BUSY check and the one-time irq_work initialization into
  a separate patch 2, with the full interleaving and the failure it
  prevents.
- Selftest code unchanged; commit message wording only.

v3: https://lore.kernel.org/all/20260922100042.136818-1-luyun_611@163.com/

Changes since v2:

- Kernel: drop irq_work_sync() from the callback and both
  WARN_ON_ONCE() checks, as requested in review.  Following the
  suggested direction, the scheduling handler now takes its own task
  reference around task_work_add() and the post-add cancellation; the
  temporary ctx refcount from v2 is gone as well.  The callback ends
  up unchanged from upstream.
- The BUSY check in acquire is kept: without it a handler delayed
  between task_work_add() and its state cmpxchg can flip the next
  round into a bogus SCHEDULED, which later routes map-value deletion
  into task_work_cancel() with an already-reset ctx->task.
- Selftest is unchanged apart from the rebase.

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

Changes since v1:

- Kernel: rework the fix per review feedback.  Replace the round_refs
  ownership counter and the extra irq_work objects with a completion
  edge: the callback claims RUNNING and waits for the scheduling
  irq_work via irq_work_sync(), plus one temporary reference from the
  existing ctx refcount and a BUSY check covering the add-failure
  path.  No context fields or states are added.
- Selftest: address review feedback (single packed result store,
  bounded thread-start wait, tolerate the expected deletion-race
  outcomes).

v1: https://lore.kernel.org/bpf/20260918080014.54012-1-luyun_611@163.com/

Testing:

- Built with KASAN, lockdep, PROVE_RCU, DEBUG_ATOMIC_SLEEP and
  KMEMLEAK; test_progs -t task_work and 900 stress rounds in QEMU are
  clean, no leaks reported.
- Replayed the deterministic reproducer (debug-delay only, not part of
  the series): without the fixes it panics reliably in
  task_work_cancel(); with them the callback stays non-blocking and
  the guest stays healthy.
- Directed delay tests cover both cancellation paths and both BUSY
  rejection paths.  The cross-round interleaving from patch 2's commit
  log crashes the kernel without patch 2 and survives with it.

Yun Lu (3):
  bpf: Fix NULL task dereference in task work cancellation
  bpf: Prevent task work context reuse while irq_work is busy
  selftests/bpf: Add task work scheduling race test

 kernel/bpf/helpers.c                          |  54 +++-
 .../selftests/bpf/prog_tests/test_task_work.c | 283 ++++++++++++++++++
 .../selftests/bpf/progs/task_work_race.c      | 126 ++++++++
 3 files changed, 453 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/task_work_race.c


base-commit: 5dd1818b15d98d4a20806cd00b1b40320b06004f
-- 
2.43.0


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

* [PATCH bpf-next v4 1/3] bpf: Fix NULL task dereference in task work cancellation
  2026-09-29  7:27 [PATCH bpf-next v4 0/3] bpf: Fix task work scheduling races Yun Lu
@ 2026-09-29  7:27 ` Yun Lu
  2026-09-29  7:27 ` [PATCH bpf-next v4 2/3] bpf: Prevent task work context reuse while irq_work is busy Yun Lu
  2026-09-29  7:28 ` [PATCH bpf-next v4 3/3] selftests/bpf: Add task work scheduling race test Yun Lu
  2 siblings, 0 replies; 8+ messages in thread
From: Yun Lu @ 2026-09-29  7:27 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_irq() publishes ctx->work with task_work_add() before
changing the context from SCHEDULING to SCHEDULED.  The target task can
run the callback on another CPU before the handler reaches its final
state transition.

The handler then reloads ctx->task for cancellation if its final cmpxchg
observes FREED.  By that point the callback or context destruction may
already have cleared ctx->task, causing task_work_cancel() to dereference
a NULL task pointer.

One concrete interleaving is:

  1. CPU 0 changes PENDING to SCHEDULING and successfully queues
     ctx->work with task_work_add(), but has not attempted the final
     state cmpxchg.

  2. The target task on CPU 1 runs the callback.  It changes SCHEDULING
     to RUNNING, executes the BPF subprogram, resets ctx->task,
     publishes STANDBY and drops the callback's context reference.

  3. CPU 2 deletes the map value.  It changes STANDBY to FREED and drops
     the map's context reference without queuing asynchronous
     cancellation.

  4. CPU 0 resumes.  Its SCHEDULING-to-SCHEDULED cmpxchg observes FREED,
     and the cancellation attempt passes NULL to task_work_cancel().

If deletion publishes FREED before the callback claims RUNNING, the
callback takes its FREED exit and drops its context reference.  Context
destruction can then clear ctx->task before CPU 0 attempts cancellation.

A controlled reproducer with a delay before the final cmpxchg triggered
the following fault on a v7.3-rc3 based x86-64 KVM guest:

  BUG: kernel NULL pointer dereference, address: 0000000000000890
  RIP: 0010: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
   </IRQ>

0x890 is the task_struct::task_works offset in that build, read through
the NULL task argument.

Capture ctx->task before task_work_add() publishes the callback and use
that pointer for the handler's cancellation attempt.  The round still
holds its task reference when the pointer is captured.  Releasing that
reference uses bpf_task_release(), which calls put_task_struct_rcu_user(),
so the handler's existing RCU read-side section keeps the captured task
alive even if ctx->task is subsequently cleared.

The asynchronous cancellation path reads ctx->task directly, which is
safe there: queuing it requires deletion to observe SCHEDULED, and that
xchg wins against the callback's transition to RUNNING, so the callback
takes its FREED exit without resetting ctx->task, while the map
reference transferred to the cancellation handler prevents context
destruction until cancellation completes.

The handler-tail and asynchronous cancellation attempts are mutually
exclusive: asynchronous cancellation is queued only if deletion observes
SCHEDULED after the scheduling handler's final cmpxchg has succeeded.
If that cmpxchg instead observes FREED, the scheduling handler attempts
cancellation with the captured pointer.

Fixes: 38aa7003e369 ("bpf: task work scheduling kfuncs")
Reported-by: Jackie Liu <liuyun01@kylinos.cn>
Suggested-by: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
 kernel/bpf/helpers.c | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b3cc5c8fc875..028fc6796b59 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -4437,15 +4437,21 @@ static void bpf_task_work_ctx_put(struct bpf_task_work_ctx *ctx)
 	}
 }
 
-static void bpf_task_work_cancel(struct bpf_task_work_ctx *ctx)
+static void bpf_task_work_cancel(struct bpf_task_work_ctx *ctx,
+				 struct task_struct *task)
 {
 	/*
 	 * 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 caller passes the task explicitly: the scheduling handler must
+	 * not read ctx->task after the work is published (the callback may
+	 * have reset it), while in the asynchronous cancellation path
+	 * ctx->task is stable, see bpf_task_work_cancel_scheduled().
 	 */
-	if (task_work_cancel(ctx->task, &ctx->work))
+	if (task_work_cancel(task, &ctx->work))
 		bpf_task_work_ctx_put(ctx);
 }
 
@@ -4486,6 +4492,7 @@ static void bpf_task_work_callback(struct callback_head *cb)
 static void bpf_task_work_irq(struct irq_work *irq_work)
 {
 	struct bpf_task_work_ctx *ctx = container_of(irq_work, struct bpf_task_work_ctx, irq_work);
+	struct task_struct *task;
 	enum bpf_task_work_state state;
 	int err;
 
@@ -4496,7 +4503,16 @@ static void bpf_task_work_irq(struct irq_work *irq_work)
 		return;
 	}
 
-	err = task_work_add(ctx->task, &ctx->work, ctx->mode);
+	/*
+	 * Capture the task pointer before the work is published: once the
+	 * callback runs, it may reset ctx->task. The capture happens while
+	 * this round's task reference is held, and every path that can
+	 * release it runs after this handler entered its rcu read-side
+	 * section. bpf_task_release() is put_task_struct_rcu_user(), so the
+	 * task_struct itself cannot be freed before this section ends.
+	 */
+	task = ctx->task;
+	err = task_work_add(task, &ctx->work, ctx->mode);
 	if (err) {
 		bpf_task_work_ctx_reset(ctx);
 		/*
@@ -4510,14 +4526,14 @@ static void bpf_task_work_irq(struct irq_work *irq_work)
 
 	/*
 	 * It's technically possible for just scheduled task_work callback to
-	 * complete running by now, going SCHEDULING -> RUNNING and then
-	 * dropping its ctx refcount. Instead of capturing an extra ref just
-	 * to protect below ctx->state access, we rely on rcu_read_lock
-	 * above to prevent kfree_rcu from freeing ctx before we return.
+	 * complete running by now, going SCHEDULING -> RUNNING, resetting
+	 * ctx->task and dropping its ctx refcount. The rcu read-side section
+	 * above keeps both the ctx memory and the captured task pointer
+	 * valid until this handler returns.
 	 */
 	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_cancel(ctx, task); /* clean up if we switched into FREED state */
 }
 
 static struct bpf_task_work_ctx *bpf_task_work_fetch_ctx(struct bpf_task_work *tw,
@@ -4777,7 +4793,13 @@ static void bpf_task_work_cancel_scheduled(struct irq_work *irq_work)
 {
 	struct bpf_task_work_ctx *ctx = container_of(irq_work, struct bpf_task_work_ctx, irq_work);
 
-	bpf_task_work_cancel(ctx); /* this might put task_work callback's ref */
+	/*
+	 * Deletion observed SCHEDULED and won against the callback's
+	 * transition to RUNNING. The callback takes its FREED exit without
+	 * resetting ctx->task, and the transferred map reference prevents
+	 * destruction until cancellation completes.
+	 */
+	bpf_task_work_cancel(ctx, ctx->task); /* 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 */
 }
 

-- 
2.43.0


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

* [PATCH bpf-next v4 2/3] bpf: Prevent task work context reuse while irq_work is busy
  2026-09-29  7:27 [PATCH bpf-next v4 0/3] bpf: Fix task work scheduling races Yun Lu
  2026-09-29  7:27 ` [PATCH bpf-next v4 1/3] bpf: Fix NULL task dereference in task work cancellation Yun Lu
@ 2026-09-29  7:27 ` Yun Lu
  2026-09-29 12:02   ` Mykyta Yatsenko
  2026-09-29  7:28 ` [PATCH bpf-next v4 3/3] selftests/bpf: Add task work scheduling race test Yun Lu
  2 siblings, 1 reply; 8+ messages in thread
From: Yun Lu @ 2026-09-29  7:27 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>

A bpf_task_work context can return to STANDBY before its scheduling
irq_work handler returns.  Both callback completion and task_work_add()
failure can publish STANDBY while the irq_work is still BUSY.  A new
round can then reinitialize and queue the same irq_work on another CPU
while the old handler is still running.

One concrete interleaving is:

  1. CPU 0 successfully queues ctx->work with task_work_add() and pauses
     before its SCHEDULING-to-SCHEDULED cmpxchg.

  2. The target task on CPU 1 completes the callback, resets ctx->task
     and publishes STANDBY.

  3. CPU 2 starts a new round, changes STANDBY to PENDING and queues the
     same irq_work.  The new handler changes PENDING to SCHEDULING, but
     task_work_add() returns -ESRCH because its target task has exited.
     The handler has not yet reset the context or rolled back the state.

  4. CPU 0 resumes and changes the new round's SCHEDULING to SCHEDULED,
     even though no task work was queued for that round.

  5. The new handler resets ctx->task, but its SCHEDULING-to-STANDBY
     cmpxchg fails.  The context remains SCHEDULED with a NULL task, and
     subsequent attempts to acquire this context fail with -EBUSY.

  6. Map-value deletion observes SCHEDULED and queues asynchronous
     cancellation, which passes the NULL ctx->task to task_work_cancel().

Overlapping invocations also share one BUSY bit.  An invocation can clear
that bit while the other invocation is still running.

Initialize the scheduling irq_work when the context is created and stop
reinitializing it for each round.  After claiming the context with the
STANDBY-to-PENDING cmpxchg, check BUSY and reject reuse with -EBUSY if a
previous invocation is still in progress.  Checking after the cmpxchg
orders the BUSY read after the STANDBY publication.

No irq_work has been queued for the attempted round at this point, so
the rejection can roll PENDING back to STANDBY.  Use cmpxchg for that
rollback to preserve FREED if a concurrent deletion has published it.
Once BUSY clears, scheduling can be retried.

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

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 028fc6796b59..350269b8646e 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -4553,6 +4553,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 */
 	ctx->state = BPF_TW_STANDBY;
+	init_irq_work(&ctx->irq_work, bpf_task_work_irq);
 
 	old_ctx = cmpxchg(&twk->ctx, NULL, ctx);
 	if (old_ctx) {
@@ -4595,6 +4596,18 @@ static struct bpf_task_work_ctx *bpf_task_work_acquire_ctx(struct bpf_task_work
 		bpf_task_work_ctx_put(ctx);
 		return ERR_PTR(-EBUSY);
 	}
+	/*
+	 * STANDBY can be published before the previous scheduling irq_work
+	 * returns. Claim PENDING before checking BUSY to order the check after
+	 * that publication. No irq_work has been queued for this attempt yet,
+	 * so reject reuse and roll back to STANDBY while BUSY is set. Preserve
+	 * FREED if deletion wins the rollback race.
+	 */
+	if (unlikely(irq_work_is_busy(&ctx->irq_work))) {
+		(void)cmpxchg(&ctx->state, BPF_TW_PENDING, BPF_TW_STANDBY);
+		bpf_task_work_ctx_put(ctx);
+		return ERR_PTR(-EBUSY);
+	}
 
 	/*
 	 * If no process or bpffs is holding a reference to the map, no new callbacks should be
@@ -4644,7 +4657,6 @@ static int bpf_task_work_schedule(struct task_struct *task, struct bpf_task_work
 	ctx->map = map;
 	ctx->map_val = (void *)tw - map->record->task_work_off;
 	init_task_work(&ctx->work, bpf_task_work_callback);
-	init_irq_work(&ctx->irq_work, bpf_task_work_irq);
 
 	irq_work_queue(&ctx->irq_work);
 	return 0;
-- 
2.43.0


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

* [PATCH bpf-next v4 3/3] selftests/bpf: Add task work scheduling race test
  2026-09-29  7:27 [PATCH bpf-next v4 0/3] bpf: Fix task work scheduling races Yun Lu
  2026-09-29  7:27 ` [PATCH bpf-next v4 1/3] bpf: Fix NULL task dereference in task work cancellation Yun Lu
  2026-09-29  7:27 ` [PATCH bpf-next v4 2/3] bpf: Prevent task work context reuse while irq_work is busy Yun Lu
@ 2026-09-29  7:28 ` Yun Lu
  2026-09-29  8:12   ` bot+bpf-ci
  2 siblings, 1 reply; 8+ messages in thread
From: Yun Lu @ 2026-09-29  7:28 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 task-work scheduling and callback execution on different CPUs
while userspace reuses or deletes the associated map value.  Run 1500
rounds rotating through context reuse, deletion after the callback has
recorded its generation, and deletion racing with scheduling.

Publish the scheduling result and its generation in one packed value, so
userspace does not rely on ordering between separate map slots.  Record
callbacks in separate per-generation slots so an earlier callback cannot
satisfy a later round's completion check.

Retry transient -EBUSY results while the context is unavailable for
reuse.  In the deletion-race case, allow -ENOENT if the element is
removed before a retry finds it.

Choose two CPUs from the process affinity mask and skip if fewer than
two are available.  Bound the thread-start wait, report thread creation
and affinity failures, and stop the worker threads with an atomic
handshake.

Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
 .../selftests/bpf/prog_tests/test_task_work.c | 283 ++++++++++++++++++
 .../selftests/bpf/progs/task_work_race.c      | 126 ++++++++
 2 files changed, 409 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..ff56976f4226 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,
+	/* High 32 bits hold the sequence, low 32 bits the signed error. */
+	RACE_RESULT,
+};
+
+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_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_RESULT, &value) &&
+		    (__u32)((__u64)value >> 32) == seq)
+			/*
+			 * The BPF program finished between the two reads:
+			 * RESULT makes waiting for READY unnecessary. Any error
+			 * that occurred before READY is checked by the caller.
+			 */
+			return 0;
+		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)
+{
+	if (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)
+{
+	int i;
+
+	for (i = 0; i < 2000000 / 100; i++) {
+		__s64 result = 0;
+
+		if (!task_work_race_status(skel, RACE_RESULT, &result) &&
+		    (__u32)((__u64)result >> 32) == seq)
+			return (__s32)result;
+		usleep(100);
+	}
+	return -ETIMEDOUT;
+}
+
+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;
+
+	for (i = 0; i < 5000000 / 100; i++) {
+		if (__atomic_load_n(&ctx.trigger_tid, __ATOMIC_ACQUIRE) &&
+		    __atomic_load_n(&ctx.target_tid, __ATOMIC_ACQUIRE))
+			break;
+		usleep(100);
+	}
+	if (!ASSERT_TRUE(__atomic_load_n(&ctx.trigger_tid, __ATOMIC_ACQUIRE) &&
+			 __atomic_load_n(&ctx.target_tid, __ATOMIC_ACQUIRE),
+			 "thread start"))
+		goto stop;
+	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);
+		/*
+		 * Variant 2 deletes the element right after READY, so losing
+		 * the race against the scheduling kfunc is a valid outcome:
+		 * the schedule itself fails with -EBUSY (ctx already FREED),
+		 * and the retry on the next tracepoint invocation fails with
+		 * -ENOENT because the element is gone.  Either way the round
+		 * exercised the deletion paths; a later variant 0 round
+		 * recreates the element.
+		 */
+		if (variant == 2 && (err == -EBUSY || err == -ENOENT))
+			err = 0;
+		if (!ASSERT_OK(err, "schedule")) {
+			fprintf(stderr, "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;
+
+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..c94eca510cb3
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/task_work_race.c
@@ -0,0 +1,126 @@
+// 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,
+	/* High 32 bits hold the sequence, low 32 bits the signed error. */
+	RACE_RESULT,
+	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, *result;
+	__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_RESULT;
+	result = bpf_map_lookup_elem(&status, &key);
+	if (!arm || !result || *arm <= 0 ||
+	    (__u32)((__u64)*result >> 32) == *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:
+	/* Publish the sequence and its error in one store. */
+	*result = ((__u64)seq << 32) | (__u32)err;
+	return 0;
+}
-- 
2.43.0


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

* Re: [PATCH bpf-next v4 3/3] selftests/bpf: Add task work scheduling race test
  2026-09-29  7:28 ` [PATCH bpf-next v4 3/3] selftests/bpf: Add task work scheduling race test Yun Lu
@ 2026-09-29  8:12   ` bot+bpf-ci
  2026-09-29 12:00     ` Mykyta Yatsenko
  0 siblings, 1 reply; 8+ messages in thread
From: bot+bpf-ci @ 2026-09-29  8:12 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: 4204 bytes --]

> selftests/bpf: Add task work scheduling race test
>
> Exercise task-work scheduling and callback execution on different CPUs
> while userspace reuses or deletes the associated map value.  Run 1500
> rounds rotating through context reuse, deletion after the callback has
> recorded its generation, and deletion racing with scheduling.
>
> Publish the scheduling result and its generation in one packed value, so
> userspace does not rely on ordering between separate map slots.  Record
> callbacks in separate per-generation slots so an earlier callback cannot
> satisfy a later round's completion check.
>
> Retry transient -EBUSY results while the context is unavailable for
> reuse.  In the deletion-race case, allow -ENOENT if the element is
> removed before a retry finds it.
>
> Choose two CPUs from the process affinity mask and skip if fewer than
> two are available.  Bound the thread-start wait, report thread creation
> and affinity failures, and stop the worker threads with an atomic
> handshake.
>
> Signed-off-by: Yun Lu <luyun@kylinos.cn>

This isn't a bug, but could the changelog say which of the preceding fixes
this test is meant to cover, and which interleaving each variant targets,
instead of listing implementation details like the bounded thread-start
wait and the atomic stop handshake?

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_task_work.c b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
> --- 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 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;
> +}

This isn't a bug, but could the trigger and target threads share one
function with a small per-thread argument (cpu and tid pointer)? And could
the repeated 2000000 / 100 poll bound become a named constant, like
TASK_WORK_RACE_ROUNDS?

[ ... ]

> +		err = task_work_race_check_done(skel, seq);
> +		/*
> +		 * Variant 2 deletes the element right after READY, so losing
> +		 * the race against the scheduling kfunc is a valid outcome:
> +		 * the schedule itself fails with -EBUSY (ctx already FREED),
> +		 * and the retry on the next tracepoint invocation fails with
> +		 * -ENOENT because the element is gone.  Either way the round
> +		 * exercised the deletion paths; a later variant 0 round
> +		 * recreates the element.
> +		 */
> +		if (variant == 2 && (err == -EBUSY || err == -ENOENT))
> +			err = 0;

This isn't a bug, but can err actually be -EBUSY here, given that
race_sched_work() returns without publishing RESULT on -EBUSY? If not,
could the condition just check -ENOENT?

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/task_work_race.c b/tools/testing/selftests/bpf/progs/task_work_race.c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/task_work_race.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/36538172291

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

* Re: [PATCH bpf-next v4 3/3] selftests/bpf: Add task work scheduling race test
  2026-09-29  8:12   ` bot+bpf-ci
@ 2026-09-29 12:00     ` Mykyta Yatsenko
  2026-09-30  8:37       ` luyun
  0 siblings, 1 reply; 8+ messages in thread
From: Mykyta Yatsenko @ 2026-09-29 12:00 UTC (permalink / raw)
  To: bot+bpf-ci, luyun_611, ast, daniel, andrii, eddyz87, memxor,
	martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai
  Cc: yatsenko, bpf, martin.lau, mason



On 9/29/26 9:12 AM, bot+bpf-ci@kernel.org wrote:
>> selftests/bpf: Add task work scheduling race test
>>
>> Exercise task-work scheduling and callback execution on different CPUs
>> while userspace reuses or deletes the associated map value.  Run 1500
>> rounds rotating through context reuse, deletion after the callback has
>> recorded its generation, and deletion racing with scheduling.
>>
>> Publish the scheduling result and its generation in one packed value, so
>> userspace does not rely on ordering between separate map slots.  Record
>> callbacks in separate per-generation slots so an earlier callback cannot
>> satisfy a later round's completion check.
>>
>> Retry transient -EBUSY results while the context is unavailable for
>> reuse.  In the deletion-race case, allow -ENOENT if the element is
>> removed before a retry finds it.
>>
>> Choose two CPUs from the process affinity mask and skip if fewer than
>> two are available.  Bound the thread-start wait, report thread creation
>> and affinity failures, and stop the worker threads with an atomic
>> handshake.
>>
>> Signed-off-by: Yun Lu <luyun@kylinos.cn>
> 
> This isn't a bug, but could the changelog say which of the preceding fixes
> this test is meant to cover, and which interleaving each variant targets,
> instead of listing implementation details like the bounded thread-start
> wait and the atomic stop handshake?
> 
>> diff --git a/tools/testing/selftests/bpf/prog_tests/test_task_work.c b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
>> --- 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 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;
>> +}
> 
> This isn't a bug, but could the trigger and target threads share one
> function with a small per-thread argument (cpu and tid pointer)? And could
> the repeated 2000000 / 100 poll bound become a named constant, like
> TASK_WORK_RACE_ROUNDS?
> 

I think this is worth addressing.

> [ ... ]
> 
>> +		err = task_work_race_check_done(skel, seq);
>> +		/*
>> +		 * Variant 2 deletes the element right after READY, so losing
>> +		 * the race against the scheduling kfunc is a valid outcome:
>> +		 * the schedule itself fails with -EBUSY (ctx already FREED),
>> +		 * and the retry on the next tracepoint invocation fails with
>> +		 * -ENOENT because the element is gone.  Either way the round
>> +		 * exercised the deletion paths; a later variant 0 round
>> +		 * recreates the element.
>> +		 */
>> +		if (variant == 2 && (err == -EBUSY || err == -ENOENT))
>> +			err = 0;
> 
> This isn't a bug, but can err actually be -EBUSY here, given that
> race_sched_work() returns without publishing RESULT on -EBUSY? If not,
> could the condition just check -ENOENT?
> 
> [ ... ]
> 
>> diff --git a/tools/testing/selftests/bpf/progs/task_work_race.c b/tools/testing/selftests/bpf/progs/task_work_race.c
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/task_work_race.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/36538172291


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

* Re: [PATCH bpf-next v4 2/3] bpf: Prevent task work context reuse while irq_work is busy
  2026-09-29  7:27 ` [PATCH bpf-next v4 2/3] bpf: Prevent task work context reuse while irq_work is busy Yun Lu
@ 2026-09-29 12:02   ` Mykyta Yatsenko
  0 siblings, 0 replies; 8+ messages in thread
From: Mykyta Yatsenko @ 2026-09-29 12:02 UTC (permalink / raw)
  To: Yun Lu, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai
  Cc: yatsenko, bpf



On 9/29/26 8:27 AM, Yun Lu wrote:
> From: Yun Lu <luyun@kylinos.cn>
> 
> A bpf_task_work context can return to STANDBY before its scheduling
> irq_work handler returns.  Both callback completion and task_work_add()
> failure can publish STANDBY while the irq_work is still BUSY.  A new
> round can then reinitialize and queue the same irq_work on another CPU
> while the old handler is still running.
> 
> One concrete interleaving is:
> 
>   1. CPU 0 successfully queues ctx->work with task_work_add() and pauses
>      before its SCHEDULING-to-SCHEDULED cmpxchg.
> 
>   2. The target task on CPU 1 completes the callback, resets ctx->task
>      and publishes STANDBY.
> 
>   3. CPU 2 starts a new round, changes STANDBY to PENDING and queues the
>      same irq_work.  The new handler changes PENDING to SCHEDULING, but
>      task_work_add() returns -ESRCH because its target task has exited.
>      The handler has not yet reset the context or rolled back the state.
> 
>   4. CPU 0 resumes and changes the new round's SCHEDULING to SCHEDULED,
>      even though no task work was queued for that round.
> 
>   5. The new handler resets ctx->task, but its SCHEDULING-to-STANDBY
>      cmpxchg fails.  The context remains SCHEDULED with a NULL task, and
>      subsequent attempts to acquire this context fail with -EBUSY.
> 
>   6. Map-value deletion observes SCHEDULED and queues asynchronous
>      cancellation, which passes the NULL ctx->task to task_work_cancel().
> 
> Overlapping invocations also share one BUSY bit.  An invocation can clear
> that bit while the other invocation is still running.
> 
> Initialize the scheduling irq_work when the context is created and stop
> reinitializing it for each round.  After claiming the context with the
> STANDBY-to-PENDING cmpxchg, check BUSY and reject reuse with -EBUSY if a
> previous invocation is still in progress.  Checking after the cmpxchg
> orders the BUSY read after the STANDBY publication.
> 
> No irq_work has been queued for the attempted round at this point, so
> the rejection can roll PENDING back to STANDBY.  Use cmpxchg for that
> rollback to preserve FREED if a concurrent deletion has published it.
> Once BUSY clears, scheduling can be retried.
> 
> Fixes: 38aa7003e369 ("bpf: task work scheduling kfuncs")
> Signed-off-by: Yun Lu <luyun@kylinos.cn>
> ---

Acked-by: Mykyta Yatsenko <yatsenko@meta.com>

>  kernel/bpf/helpers.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 028fc6796b59..350269b8646e 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -4553,6 +4553,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 */
>  	ctx->state = BPF_TW_STANDBY;
> +	init_irq_work(&ctx->irq_work, bpf_task_work_irq);
>  
>  	old_ctx = cmpxchg(&twk->ctx, NULL, ctx);
>  	if (old_ctx) {
> @@ -4595,6 +4596,18 @@ static struct bpf_task_work_ctx *bpf_task_work_acquire_ctx(struct bpf_task_work
>  		bpf_task_work_ctx_put(ctx);
>  		return ERR_PTR(-EBUSY);
>  	}
> +	/*
> +	 * STANDBY can be published before the previous scheduling irq_work
> +	 * returns. Claim PENDING before checking BUSY to order the check after
> +	 * that publication. No irq_work has been queued for this attempt yet,
> +	 * so reject reuse and roll back to STANDBY while BUSY is set. Preserve
> +	 * FREED if deletion wins the rollback race.
> +	 */
> +	if (unlikely(irq_work_is_busy(&ctx->irq_work))) {
> +		(void)cmpxchg(&ctx->state, BPF_TW_PENDING, BPF_TW_STANDBY);
> +		bpf_task_work_ctx_put(ctx);
> +		return ERR_PTR(-EBUSY);
> +	}
>  
>  	/*
>  	 * If no process or bpffs is holding a reference to the map, no new callbacks should be
> @@ -4644,7 +4657,6 @@ static int bpf_task_work_schedule(struct task_struct *task, struct bpf_task_work
>  	ctx->map = map;
>  	ctx->map_val = (void *)tw - map->record->task_work_off;
>  	init_task_work(&ctx->work, bpf_task_work_callback);
> -	init_irq_work(&ctx->irq_work, bpf_task_work_irq);
>  
>  	irq_work_queue(&ctx->irq_work);
>  	return 0;


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

* Re: [PATCH bpf-next v4 3/3] selftests/bpf: Add task work scheduling race test
  2026-09-29 12:00     ` Mykyta Yatsenko
@ 2026-09-30  8:37       ` luyun
  0 siblings, 0 replies; 8+ messages in thread
From: luyun @ 2026-09-30  8:37 UTC (permalink / raw)
  To: Mykyta Yatsenko, bot+bpf-ci, ast, daniel, andrii, eddyz87, memxor,
	martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai
  Cc: yatsenko, bpf, martin.lau, mason


在 2026/9/29 20:00, Mykyta Yatsenko 写道:
>
> On 9/29/26 9:12 AM, bot+bpf-ci@kernel.org wrote:
>>> selftests/bpf: Add task work scheduling race test
>>>
>>> Exercise task-work scheduling and callback execution on different CPUs
>>> while userspace reuses or deletes the associated map value.  Run 1500
>>> rounds rotating through context reuse, deletion after the callback has
>>> recorded its generation, and deletion racing with scheduling.
>>>
>>> Publish the scheduling result and its generation in one packed value, so
>>> userspace does not rely on ordering between separate map slots.  Record
>>> callbacks in separate per-generation slots so an earlier callback cannot
>>> satisfy a later round's completion check.
>>>
>>> Retry transient -EBUSY results while the context is unavailable for
>>> reuse.  In the deletion-race case, allow -ENOENT if the element is
>>> removed before a retry finds it.
>>>
>>> Choose two CPUs from the process affinity mask and skip if fewer than
>>> two are available.  Bound the thread-start wait, report thread creation
>>> and affinity failures, and stop the worker threads with an atomic
>>> handshake.
>>>
>>> Signed-off-by: Yun Lu <luyun@kylinos.cn>
>> This isn't a bug, but could the changelog say which of the preceding fixes
>> this test is meant to cover, and which interleaving each variant targets,
>> instead of listing implementation details like the bounded thread-start
>> wait and the atomic stop handshake?
>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/test_task_work.c b/tools/testing/selftests/bpf/prog_tests/test_task_work.c
>>> --- 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 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;
>>> +}
>> This isn't a bug, but could the trigger and target threads share one
>> function with a small per-thread argument (cpu and tid pointer)? And could
>> the repeated 2000000 / 100 poll bound become a named constant, like
>> TASK_WORK_RACE_ROUNDS?
>>
> I think this is worth addressing.
Hi, Mykyta

Thanks for your reviewing.
I carefully considered the 3 suggestions from the bot+bpf-ci,
and believe they are all necessary. I will incorporate them into
the v5 revision.

I will send out the v5 revision later (selftest changes only).

---

Thanks,

Yun Lu

>> [ ... ]
>>
>>> +		err = task_work_race_check_done(skel, seq);
>>> +		/*
>>> +		 * Variant 2 deletes the element right after READY, so losing
>>> +		 * the race against the scheduling kfunc is a valid outcome:
>>> +		 * the schedule itself fails with -EBUSY (ctx already FREED),
>>> +		 * and the retry on the next tracepoint invocation fails with
>>> +		 * -ENOENT because the element is gone.  Either way the round
>>> +		 * exercised the deletion paths; a later variant 0 round
>>> +		 * recreates the element.
>>> +		 */
>>> +		if (variant == 2 && (err == -EBUSY || err == -ENOENT))
>>> +			err = 0;
>> This isn't a bug, but can err actually be -EBUSY here, given that
>> race_sched_work() returns without publishing RESULT on -EBUSY? If not,
>> could the condition just check -ENOENT?
>>
>> [ ... ]
>>
>>> diff --git a/tools/testing/selftests/bpf/progs/task_work_race.c b/tools/testing/selftests/bpf/progs/task_work_race.c
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/bpf/progs/task_work_race.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/36538172291


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

end of thread, other threads:[~2026-09-30  8:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-29  7:27 [PATCH bpf-next v4 0/3] bpf: Fix task work scheduling races Yun Lu
2026-09-29  7:27 ` [PATCH bpf-next v4 1/3] bpf: Fix NULL task dereference in task work cancellation Yun Lu
2026-09-29  7:27 ` [PATCH bpf-next v4 2/3] bpf: Prevent task work context reuse while irq_work is busy Yun Lu
2026-09-29 12:02   ` Mykyta Yatsenko
2026-09-29  7:28 ` [PATCH bpf-next v4 3/3] selftests/bpf: Add task work scheduling race test Yun Lu
2026-09-29  8:12   ` bot+bpf-ci
2026-09-29 12:00     ` Mykyta Yatsenko
2026-09-30  8:37       ` luyun

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