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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] User-space watchpoints with MTE-tagged addresses incorrectly fail validation checks and exact-match detection because tag bits are not stripped.
- [High] System register writes to debug registers are not immediately synchronized, violating architectural context synchronization rules.
- [High] Unaligned execution breakpoints bypass length validation and are programmed into hardware with invalid BAS constraints, corrupting RES0 bits.
--

commit 53ef92756387d64bcb01ab3be39ec7396912b68e
Author: Breno Leitao <leitao@debian.org>

arm64/hw_breakpoint: reject unaligned watchpoints that would truncate BAS

This patch addresses an issue where unaligned non-compat AArch64 watchpoints
could silently truncate the BAS bit pattern when the offset shifted the length
past 8 bits. The kernel now correctly rejects these unrepresentable length
and offset combinations with -EINVAL, preventing the hardware from watching
fewer bytes than requested.

> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index ab76b36dce820..73cce8ac83681 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;

[Severity: High]
This isn't a bug introduced by this patch, but I noticed that MTE-tagged
addresses might not be handled correctly here.

When perf_event_open() is called with an MTE-tagged user-space address,
arch_build_bp_info() saves the address into hw->address without untagging it.

Since hw->address retains the tag, could this cause the bounds check
(va >= TASK_SIZE) in arch_check_bp_in_kernelspace() to evaluate to true due
to the high-order tag bits? This would erroneously classify the user
watchpoint as a kernel watchpoint and reject it.

Additionally, in get_distance_from_watchpoint(), the tagged address
retrieved from the hardware register is used to calculate wp_low, which is
then mathematically compared against the explicitly untagged fault address:

arch/arm64/kernel/hw_breakpoint.c:get_distance_from_watchpoint() {
    addr = untagged_addr(addr);
    ...
    wp_low = val + lens;
    wp_high = val + lene;
    if (addr < wp_low)
    ...
}

Might this cause the distance calculation to silently fail the exact-match
watchpoint heuristic?

> +
> +		/*
> +		 * 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) > ARM_BREAKPOINT_LEN_8)
> +			return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does this new check allow unaligned
execution breakpoints to bypass length validation?

For AArch64 execution breakpoints, hw->ctrl.len is fixed at 0xF (4 bits).
If an unaligned address is provided (e.g., offset 1, 2, or 3), the shifted
length becomes 0x1E, 0x3C, or 0x78. Since these are less than 0xFF
(ARM_BREAKPOINT_LEN_8), they bypass this validation check.

When encode_ctrl_reg() executes (ctrl.len << 5), wouldn't these invalid
lengths overflow out of the 4-bit BAS field (bits [8:5] in DBGBCR_EL1)
and overwrite the RES0 bits [11:9]? 

This would also fail to program the architecturally mandated BAS value of
0b1111, leading to CONSTRAINED UNPREDICTABLE hardware behavior.

>  	}
>  
>  	hw->address &= ~alignment_mask;

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem with system
register synchronization in write_wb_reg():

arch/arm64/kernel/hw_breakpoint.c:write_wb_reg() {
...
#define WRITE_WB_REG_CASE(OFF, N, REG, VAL) \
	case (OFF + N): \
		AARCH64_DBG_WRITE(N, REG, VAL); \
		break
...
	switch (reg + n) {
	GEN_WRITE_WB_REG_CASES(AARCH64_DBG_REG_BVR, AARCH64_DBG_REG_NAME_BVR, val);
	...
	}
	isb();
}

According to ARM64 architectural rules, every write to a control-plane system
register must be followed by an isb() as the very next instruction.

Will the break statement in WRITE_WB_REG_CASE introduce an intervening branch
between the system register write and the isb() at the end of the switch
block?

Executing an intervening branch before the Context Synchronization Event (CSE)
means the branch observes an architecturally undefined pipeline state.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-arm64_bas-v2-1-9f34dbbded88@debian.org?part=1