[PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit

b10902118 posted 3 patches 5 months, 2 weeks ago
There is a newer version of this series
arch/arm/kernel/ptrace.c   | 16 ++++++++++++-
arch/arm64/kernel/ptrace.c | 49 ++++++++++++++++++++++++++++++++------
2 files changed, 57 insertions(+), 8 deletions(-)
[PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit
Posted by b10902118 5 months, 2 weeks ago
PTRACE_SETREGSET and PTRACE_SETHBPREGS fail 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:

The kernel modifies a breakpoint's addr and ctrl separately, but
each modification comes with validation, so the first validation
sees the user-provided addr and old ctrl (and vice versa for
PTRACE_SETHBPREGS). This combination can be invalid if the address
is unaligned and the control's length is too long (ex: the 4-byte
default).

The kernel only checks the alignment for 32-bit tracees, so this only
happens to them.


The Fix:

1. Fix PTRACE_SETREGSET by modifying addr and ctrl together.

2. Mitigate PTRACE_SETHBPREGS by minimizing default length.

   This cannot be fixed without removing or relaxing the alignment
   check because partially modified breakpoints are unavoidable for
   PTRACE_SETHBPREGS, which receives either addr or ctrl per call.


b10902118 (3):
  arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
  arm64: ptrace: minimize default bp_len for hw breakpoints to pass
    check
  ARM: ptrace: minimize default bp_len for hw breakpoints to pass check

 arch/arm/kernel/ptrace.c   | 16 ++++++++++++-
 arch/arm64/kernel/ptrace.c | 49 ++++++++++++++++++++++++++++++++------
 2 files changed, 57 insertions(+), 8 deletions(-)

-- 
2.50.1
[PATCH v2 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit
Posted by Bill Tsui 5 months, 2 weeks ago
Fixed author name. No code change.

Bill Tsui (3):
  arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
  arm64: ptrace: minimize default bp_len for hw breakpoints to pass
    check
  ARM: ptrace: minimize default bp_len for hw breakpoints to pass check

 arch/arm/kernel/ptrace.c   | 16 ++++++++++++-
 arch/arm64/kernel/ptrace.c | 49 ++++++++++++++++++++++++++++++++------
 2 files changed, 57 insertions(+), 8 deletions(-)

-- 
2.50.1
[PATCH v3 0/1] arm64: ptrace: fix hw_break_set() to set addr and ctrl together
Posted by Bill Tsui 3 months, 3 weeks ago
After revisiting my patches a few weeks later, I have some additional
thoughts:

  1. Keep arm32 ptrace as it. Legacy APIs only need critical bug fix.
  2. For arm64 ptrace, the patch is not only a fix for 32-bit tracee
     but also a refactor that removes partially set state, making logic
	 clearer and preventing similar bugs.

The patch can be tested with the code in my previous mail:
https://lore.kernel.org/all/0fd573cc044584f00976c410955ed486@ntu.edu.tw/
---

Changes in v3:
- Remove patches for arm32 and compat ptrace
- Rewrite commit message
- Reformat the arg of the added function

Changes in v2:
- Fix username

Bill Tsui (1):
  arm64: ptrace: fix hw_break_set() to set addr and ctrl together

 arch/arm64/kernel/ptrace.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

-- 
2.51.0
[PATCH v4 0/1] arm64: ptrace: fix hw_break_set() to set addr and ctrl together
Posted by Bill Tsui 3 months, 3 weeks ago
Fix the unused functions detected by kernel test robot.

Link: https://lore.kernel.org/all/202510181547.dI5kyPuT-lkp@intel.com/
---

Changes in v4:
- Fix build warning of unused functions by moving ptrace_hbp_set_ctrl()
  and ptrace_hbp_set_addr() under CONFIG_COMPAT and add compat_ prefix
  to the function names.

Changes in v3:
- Remove patches for arm32 and compat ptrace
- Rewrite commit message
- Reformat the arg of the added function

Changes in v2:
- Fix username

Bill Tsui (1):
  arm64: ptrace: fix hw_break_set() to set addr and ctrl together

 arch/arm64/kernel/ptrace.c | 90 ++++++++++++++++++++++++--------------
 1 file changed, 58 insertions(+), 32 deletions(-)

-- 
2.51.0
[PATCH v4 1/1] arm64: ptrace: fix hw_break_set() to set addr and ctrl together
Posted by Bill Tsui 3 months, 3 weeks ago
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. This is supported
by modify_user_hw_breakpoint(), the function that sets and checks
breakpoints. The original implementation wrap this function to
ptrace_hbp_set_addr() and ptrace_hbp_set_ctrl() for 32-bit API
PTRACE_SETHBPREGS simply because it can only modify one register (address
or control) per call. For the 64-bit PTRACE_SETREGSET API, this
restriction does not apply, so a new helper function ptrace_hbp_set() was
added to set both in a single call to modify_user_hw_breakpoint().

Then ptrace_hbp_set_addr() and ptrace_hbp_set_ctrl() are only used by
compat_ptrace_hbp_set(), so moved into CONFIG_COMPAT block and renamed
with prefix compat_.

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()

Signed-off-by: Bill Tsui <b10902118@ntu.edu.tw>
---
 arch/arm64/kernel/ptrace.c | 90 ++++++++++++++++++++++++--------------
 1 file changed, 58 insertions(+), 32 deletions(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 4b001121c72d..c8caf4e0805e 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -421,10 +421,11 @@ static struct perf_event *ptrace_hbp_get_initialised_bp(unsigned int note_type,
 	return bp;
 }
 
-static int ptrace_hbp_set_ctrl(unsigned int note_type,
-			       struct task_struct *tsk,
-			       unsigned long idx,
-			       u32 uctrl)
+/* Set the address and control together for non-compat ptrace */
+static int ptrace_hbp_set(unsigned int note_type,
+				   struct task_struct *tsk,
+				   unsigned long idx,
+				   u64 addr, u32 uctrl)
 {
 	int err;
 	struct perf_event *bp;
@@ -438,6 +439,8 @@ static int ptrace_hbp_set_ctrl(unsigned int note_type,
 	}
 
 	attr = bp->attr;
+	attr.bp_addr = addr;
+
 	decode_ctrl_reg(uctrl, &ctrl);
 	err = ptrace_hbp_fill_attr_ctrl(note_type, ctrl, &attr);
 	if (err)
@@ -446,27 +449,6 @@ static int ptrace_hbp_set_ctrl(unsigned int note_type,
 	return modify_user_hw_breakpoint(bp, &attr);
 }
 
-static int ptrace_hbp_set_addr(unsigned int note_type,
-			       struct task_struct *tsk,
-			       unsigned long idx,
-			       u64 addr)
-{
-	int err;
-	struct perf_event *bp;
-	struct perf_event_attr attr;
-
-	bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
-	if (IS_ERR(bp)) {
-		err = PTR_ERR(bp);
-		return err;
-	}
-
-	attr = bp->attr;
-	attr.bp_addr = addr;
-	err = modify_user_hw_breakpoint(bp, &attr);
-	return err;
-}
-
 #define PTRACE_HBP_ADDR_SZ	sizeof(u64)
 #define PTRACE_HBP_CTRL_SZ	sizeof(u32)
 #define PTRACE_HBP_PAD_SZ	sizeof(u32)
@@ -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);
 		if (ret)
 			return ret;
-		offset += PTRACE_HBP_CTRL_SZ;
 
 		user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
 					  offset, offset + PTRACE_HBP_PAD_SZ);
@@ -2139,6 +2119,52 @@ static int compat_ptrace_hbp_get(unsigned int note_type,
 	return err;
 }
 
+static int compat_ptrace_hbp_set_ctrl(unsigned int note_type,
+			       struct task_struct *tsk,
+			       unsigned long idx,
+			       u32 uctrl)
+{
+	int err;
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+	struct arch_hw_breakpoint_ctrl ctrl;
+
+	bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
+	if (IS_ERR(bp)) {
+		err = PTR_ERR(bp);
+		return err;
+	}
+
+	attr = bp->attr;
+	decode_ctrl_reg(uctrl, &ctrl);
+	err = ptrace_hbp_fill_attr_ctrl(note_type, ctrl, &attr);
+	if (err)
+		return err;
+
+	return modify_user_hw_breakpoint(bp, &attr);
+}
+
+static int compat_ptrace_hbp_set_addr(unsigned int note_type,
+			       struct task_struct *tsk,
+			       unsigned long idx,
+			       u64 addr)
+{
+	int err;
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+
+	bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
+	if (IS_ERR(bp)) {
+		err = PTR_ERR(bp);
+		return err;
+	}
+
+	attr = bp->attr;
+	attr.bp_addr = addr;
+	err = modify_user_hw_breakpoint(bp, &attr);
+	return err;
+}
+
 static int compat_ptrace_hbp_set(unsigned int note_type,
 				 struct task_struct *tsk,
 				 compat_long_t num,
@@ -2151,10 +2177,10 @@ static int compat_ptrace_hbp_set(unsigned int note_type,
 
 	if (num & 1) {
 		addr = *kdata;
-		err = ptrace_hbp_set_addr(note_type, tsk, idx, addr);
+		err = compat_ptrace_hbp_set_addr(note_type, tsk, idx, addr);
 	} else {
 		ctrl = *kdata;
-		err = ptrace_hbp_set_ctrl(note_type, tsk, idx, ctrl);
+		err = compat_ptrace_hbp_set_ctrl(note_type, tsk, idx, ctrl);
 	}
 
 	return err;
-- 
2.51.0
Re: [PATCH v4 1/1] arm64: ptrace: fix hw_break_set() to set addr and ctrl together
Posted by Will Deacon 2 months, 3 weeks ago
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
Re: [PATCH v4 1/1] arm64: ptrace: fix hw_break_set() to set addr and ctrl together
Posted by Bill Tsui 2 months, 3 weeks ago
On 2025-11-15 00:14, Will Deacon wrote:
> ... but you only implement this for the native (64-bit) case, so I 
> don't
> understand how it helps with the problem above.

Yes it is only for 64-bit tracer to debug 32-bit processes because
64-bit tracer always use 64-bit ptrace API (PTRACE_SETREGSET) regardless
of tracee, so only 64-bit code were changed.

>> @@ -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.
> 

The case doesn't exist for normal use. I saw that as a validation
rather than an intended stop point. If in doubt, I can remove it.

The signature from man ptrace(2)
	ptrace(PTRACE_SETREGSET, pid, NT_foo, &iov)
requires all registers' values to be specified in iov in order. And the
unit of debug registers is the inline structure in user_hwdebug_state:
	struct {
		__u64	addr;
		__u32	ctrl;
		__u32	pad;
	}		dbg_regs[16];
and userspace uses by:
	struct user_hwdebug_state dreg_state;
	ioVec.iov_base = &dreg_state;
	ioVec.iov_len = sizeof(dreg_state.dbg_info) + sizeof(dreg_state.pad) 
+				(sizeof(dreg_state.dbg_regs[0]) * m_max_hwp_supported);

	for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
		dreg_state.dbg_regs[i].addr = m_hwp_regs[i].address;
		dreg_state.dbg_regs[i].ctrl = m_hwp_regs[i].control;
	}
The only case of setting only address is to have iov length only reach
the first register's .addr, but I don't think that memory layout should
be relied on.

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

I am working on that, the side effects are hard to trace, and I think
this patch can make removing perf easier by reducing calls to it and
make clearer what hardware API ptrace needs. This recurs to me and I
think it can be a separated patch.

Here is my progress on removing perf:
I tried to directly set by hw_breakpoint_control, which perf finally
use, but we also relied on perf event handling cpu interrupts when
breakpoints get triggered. This is the hardest part and I am learning
how the kernel handles it across so many others. I feel that the use
of perf has its reasons but definitely there is space to improve.

Anyway, I respect maintainer's decision. I just felt that you thought
this is complicated so I simplifed it. Now I explains the correctness
and how it relates to your final goal. If there are still concerns,
please directly reject me, then I can also stop holding on it.

Thanks,
Bill
[PATCH v3 1/1] arm64: ptrace: fix hw_break_set() to set addr and ctrl together
Posted by Bill Tsui 3 months, 3 weeks ago
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. This is supported
by modify_user_hw_breakpoint(), the function that sets and checks
breakpoints. The original implementation wrap this function to
ptrace_hbp_set_addr() and ptrace_hbp_set_ctrl() for 32-bit API
PTRACE_SETHBPREGS simply because it can only modify one register (address
or control) per call. For the 64-bit PTRACE_SETREGSET API, this
restriction does not apply, so a new helper function ptrace_hbp_set() was
added to set both in a single call to modify_user_hw_breakpoint().

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()

Signed-off-by: Bill Tsui <b10902118@ntu.edu.tw>
---
 arch/arm64/kernel/ptrace.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 4b001121c..8963f40ba 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -467,6 +467,34 @@ static int ptrace_hbp_set_addr(unsigned int note_type,
 	return err;
 }
 
+/* Set the address and control together for non-compat ptrace */
+static int ptrace_hbp_set(unsigned int note_type,
+				   struct task_struct *tsk,
+				   unsigned long idx,
+				   u64 addr, u32 uctrl)
+{
+	int err;
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+	struct arch_hw_breakpoint_ctrl ctrl;
+
+	bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
+	if (IS_ERR(bp)) {
+		err = PTR_ERR(bp);
+		return err;
+	}
+
+	attr = bp->attr;
+	attr.bp_addr = addr;
+
+	decode_ctrl_reg(uctrl, &ctrl);
+	err = ptrace_hbp_fill_attr_ctrl(note_type, ctrl, &attr);
+	if (err)
+		return err;
+
+	return modify_user_hw_breakpoint(bp, &attr);
+}
+
 #define PTRACE_HBP_ADDR_SZ	sizeof(u64)
 #define PTRACE_HBP_CTRL_SZ	sizeof(u32)
 #define PTRACE_HBP_PAD_SZ	sizeof(u32)
@@ -524,9 +552,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 +562,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);
 		if (ret)
 			return ret;
-		offset += PTRACE_HBP_CTRL_SZ;
 
 		user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
 					  offset, offset + PTRACE_HBP_PAD_SZ);
-- 
2.51.0
Re: [PATCH v3 1/1] arm64: ptrace: fix hw_break_set() to set addr and ctrl together
Posted by kernel test robot 3 months, 3 weeks ago
Hi Bill,

kernel test robot noticed the following build warnings:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on linus/master v6.18-rc1 next-20251017]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bill-Tsui/arm64-ptrace-fix-hw_break_set-to-set-addr-and-ctrl-together/20251016-235711
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
patch link:    https://lore.kernel.org/r/20251016154401.35799-2-b10902118%40ntu.edu.tw
patch subject: [PATCH v3 1/1] arm64: ptrace: fix hw_break_set() to set addr and ctrl together
config: arm64-randconfig-r113-20251018 (https://download.01.org/0day-ci/archive/20251018/202510181547.dI5kyPuT-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251018/202510181547.dI5kyPuT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510181547.dI5kyPuT-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> arch/arm64/kernel/ptrace.c:449:12: warning: 'ptrace_hbp_set_addr' defined but not used [-Wunused-function]
    static int ptrace_hbp_set_addr(unsigned int note_type,
               ^~~~~~~~~~~~~~~~~~~
>> arch/arm64/kernel/ptrace.c:424:12: warning: 'ptrace_hbp_set_ctrl' defined but not used [-Wunused-function]
    static int ptrace_hbp_set_ctrl(unsigned int note_type,
               ^~~~~~~~~~~~~~~~~~~


vim +/ptrace_hbp_set_addr +449 arch/arm64/kernel/ptrace.c

478fcb2cdb2351 Will Deacon 2012-03-05  423  
478fcb2cdb2351 Will Deacon 2012-03-05 @424  static int ptrace_hbp_set_ctrl(unsigned int note_type,
478fcb2cdb2351 Will Deacon 2012-03-05  425  			       struct task_struct *tsk,
478fcb2cdb2351 Will Deacon 2012-03-05  426  			       unsigned long idx,
478fcb2cdb2351 Will Deacon 2012-03-05  427  			       u32 uctrl)
478fcb2cdb2351 Will Deacon 2012-03-05  428  {
478fcb2cdb2351 Will Deacon 2012-03-05  429  	int err;
478fcb2cdb2351 Will Deacon 2012-03-05  430  	struct perf_event *bp;
478fcb2cdb2351 Will Deacon 2012-03-05  431  	struct perf_event_attr attr;
478fcb2cdb2351 Will Deacon 2012-03-05  432  	struct arch_hw_breakpoint_ctrl ctrl;
478fcb2cdb2351 Will Deacon 2012-03-05  433  
478fcb2cdb2351 Will Deacon 2012-03-05  434  	bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
478fcb2cdb2351 Will Deacon 2012-03-05  435  	if (IS_ERR(bp)) {
478fcb2cdb2351 Will Deacon 2012-03-05  436  		err = PTR_ERR(bp);
478fcb2cdb2351 Will Deacon 2012-03-05  437  		return err;
478fcb2cdb2351 Will Deacon 2012-03-05  438  	}
478fcb2cdb2351 Will Deacon 2012-03-05  439  
478fcb2cdb2351 Will Deacon 2012-03-05  440  	attr = bp->attr;
478fcb2cdb2351 Will Deacon 2012-03-05  441  	decode_ctrl_reg(uctrl, &ctrl);
478fcb2cdb2351 Will Deacon 2012-03-05  442  	err = ptrace_hbp_fill_attr_ctrl(note_type, ctrl, &attr);
478fcb2cdb2351 Will Deacon 2012-03-05  443  	if (err)
478fcb2cdb2351 Will Deacon 2012-03-05  444  		return err;
478fcb2cdb2351 Will Deacon 2012-03-05  445  
478fcb2cdb2351 Will Deacon 2012-03-05  446  	return modify_user_hw_breakpoint(bp, &attr);
478fcb2cdb2351 Will Deacon 2012-03-05  447  }
478fcb2cdb2351 Will Deacon 2012-03-05  448  
478fcb2cdb2351 Will Deacon 2012-03-05 @449  static int ptrace_hbp_set_addr(unsigned int note_type,
478fcb2cdb2351 Will Deacon 2012-03-05  450  			       struct task_struct *tsk,
478fcb2cdb2351 Will Deacon 2012-03-05  451  			       unsigned long idx,
478fcb2cdb2351 Will Deacon 2012-03-05  452  			       u64 addr)
478fcb2cdb2351 Will Deacon 2012-03-05  453  {
478fcb2cdb2351 Will Deacon 2012-03-05  454  	int err;
478fcb2cdb2351 Will Deacon 2012-03-05  455  	struct perf_event *bp;
478fcb2cdb2351 Will Deacon 2012-03-05  456  	struct perf_event_attr attr;
478fcb2cdb2351 Will Deacon 2012-03-05  457  
478fcb2cdb2351 Will Deacon 2012-03-05  458  	bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
478fcb2cdb2351 Will Deacon 2012-03-05  459  	if (IS_ERR(bp)) {
478fcb2cdb2351 Will Deacon 2012-03-05  460  		err = PTR_ERR(bp);
478fcb2cdb2351 Will Deacon 2012-03-05  461  		return err;
478fcb2cdb2351 Will Deacon 2012-03-05  462  	}
478fcb2cdb2351 Will Deacon 2012-03-05  463  
478fcb2cdb2351 Will Deacon 2012-03-05  464  	attr = bp->attr;
478fcb2cdb2351 Will Deacon 2012-03-05  465  	attr.bp_addr = addr;
478fcb2cdb2351 Will Deacon 2012-03-05  466  	err = modify_user_hw_breakpoint(bp, &attr);
478fcb2cdb2351 Will Deacon 2012-03-05  467  	return err;
478fcb2cdb2351 Will Deacon 2012-03-05  468  }
478fcb2cdb2351 Will Deacon 2012-03-05  469  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
Posted by Bill Tsui 5 months, 2 weeks ago
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(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 4b001121c..73c67f743 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -467,6 +467,32 @@ static int ptrace_hbp_set_addr(unsigned int note_type,
 	return err;
 }
 
+/* Set the address and control together for non-compat ptrace */
+static int ptrace_hbp_set(unsigned int note_type, struct task_struct *tsk,
+			  unsigned long idx, u64 addr, u32 uctrl)
+{
+	int err;
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+	struct arch_hw_breakpoint_ctrl ctrl;
+
+	bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
+	if (IS_ERR(bp)) {
+		err = PTR_ERR(bp);
+		return err;
+	}
+
+	attr = bp->attr;
+	attr.bp_addr = addr;
+
+	decode_ctrl_reg(uctrl, &ctrl);
+	err = ptrace_hbp_fill_attr_ctrl(note_type, ctrl, &attr);
+	if (err)
+		return err;
+
+	return modify_user_hw_breakpoint(bp, &attr);
+}
+
 #define PTRACE_HBP_ADDR_SZ	sizeof(u64)
 #define PTRACE_HBP_CTRL_SZ	sizeof(u32)
 #define PTRACE_HBP_PAD_SZ	sizeof(u32)
@@ -524,9 +550,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 +560,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);
 		if (ret)
 			return ret;
-		offset += PTRACE_HBP_CTRL_SZ;
 
 		user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
 					  offset, offset + PTRACE_HBP_PAD_SZ);
-- 
2.50.1
Re: [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
Posted by Will Deacon 5 months ago
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
Re: [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
Posted by Bill Tsui 5 months ago
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
(Fixed email name with real name. Sorry again.)

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
Re: [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
Posted by Bill Tsui 4 months, 3 weeks ago
The following can reproduce the error for `arm64 compat` watchpoints
(breakpoints follow the same logic). I realized that testing my patch
can take time to set up from scratch, hoping this can facilitate the
review process.

There are three parts:
1. watch_unaligned.c
2. tracee.c
3. compile & run

Basically, watch_unaligned runs tracee and set 1-byte watchpoints on it.


1. watch_unaligned.c

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
#include <errno.h>
#include <sys/ptrace.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <linux/ptrace.h>
#include <asm/ptrace.h>
#include <sys/uio.h>
#include <elf.h>
#include <sys/uio.h>
#include <asm/ptrace.h>

#define MAX_WATCHPOINTS 4

#define AARCH64_BREAKPOINT_EL0 2

#define ENABLE 1

typedef enum {
     arm_hwbp_break = 0,
     arm_hwbp_load = 1,
     arm_hwbp_store = 2,
     arm_hwbp_access = 3
} arm_hwbp_type;

void error_detach_close(pid_t pid, FILE* pipe, const char* msg) {
     perror(msg);
     ptrace(PTRACE_DETACH, pid, NULL, NULL);
     pclose(pipe);
}

void watch_1byte(int pid, uint32_t addr) {
     // Set hardware watchpoint on ARM64
     // Use PTRACE_SETREGSET with NT_ARM_HW_WATCH for watchpoints
     printf("Setting watchpoint at address 0x%x\n", addr);
     struct user_hwdebug_state watch;
     memset(&watch, 0, sizeof(watch));
     watch.dbg_regs[0].addr = addr;
     uint32_t size = 1;
     uint32_t byte_mask = (1u << size) - 1u;
     uint32_t type = arm_hwbp_access;
     uint32_t priv = AARCH64_BREAKPOINT_EL0;
     watch.dbg_regs[0].ctrl =
         (byte_mask << 5) | (type << 3) | (priv << 1) | ENABLE;

     struct iovec ioVec;
     memset(&ioVec, 0, sizeof(ioVec));
     ioVec.iov_base = &watch;
     // According to lldb. Otherwise dbg_regs[16] will cause error
     ioVec.iov_len = sizeof(watch.dbg_info) + sizeof(watch.pad) +
                     (sizeof(watch.dbg_regs[0]) * MAX_WATCHPOINTS);

     if (ptrace(PTRACE_SETREGSET, pid, NT_ARM_HW_WATCH, &ioVec) == -1) {
         perror("PTRACE_SETREGSET");
     } else {
         printf("Watchpoint at address 0x%x set successfully\n", addr);
     }
     printf("---\n");
}

FILE* run_tracee(pid_t *pid, uint32_t *addr) {
     // run tracee and get pid & addr
     FILE *pipe = popen("./tracee", "r");
     if (!pipe) {
         perror("popen failed");
         return NULL;
     }

     char line[64];
     if (fgets(line, sizeof(line), pipe) == NULL) {
         perror("read pid failed");
     }
     *pid = atoi(line);

     if (fgets(line, sizeof(line), pipe) == NULL) {
         perror("read addr failed");
     }
     *addr = strtoul(line, NULL, 0);

     return pipe;
}

int main() {
     pid_t pid;
     uint32_t addr;

     FILE *pipe = run_tracee(&pid, &addr);
     if (!pipe) {
         return 1;
     }

     printf("Attaching to PID %d\n", pid);
     if (ptrace(PTRACE_ATTACH, pid, NULL, NULL) == -1) {
         error_detach_close(pid, pipe, "ptrace(ATTACH)");
         return 1;
     }
     printf("Attached\n\n");

     int status = 0;
     if (waitpid(pid, &status, 0) == -1) {
         error_detach_close(pid, pipe, "waitpid");
         return 1;
     }

     // The TEST

     // watch non-4-byte aligned. (Fail)
     watch_1byte(pid, addr);
     // watch any 4-byte aligned. (Success)
     watch_1byte(pid, 0);
     // watch non-4-byte aligned again. (Success)
     // because bp_len was previously set to 2 successfully
     watch_1byte(pid, addr);

     // continue and let it trigger, proving functionality
     if (ptrace(PTRACE_CONT, pid, NULL, NULL) == -1) {
         error_detach_close(pid, pipe,"ptrace(CONT)");
         return 1;
     }
     if (waitpid(pid, &status, 0) == -1) {
         error_detach_close(pid, pipe, "waitpid (after CONT)");
         pclose(pipe);
         return 1;
     }
     if (WIFSTOPPED(status)) {
         printf("Watchpoint triggered, signal: %d\n", WSTOPSIG(status));
         ptrace(PTRACE_CONT, pid, NULL, NULL);
         ptrace(PTRACE_DETACH, pid, NULL, NULL);
     }
     pclose(pipe);
     return 0;
}


2. tracee.c

#include <stdio.h>
#include <unistd.h>
#include <stdint.h>

int main() {
     volatile int a = 42;
     printf("%d\n%p\n", getpid(), (void*)((uintptr_t)&a + 1));
     fflush(stdout);
     while (1) {
         a += 1;
     }

     return 0;
}


3. compile & run

# 64-bit tracer
aarch64-linux-gnu-gcc watch_unaligned.c -o watch_unaligned

# 32-bit tracee
arm-linux-gnueabi-gcc tracee.c -o tracee

# run
./watch_unaligned

output:

     Attaching to PID 325
     Attached

     Setting watchpoint at address 0xffacca39
     PTRACE_SETREGSET: Invalid argument
     ---
     Setting watchpoint at address 0x0
     Watchpoint at address 0x0 set successfully
     ---
     Setting watchpoint at address 0xffacca39
     Watchpoint at address 0xffacca39 set successfully
     ---
     Watchpoint triggered, signal: 5

Summary: the first watchpoint failed but should be supported since it 
was
successfully set and triggered just after addr 0x0 with length 1.

Thanks,
Bill
Re: [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
Posted by b10902118 5 months ago
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
[PATCH v2 2/3] arm64: ptrace: minimize default bp_len for hw breakpoints to pass check
Posted by Bill Tsui 5 months, 2 weeks ago
PTRACE_SETHBPREGS 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 compat_ptrace_hbp_set() can only modify either
addr or ctrl of a breakpoint per call, but always checks alignment in
hw_breakpoint_arch_parse(). If a breakpoint has ctrl.len=4 (the
default), then users cannot set the addr to a non-4-byte aligned
address. Likewise, if the addr was previously set unaligned, users
cannot set ctrl.len to 4.

This patch mitigates the issue by minimizing the default bp_len, so
that any possibly valid address can pass the check with it. However, it
does not solve misaligned addr/len in modifying existing breakpoints;
further work may be needed to remove or relax the alignment check.

Signed-off-by: Bill Tsui <b10902118@ntu.edu.tw>
---
 arch/arm64/kernel/ptrace.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 73c67f743..70c9acd94 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -288,14 +288,25 @@ static struct perf_event *ptrace_hbp_create(unsigned int note_type,
 {
 	struct perf_event *bp;
 	struct perf_event_attr attr;
-	int err, type;
+	int err, type, min_len;
 
+	/*
+	 * min_len ensures any possibly valid address can pass alignment
+	 * validation with it.
+	 * This is for compat mode, in which addr and ctrl can only be set
+	 * one after one with SETHBPREGS. If addr is set first, ctrl will
+	 * remain as initial value.
+	 */
 	switch (note_type) {
 	case NT_ARM_HW_BREAK:
 		type = HW_BREAKPOINT_X;
+		min_len = (tsk && is_compat_thread(task_thread_info(tsk))) ?
+				  HW_BREAKPOINT_LEN_2 :
+				  HW_BREAKPOINT_LEN_4;
 		break;
 	case NT_ARM_HW_WATCH:
 		type = HW_BREAKPOINT_RW;
+		min_len = HW_BREAKPOINT_LEN_1;
 		break;
 	default:
 		return ERR_PTR(-EINVAL);
@@ -308,7 +319,7 @@ static struct perf_event *ptrace_hbp_create(unsigned int note_type,
 	 * (i.e. values that will pass validation).
 	 */
 	attr.bp_addr	= 0;
-	attr.bp_len	= HW_BREAKPOINT_LEN_4;
+	attr.bp_len	= min_len;
 	attr.bp_type	= type;
 	attr.disabled	= 1;
 
-- 
2.50.1
[PATCH v2 3/3] ARM: ptrace: minimize default bp_len for hw breakpoints to pass check
Posted by Bill Tsui 5 months, 2 weeks ago
PTRACE_SETHBPREGS 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 ptrace_sethbpregs() can only modify either
addr or ctrl of a breakpoint per call, but always checks alignment in
hw_breakpoint_arch_parse(). If a breakpoint has ctrl.len=4 (the
default), then users cannot set the addr to a non-4-byte aligned
address. Likewise, if the addr was previously set unaligned, users
cannot set ctrl.len to 4.

This patch mitigates the issue by minimizing the default bp_len, so
that any possibly valid address can pass the check with it. However, it
does not solve misaligned addr/len in modifying existing breakpoints;
further work may be needed to remove or relax the alignment check.

Signed-off-by: Bill Tsui <b10902118@ntu.edu.tw>
---
 arch/arm/kernel/ptrace.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 7951b2c06..920cf77d1 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -415,12 +415,26 @@ static u32 ptrace_get_hbp_resource_info(void)
 static struct perf_event *ptrace_hbp_create(struct task_struct *tsk, int type)
 {
 	struct perf_event_attr attr;
+	int min_len;
+
+	/*
+	 * min_len ensures any possibly valid address can pass alignment
+	 * validation with it.
+	 * In PTRACE_SETHBPREGS, addr and ctrl can only be set one after one. If
+	 * addr is set first, ctrl will remain as initial value.
+	 */
+	if (type == HW_BREAKPOINT_X)
+		min_len = (tsk && is_compat_thread(task_thread_info(tsk))) ?
+				  HW_BREAKPOINT_LEN_2 :
+				  HW_BREAKPOINT_LEN_4;
+	else // watchpoint
+		min_len = HW_BREAKPOINT_LEN_1;
 
 	ptrace_breakpoint_init(&attr);
 
 	/* Initialise fields to sane defaults. */
 	attr.bp_addr	= 0;
-	attr.bp_len	= HW_BREAKPOINT_LEN_4;
+	attr.bp_len	= min_len;
 	attr.bp_type	= type;
 	attr.disabled	= 1;
 
-- 
2.50.1
Re: [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit
Posted by Catalin Marinas 5 months, 2 weeks ago
On Sun, Aug 24, 2025 at 08:43:14PM +0800, b10902118 wrote:
> b10902118 (3):
>   arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
>   arm64: ptrace: minimize default bp_len for hw breakpoints to pass
>     check
>   ARM: ptrace: minimize default bp_len for hw breakpoints to pass check

We require real names from kernel contributors. Unless you can show
b10902118 is your name.

-- 
Catalin