[PATCH] arm64/hw_breakpoint: reject unaligned watchpoints that would truncate BAS

[Will Deacon]

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:
> > Hi Breno,
> > 
> > Thanks for sending this out.
> > 
> > On Thu, Apr 30, 2026 at 02:40:10AM -0700, Breno Leitao wrote:
> > > hw_breakpoint_arch_parse() positions the BAS bit pattern in
> > > hw->ctrl.len with
> > > 
> > > 	offset = hw->address & alignment_mask;	/* 0..7 */
> > > 	hw->ctrl.len <<= offset;
> > > 
> > > ctrl.len is an 8-bit bitfield (struct arch_hw_breakpoint_ctrl::len is
> > > u32 :8), so the shift silently drops any bits past bit 7.  For
> > > non-compat AArch64 watchpoints the offset is unbounded relative to
> > > ctrl.len: a perf_event_open(PERF_TYPE_BREAKPOINT) caller asking for
> > > HW_BREAKPOINT_W with bp_addr=page+1 and bp_len=HW_BREAKPOINT_LEN_8
> > > ends up with 0xff << 1 = 0x1fe, stored as 0xfe.  The kernel programs
> > > WCR.BAS=0xfe and the hardware watches bytes [1..7] instead of the
> > > requested [1..8] -- the eighth byte is silently dropped.  The
> > > syscall still returns success, leaving userspace to discover the
> > > gap by empirical probing.
> > > 
> > > The same class affects HW_BREAKPOINT_LEN_{2,4} when offset pushes the
> > > high BAS bit past bit 7 (e.g. LEN_4 with offset=5 yields 0xe0
> > > instead of 0x1e0).  No memory-safety impact -- the value is masked
> > > into 8 bits before encoding -- but debuggers and perf users observe
> > > missed events on bytes they thought they were watching.
> > > 
> > > The AArch32 branch immediately above already rejects unrepresentable
> > > (offset, len) combinations via an explicit switch.  Mirror that for
> > > the non-compat branch by checking that the shifted pattern fits in
> > > the BAS field, returning -EINVAL when it does not.
> > > 
> > > Reproducer:
> > > 
> > >   struct perf_event_attr a = {
> > >       .type = PERF_TYPE_BREAKPOINT, .size = sizeof(a),
> > >       .bp_type = HW_BREAKPOINT_W,
> > >       .bp_addr = (uintptr_t)(buf + 1),
> > >       .bp_len = HW_BREAKPOINT_LEN_8,
> > >       .exclude_kernel = 1, .exclude_hv = 1,
> > >   };
> > >   int fd = perf_event_open(&a, 0, -1, -1, 0);
> > >   /* before this fix: succeeds, watches 7 bytes (buf+1..buf+7)   */
> > >   /* after  this fix: fails with EINVAL                          */
> > > 
> > > Signed-off-by: Breno Leitao <leitao at debian.org>
> > > Fixes: b08fb180bb88 ("arm64: Allow hw watchpoint at varied offset from base address")
> > 
> > Oh man, this has been broken for nearly a decade :/
> > 
> > I think we probably should've stuck with the old behaviour of rejecting
> > unaligned base addresses, but it's too late now. Damn.
> > 
> > >  arch/arm64/kernel/hw_breakpoint.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> > > index ab76b36dce820..b8a1402119f3a 100644
> > > --- a/arch/arm64/kernel/hw_breakpoint.c
> > > +++ b/arch/arm64/kernel/hw_breakpoint.c
> > > @@ -559,6 +559,15 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
> > >  		else
> > >  			alignment_mask = 0x7;
> > >  		offset = hw->address & alignment_mask;
> > > +
> > > +		/*
> > > +		 * BAS is an 8-bit field in WCR/BCR; the shift below would
> > > +		 * silently drop the high bits of ctrl.len when offset + len
> > > +		 * exceeds 8, programming hardware to watch fewer bytes than
> > > +		 * the user requested.
> > > +		 */
> > > +		if (((u32)hw->ctrl.len << offset) > 0xff)
> > 
> > nit: Use ARM_BREAKPOINT_LEN_8 instead of 0xff
> > 
> > > +			return -EINVAL;
> > >  	}
> > 
> > I must confess, I'm very nervous about breaking userspace here. If GDB
> > is triggering this path, then this patch will change an unreliable
> > watchpoint into a hard error (which probably means GDB exits). Have you
> > looked to see what GDB and/or any other debuggers do?
> > 
> > I had a quick peek and found the bugzilla entry which motivated the
> > buggy change in the first place:
> > 
> > https://sourceware.org/bugzilla/show_bug.cgi?id=20207
> > 
> > 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()} ?

> Second, even if a debugger did hand the kernel such a request, GDB
> already treats EINVAL on NT_ARM_HW_WATCH as a downgrade signal, not a
> fatal error. aarch64_linux_set_debug_regs() catches it, clears
> kernel_supports_any_contiguous_range, calls aarch64_downgrade_regs()
> (which rounds the BAS up to a legacy 0x01/03/0f/ff mask and aligns the
> base down), and retries. That fallback is exactly the PR-20207 path.
> 
> Confirmed on a Grace box: with the patch applied (under virtme-ng), the
> reproducer's unaligned LEN_8 now returns -EINVAL while aligned LEN_8
> still succeeds, and GDB still inserts the watchpoint and it still fires
> on every write.

Thanks for testing it, that's somewhat reassuring.

> GDB in fact downgrades on the current kernel independently of this patch, so
> behaviour is unchanged for it.

That sounds like a bug?

Will