linux-kernel - Re: [PATCH v4 1/1] arm64: ptrace: fix hw_break

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aRdVZmeOHYbm24NJ@willie-the-truck>
Date: Fri, 14 Nov 2025 16:14:30 +0000
From: Will Deacon <will@...nel.org>
To: Bill Tsui <b10902118@....edu.tw>
Cc: oleg@...hat.com, catalin.marinas@....com, nathan@...nel.org,
	nick.desaulniers+lkml@...il.com, morbo@...gle.com,
	justinstitt@...gle.com, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v4 1/1] arm64: ptrace: fix hw_break_set() to set addr and
 ctrl together

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