[PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it

[PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it

[chenyuan_fl]

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

Re: [PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it

[bot+bpf-ci]

[-- 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

Re: [PATCH] bpf: Claim the per-CPU send_signal irq_work before filling it

[Alexei Starovoitov]

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