BPF List
 help / color / mirror / Atom feed
* [PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it
@ 2026-09-28  8:11 chenyuan_fl
  2026-09-28  9:05 ` bot+bpf-ci
  2026-10-02 11:32 ` Alexei Starovoitov
  0 siblings, 2 replies; 4+ messages in thread
From: chenyuan_fl @ 2026-09-28  8:11 UTC (permalink / raw)
  To: ast, daniel, bpf
  Cc: yonghong.song, andrii, eddyz87, memxor, martin.lau, song, jolsa,
	ihor.solodrai, linux-kernel, linux-trace-kernel, Yuan Chen

From: Yuan Chen <chenyuan@kylinos.cn>

irq_work_is_busy() cannot see the per-CPU send_signal_work while
it is being filled: the check only matches after irq_work_queue()
has claimed the work. An NMI interrupting the fill therefore passes
it, both callers race for the same irq_work, and the loser's signal
is silently lost along with its task reference while the queued
work runs with a mix of both callers' fields.

Claim the work with an atomic gate before touching any of its
fields and release it only after the callback has consumed them. A
context finding the work claimed returns the documented -EBUSY,
and the return value of irq_work_queue() is now handled.

Fixes: 1bc7896e9ef4 ("bpf: Fix deadlock with rq_lock in bpf_send_signal()")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/trace/bpf_trace.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 891897f8a1b3..03552637aab9 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -823,6 +823,8 @@ const struct bpf_func_proto bpf_task_pt_regs_proto = {
 
 struct send_signal_irq_work {
 	struct irq_work irq_work;
+	/* Covers the fill-to-run span which irq_work_is_busy() cannot see. */
+	atomic_t claimed;
 	struct task_struct *task;
 	u32 sig;
 	enum pid_type type;
@@ -842,6 +844,8 @@ static void do_bpf_send_signal(struct irq_work *entry)
 
 	group_send_sig_info(work->sig, siginfo, work->task, work->type);
 	put_task_struct(work->task);
+	/* Release once the fields are consumed. */
+	atomic_set_release(&work->claimed, 0);
 }
 
 static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struct *task, u64 value)
@@ -885,7 +889,7 @@ static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struc
 			return -EINVAL;
 
 		work = this_cpu_ptr(&send_signal_work);
-		if (irq_work_is_busy(&work->irq_work))
+		if (atomic_xchg(&work->claimed, 1))
 			return -EBUSY;
 
 		/* Add the current task, which is the target of sending signal,
@@ -898,7 +902,12 @@ static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struc
 			copy_siginfo(&work->info, &info);
 		work->sig = sig;
 		work->type = type;
-		irq_work_queue(&work->irq_work);
+		if (unlikely(!irq_work_queue(&work->irq_work))) {
+			/* Unreachable while the claim is held. */
+			put_task_struct(task);
+			atomic_set_release(&work->claimed, 0);
+			return -EBUSY;
+		}
 		return 0;
 	}
 
-- 
2.54.0


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

* Re: [PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it
  2026-09-28  8:11 [PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it chenyuan_fl
@ 2026-09-28  9:05 ` bot+bpf-ci
  2026-10-02 11:32 ` Alexei Starovoitov
  1 sibling, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-09-28  9:05 UTC (permalink / raw)
  To: chenyuan_fl, ast, daniel, bpf
  Cc: yonghong.song, andrii, eddyz87, memxor, martin.lau, song, jolsa,
	ihor.solodrai, linux-kernel, linux-trace-kernel, chenyuan, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, mason,
	ihor.solodrai

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

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 195f78db9bda..e60b4fdf2923 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -823,6 +823,8 @@ const struct bpf_func_proto bpf_task_pt_regs_proto = {
>
>  struct send_signal_irq_work {
>  	struct irq_work irq_work;
> +	/* Covers the fill-to-run span which irq_work_is_busy() cannot see. */
> +	atomic_t claimed;
>  	struct task_struct *task;
>  	u32 sig;
>  	enum pid_type type;
> @@ -842,6 +844,8 @@ static void do_bpf_send_signal(struct irq_work *entry)
>
>  	group_send_sig_info(work->sig, siginfo, work->task, work->type);
>  	put_task_struct(work->task);
> +	/* Release once the fields are consumed. */
> +	atomic_set_release(&work->claimed, 0);
>  }
>
>  static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struct *task, u64 value)
> @@ -885,7 +889,7 @@ static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struc
>  			return -EINVAL;
>
>  		work = this_cpu_ptr(&send_signal_work);
> -		if (irq_work_is_busy(&work->irq_work))
> +		if (atomic_xchg(&work->claimed, 1))
>  			return -EBUSY;
>
>  		/* Add the current task, which is the target of sending signal,
> @@ -898,7 +902,12 @@ static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struc
>  			copy_siginfo(&work->info, &info);
>  		work->sig = sig;
>  		work->type = type;
> -		irq_work_queue(&work->irq_work);
> +		if (unlikely(!irq_work_queue(&work->irq_work))) {
> +			/* Unreachable while the claim is held. */
> +			put_task_struct(task);
> +			atomic_set_release(&work->claimed, 0);
> +			return -EBUSY;
> +		}

This isn't a bug, but since the comment says this cannot happen while the
claim is held, would it be clearer to drop the branch or reduce it to a
WARN_ON_ONCE(!irq_work_queue(...))? As written it looks like a recovery
path, but it wouldn't actually undo the overwritten fields if it ever ran.

>  		return 0;
>  	}
>

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

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

* Re: [PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it
  2026-09-28  8:11 [PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it chenyuan_fl
  2026-09-28  9:05 ` bot+bpf-ci
@ 2026-10-02 11:32 ` Alexei Starovoitov
  2026-10-09  2:24   ` [PATCH v2 bpf-next] " Yuan Chen
  1 sibling, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2026-10-02 11:32 UTC (permalink / raw)
  To: chenyuan_fl, daniel, bpf
  Cc: yonghong.song, andrii, eddyz87, memxor, martin.lau, song, jolsa,
	ihor.solodrai, linux-kernel, linux-trace-kernel, Yuan Chen

On Mon, Sep 28, 2026 at 04:11 PM chenyuan_fl@163.com <chenyuan_fl@163.com> wrote:
> irq_work_is_busy() cannot see the per-CPU send_signal_work while
> it is being filled: the check only matches after irq_work_queue()
> has claimed the work. An NMI interrupting the fill therefore passes
> it, both callers race for the same irq_work, and the loser's signal
> is silently lost along with its task reference while the queued
> work runs with a mix of both callers' fields.

kprobe, tracepoint and perf_event progs exclude each other on a cpu
via bpf_prog_active, so one of the two progs has to be raw_tp or fentry.
And since commit 87c544108b61 ("bpf: Send signals asynchronously if
!preemptible") this path runs with irqs enabled too, so hard irq
can do the same. Not only NMI.
Pls describe it in the commit log.
Did you reproduce it or was it found by code inspection?

>  struct send_signal_irq_work {
>  	struct irq_work irq_work;
> +	/* Covers the fill-to-run span which irq_work_is_busy() cannot see. */
> +	atomic_t claimed;
>  	struct task_struct *task;

can work->task be the claim ?
cmpxchg(&work->task, NULL, task) instead of irq_work_is_busy() and
set it back to NULL at the end of do_bpf_send_signal().
Then no need for extra field.

> -		irq_work_queue(&work->irq_work);
> +		if (unlikely(!irq_work_queue(&work->irq_work))) {
> +			/* Unreachable while the claim is held. */
> +			put_task_struct(task);
> +			atomic_set_release(&work->claimed, 0);
> +			return -EBUSY;
> +		}

Drop this hunk. It's dead code.
irq_work_queue() fails only when IRQ_WORK_PENDING is set.
irq_work_single() clears it before calling do_bpf_send_signal()
and the claim is released at the end of it.
bpf_mmap_unlock_mm() doesn't check it either after
commit fa9dcacdcdf4 ("bpf: Fix mmap_lock leak in irq_work path").

Pls tag the respin as [PATCH v2 bpf-next].

pw-bot: cr

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

* [PATCH v2 bpf-next] bpf: Claim the per-CPU send_signal irq_work before filling it
  2026-10-02 11:32 ` Alexei Starovoitov
@ 2026-10-09  2:24   ` Yuan Chen
  0 siblings, 0 replies; 4+ messages in thread
From: Yuan Chen @ 2026-10-09  2:24 UTC (permalink / raw)
  To: daniel, bpf
  Cc: yonghong.song, andrii, eddyz87, memxor, martin.lau, song, jolsa,
	ihor.solodrai, linux-kernel, linux-trace-kernel,
	alexei.starovoitov, Yuan Chen

irq_work_is_busy() cannot see the per-CPU send_signal_work while
it is being filled: the check only matches after irq_work_queue()
has claimed the work. A concurrent caller therefore passes it, both
callers race for the same irq_work, and the loser's signal is
silently lost along with its task reference while the queued work
runs with a mix of both callers' fields.

kprobe, tracepoint and perf_event programs exclude each other on
the same CPU through bpf_prog_active, so for two callers to meet,
one of the programs has to be a raw tracepoint or an fentry one.
And since commit 87c544108b61 ("bpf: Send signals asynchronously
if !preemptible") this path runs with IRQs enabled too, so a hard
IRQ can interrupt the fill as well, not only an NMI. Found by code
inspection and reproduced with a perf_event NMI program racing an
fentry one.

Claim the work through work->task itself: cmpxchg() it from NULL
to the target task before touching the other fields, and set it
back to NULL once the callback has consumed them. A context
finding the work claimed returns the documented -EBUSY.

Fixes: 1bc7896e9ef4 ("bpf: Fix deadlock with rq_lock in bpf_send_signal()")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
Changes in v2:
- Drop the irq_work_queue() return-value handling, which is dead
  code: irq_work_single() clears IRQ_WORK_PENDING before the
  callback runs and the claim is only released after it, so the
  queue cannot fail while the claim is held.
- Use work->task itself as the claim gate, cmpxchg()ing it from
  NULL and back, so no extra field is needed.
- Describe which program pairs can actually race and that a hard
  IRQ can interrupt the fill since 87c544108b61, not only an NMI.
---
 kernel/trace/bpf_trace.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 195f78db9bda..5465e7eba238 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -823,6 +823,7 @@ const struct bpf_func_proto bpf_task_pt_regs_proto = {
 
 struct send_signal_irq_work {
 	struct irq_work irq_work;
+	/* The claim: NULL marks the slot free for the next caller. */
 	struct task_struct *task;
 	u32 sig;
 	enum pid_type type;
@@ -842,6 +843,8 @@ static void do_bpf_send_signal(struct irq_work *entry)
 
 	group_send_sig_info(work->sig, siginfo, work->task, work->type);
 	put_task_struct(work->task);
+	/* Release once the fields are consumed. */
+	smp_store_release(&work->task, NULL);
 }
 
 static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struct *task, u64 value)
@@ -885,14 +888,14 @@ static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struc
 			return -EINVAL;
 
 		work = this_cpu_ptr(&send_signal_work);
-		if (irq_work_is_busy(&work->irq_work))
+		if (cmpxchg(&work->task, NULL, task))
 			return -EBUSY;
 
 		/* Add the current task, which is the target of sending signal,
 		 * to the irq_work. The current task may change when queued
 		 * irq works get executed.
 		 */
-		work->task = get_task_struct(task);
+		get_task_struct(task);
 		work->has_siginfo = siginfo == &info;
 		if (work->has_siginfo)
 			copy_siginfo(&work->info, &info);
-- 
2.54.0


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

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

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-28  8:11 [PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it chenyuan_fl
2026-09-28  9:05 ` bot+bpf-ci
2026-10-02 11:32 ` Alexei Starovoitov
2026-10-09  2:24   ` [PATCH v2 bpf-next] " Yuan Chen

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