Re: [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together

[b10902118 <b10902118@ntu.edu.tw>]

On 2025-09-08 23:14, Will Deacon wrote:
> On Wed, Aug 27, 2025 at 09:41:11AM +0800, Bill Tsui wrote:
>> PTRACE_SETREGSET fails when setting a hardware breakpoint on a
>> non-4-byte aligned address with a valid length to a 32-bit tracee. The
>> length should be valid as long as the range started from the address
>> is within one aligned 4 bytes.
>> 
>> The cause is that hw_break_set() modifies a breakpoint's addr
>> first and then ctrl. This calls modify_user_hw_breakpoint() twice,
>> although once registering both suffices. The first modification causes
>> errors because new addr and old ctrl can be an invalid combination at
>> hw_breakpoint_arch_parse(). For example, when a user sets a hardware
>> breakpoint with addr=0x2 and ctrl.len=1, hw_breakpoint_arch_parse()
>> will first see addr=0x2 and ctrl.len=4 (the default) and return
>> -EINVAL. On the other hand, if a user sets the same value to
>> a breakpoint whose ctrl.len has previously been set to 1 or 2,
>> it succeeds.
>> 
>> The fix is to set addr and ctrl in one modify_user_hw_breakpoint(),
>> effectively eliminating the discrepancy in validation.
>> 
>> Signed-off-by: Bill Tsui <b10902118@ntu.edu.tw>
>> ---
>>  arch/arm64/kernel/ptrace.c | 34 +++++++++++++++++++++++++++++-----
>>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> Given that:
> 
>   (a) This is a pretty niche interface (primarily/exclusively (?) used
>       by GDB)
>   (b) It's been like this for a long time
>   (c) Userspace can work around the problem
> 
> I'm not sure I see the benefit of trying to handle this differently
> in the kernel.
> 
> If somebody _does_ have the time and energy for significant surgery
> on this code, then the best thing to do would be to remove the
> indirection through 'perf_events' altogether. I did make a start on
> that once but it's a thankless job and I got preempted by other stuff.
> 
> Will

(This is the plain text version. Sorry for sending html in the previous 
mail)

Thanks for the reply. I did find this bug when helping test lldb.

The primary problem is that this forbids hardware breakpoints for Thumb 
at
addresses only divisible by 2 (not 4). So half of the instructions 
cannot have
a hardware breakpoint on. This cannot be worked around.

The first patch should be accepted, at least. The arm64 case can be 
completely
fixed by simply merging modifications in one call, making the code 
reasonable
and correct.

I often debug 32bit executables on arm64, for CTF challenges and my 
hobby. I
get extremely confused when some breakpoints just cannot be set. This is 
also
why I worked on lldb and finally traced to the kernel. As a student, I 
am willing
to take the task of moving ptrace out of 'perf_events' as long as 
someone can
review my patches.

Thanks
Bill