From: Breno Leitao <leitao@debian.org>
To: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Pratyush Anand <panand@redhat.com>,
linux-arm-kernel@lists.infradead.org,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
clm@meta.com, leo.bras@arm.com, kernel-team@meta.com
Subject: Re: [PATCH] arm64/hw_breakpoint: reject unaligned watchpoints that would truncate BAS
Date: Wed, 10 Jun 2026 07:48:39 -0700 [thread overview]
Message-ID: <ail38x6jVAAU84AI@gmail.com> (raw)
In-Reply-To: <ailY2UgqiQNYxoZG@willie-the-truck>
On Wed, Jun 10, 2026 at 01:30:17PM +0100, Will Deacon wrote:
> On Mon, Jun 08, 2026 at 05:14:22AM -0700, Breno Leitao wrote:
> > On Fri, May 29, 2026 at 03:53:58PM +0100, Will Deacon wrote:
> > > and it looks like the aarch64_align_watchpoint() function does try to
> > > spill into multiple watchpoints, so perhaps your patch is ok. I'd
> > > appreciate your opinion, though.
> >
> > It won't, for two independent reasons.
>
> Sorry, not sure I understand you here when you say "it won't". GDB won't
> spill or something else?
>
> > The new -EINVAL is unreachable from GDB; only a raw perf_event_open() passing
> > an unaligned base with an oversized bp_len hits it, which is the bug.
>
> Why isn't it reachable via hw_break_set() => {ptrace_hbp_set_addr(),
> ptrace_hbp_set_ctrl()} ?
Sorry. What I should have said is: GDB handles that -EINVAL. From
gdb/nat/aarch64-linux-hw-point.c:
void
aarch64_linux_set_debug_regs (struct aarch64_debug_reg_state *state,
int tid, int watchpoint)
{
int i, count;
struct iovec iov;
struct user_hwdebug_state regs;
const CORE_ADDR *addr;
const unsigned int *ctrl;
memset (®s, 0, sizeof (regs));
iov.iov_base = ®s;
count = watchpoint ? aarch64_num_wp_regs : aarch64_num_bp_regs;
addr = watchpoint ? state->dr_addr_wp : state->dr_addr_bp;
ctrl = watchpoint ? state->dr_ctrl_wp : state->dr_ctrl_bp;
if (count == 0)
return;
iov.iov_len = (offsetof (struct user_hwdebug_state, dbg_regs)
+ count * sizeof (regs.dbg_regs[0]));
for (i = 0; i < count; i++)
{
regs.dbg_regs[i].addr = addr[i];
regs.dbg_regs[i].ctrl = ctrl[i];
}
if (ptrace (PTRACE_SETREGSET, tid,
watchpoint ? NT_ARM_HW_WATCH : NT_ARM_HW_BREAK,
(void *) &iov))
{
/* Handle Linux kernels with the PR external/20207 bug. */
if (watchpoint && errno == EINVAL
&& kernel_supports_any_contiguous_range)
{
kernel_supports_any_contiguous_range = false;
aarch64_downgrade_regs (state);
aarch64_linux_set_debug_regs (state, tid, watchpoint);
return;
}
error (_("Unexpected error setting hardware debug registers"));
}
}
From: https://fossies.org/linux/gdb/gdb/nat/aarch64-linux-hw-point.c
aarch64_downgrade_regs() rounds the BAS up to the nearest legacy
0x01/0x03/0x0f/0xff mask and aligns the base down with
align_down(..., AARCH64_HWP_ALIGNMENT)
The retry then succeeds. So a ptrace NT_ARM_HW_WATCH with an unaligned
base and an oversized BAS will, after this patch, go:
set -> -EINVAL -> downgrade -> set -> ok
> > GDB in fact downgrades on the current kernel independently of this patch, so
> > behaviour is unchanged for it.
>
> That sounds like a bug?
On an unpatched kernel GDB does NOT downgrade for the exact case this patch
fixes, precisely because the kernel returns 0 (with a truncated BAS) instead of
-EINVAL.
So the real behaviour delta this patch introduces for GDB is:
- Before: unaligned-base + oversized-len watchpoint silently watches
fewer bytes than requested. GDB never notices; user sees missed
events.
- After: same request returns -EINVAL, GDB's existing PR-20207
fallback engages, watchpoint is reinstalled with a legacy mask, and every
requested byte is covered (possibly with a few extra).
prev parent reply other threads:[~2026-06-10 14:48 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-30 9:40 [PATCH] arm64/hw_breakpoint: reject unaligned watchpoints that would truncate BAS Breno Leitao
2026-05-29 14:53 ` Will Deacon
2026-06-08 12:14 ` Breno Leitao
2026-06-10 12:30 ` Will Deacon
2026-06-10 14:48 ` Breno Leitao [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ail38x6jVAAU84AI@gmail.com \
--to=leitao@debian.org \
--cc=catalin.marinas@arm.com \
--cc=clm@meta.com \
--cc=kernel-team@meta.com \
--cc=leo.bras@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=panand@redhat.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.