Re: [RFC PATCH] ARM, ARM64, LOONGARCH: Delay HW BP notification to task_work()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Queueing the same hardware breakpoint callback_head multiple times corrupts the task_works list, leading to an infinite loop and Use-After-Free.
- [High] The use of TWA_RESUME fails to interrupt blocking system calls when a watchpoint triggers, potentially hanging the tracee.
--

--- Patch [1]: [RFC PATCH] ARM, ARM64, LOONGARCH: Delay HW BP notification to task_work() ---
commit edeb5ce1fcde3854d06fcd1845bb9a1a8e7ef02e
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

ARM, ARM64, LOONGARCH: Delay HW BP notification to task_work()

This patch delays the delivery of hardware breakpoint events to task_work()
to avoid sleeping while atomic warnings on PREEMPT_RT. It introduces a
generic callback perf_arch_hwbp_notify() that defers the architecture
specific signal generation to a return to user space.

Link: https://lore.kernel.org/all/aho0eqjMESuHxECr@redhat.com/

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 6d1f8bad7e1c5..9674cd59703fb 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c

[ ... ]

> @@ -13259,6 +13259,26 @@ static void account_event(struct perf_event *event)
>  	account_pmu_sb_event(event);
>  }
>  
> +#ifdef ARCH_NEED_PERF_HW_NOTIF
> +static void perf_arch_hwbp_send_sig(struct callback_head *head)
> +{
> +	struct perf_event *bp;
> +
> +	bp = container_of(head, struct perf_event, hw.arch_hw_notif);
> +	arch_hwbp_send_sig(bp);
> +	put_event(bp);
> +}
> +
> +void perf_arch_hwbp_notify(struct perf_event *bp, struct perf_sample_data *data,
> +			   struct pt_regs *regs)
> +{
> +	if (WARN_ON_ONCE(!atomic_long_inc_not_zero(&bp->refcount)))
> +		return;
> +	if (WARN_ON_ONCE(task_work_add(current, &bp->hw.arch_hw_notif, TWA_RESUME)))

[Severity: Critical]
If a user-space process sets a hardware watchpoint on a buffer and passes it
to a syscall that accesses the buffer multiple times (like copy_to_user),
can this queue the same callback_head multiple times before returning to
user space?

Because arch_hw_notif is a statically embedded callback_head, calling
task_work_add() a second time for the same event without checking if it is
already pending will overwrite its next pointer to point to itself. When
task_work_run() executes, the subsequent execution loop will loop infinitely
because next and work remain the same. 

The callback perf_arch_hwbp_send_sig() also calls put_event(bp), dropping
the refcount to zero and freeing the event. The next iteration reads
work->next and calls work->func(work) from freed memory, resulting in a
use-after-free and CPU lockup.

[Severity: High]
Is TWA_RESUME the correct mode here, or should it use TWA_SIGNAL?

Prior to this patch, hardware breakpoint handlers directly called
force_sig_fault(), which set TIF_SIGPENDING and properly interrupted
blocking system calls (like select or futex) to deliver the SIGTRAP.

TWA_RESUME only sets TIF_NOTIFY_RESUME and leaves signal_pending() false,
meaning the task could potentially block indefinitely without returning to
user space to process the task work, potentially deadlocking the tracee.

> +		put_event(bp);
> +}
> +#endif

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713144939.FuCj9yvZ@linutronix.de?part=1