Re: [PATCH v3] ARM, ARM64, LONGARCH, XTENSA: Delay HW BP notification to task_work()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] events: delayed or lost SIGTRAP for hardware watchpoints hit in blocking syscalls
--

commit f836aefea872e6050b4535461bc8040c46ae9a47
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

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

This commit addresses sleeping while atomic warnings on PREEMPT_RT caused by
hardware breakpoints on ARM64. It delays the hardware event callback
execution until the return to userland by queueing it via task_work_add().
The same pattern is also adopted for ARM, LongARCH, and Xtensa.

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

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index db7b76d6b68aa..b95e4d3a4f7c9 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
[ ... ]
> @@ -13374,6 +13374,28 @@ 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);
> +	xchg_relaxed(&bp->hw.arch_hw_notif_busy, 0);
> +	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 (xchg_relaxed(&bp->hw.arch_hw_notif_busy, 1) ||
> +	    WARN_ON_ONCE(task_work_add(current, &bp->hw.arch_hw_notif, TWA_RESUME)))

[Severity: Medium]
Could queuing the task work with TWA_RESUME instead of TWA_SIGNAL cause
delayed or lost SIGTRAP deliveries for hardware watchpoints?

If a tracee hits a hardware watchpoint while in a blocking syscall (for
example, when the syscall accesses memory via get_user or put_user),
using TWA_RESUME will only set TIF_NOTIFY_RESUME. 

Because it does not set TIF_SIGPENDING or TIF_NOTIFY_SIGNAL, the syscall
will not be interrupted. Does this allow the tracee to hang indefinitely in
the blocking syscall instead of aborting the syscall to deliver the
synchronous SIGTRAP?

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

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