Re: [PATCH v4 1/1] arm64: ptrace: fix hw_break_set() to set addr and ctrl together

[Will Deacon <will@kernel.org>]

On Sat, Oct 18, 2025 at 09:37:31PM +0800, Bill Tsui wrote:
> This patch fixes the failure of PTRACE_SETREGSET when setting a hardware
> breakpoint on a non-4-byte aligned address with a valid control to a
> 32-bit tracee. The issue was discovered while testing LLDB.
> 
> Link: https://github.com/llvm/llvm-project/pull/152284
> 
> The failure happens because hw_break_set() checks and sets the breakpoint
> address and control separately. This can result in an check failure when
> it first validates the address to be set with old control.
> 
> For example, the control are initialized with breakpoint length of 4.
> Combining with a non-4-byte aligned address would cross a 4-byte boundary,
> which is invalid. However, the user-provided control may actually specify a
> length of 1, which should be valid.
> 
> The fix is to set the address and control together.

... but you only implement this for the native (64-bit) case, so I don't
understand how it helps with the problem above.

> For reference, the check is in
> 	arch/arm64/kernel/hw_breakpoint.c:hw_breakpoint_arch_parse()
> which is called via:
> 	modify_user_hw_breakpoint()
> 	-> modify_user_hw_breakpoint_check()
> 	-> hw_breakpoint_parse()
> 	-> hw_breakpoint_arch_parse()

You don't need to include these details here.

> @@ -524,9 +506,6 @@ static int hw_break_set(struct task_struct *target,
>  			return -EINVAL;
>  		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &addr,
>  					 offset, offset + PTRACE_HBP_ADDR_SZ);
> -		if (ret)
> -			return ret;
> -		ret = ptrace_hbp_set_addr(note_type, target, idx, addr);
>  		if (ret)
>  			return ret;
>  		offset += PTRACE_HBP_ADDR_SZ;
> @@ -537,10 +516,11 @@ static int hw_break_set(struct task_struct *target,
>  					 offset, offset + PTRACE_HBP_CTRL_SZ);
>  		if (ret)
>  			return ret;
> -		ret = ptrace_hbp_set_ctrl(note_type, target, idx, ctrl);
> +		offset += PTRACE_HBP_CTRL_SZ;
> +
> +		ret = ptrace_hbp_set(note_type, target, idx, addr, ctrl);

Doesn't this break the case where userspace tries only to set the address?
The loop will break out when !count without updating anything.

As I mentioned before, I'd prefer to leave this code as-is short of
removing the indirection through perf entirely.

Will