linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit
@ 2025-08-24 12:43 b10902118
  2025-08-24 12:43 ` [PATCH 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together b10902118
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: b10902118 @ 2025-08-24 12:43 UTC (permalink / raw)
  To: oleg, linux, catalin.marinas, will
  Cc: linux-arm-kernel, linux-kernel, b10902118

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
  2025-08-24 12:43 [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit b10902118
@ 2025-08-24 12:43 ` b10902118
  2025-08-24 12:43 ` [PATCH 2/3] arm64: ptrace: minimize default bp_len for hw breakpoints to pass check b10902118
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: b10902118 @ 2025-08-24 12:43 UTC (permalink / raw)
  To: oleg, linux, catalin.marinas, will
  Cc: linux-arm-kernel, linux-kernel, b10902118

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: b10902118 <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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] arm64: ptrace: minimize default bp_len for hw breakpoints to pass check
  2025-08-24 12:43 [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit b10902118
  2025-08-24 12:43 ` [PATCH 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together b10902118
@ 2025-08-24 12:43 ` b10902118
  2025-08-24 12:43 ` [PATCH 3/3] ARM: " b10902118
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: b10902118 @ 2025-08-24 12:43 UTC (permalink / raw)
  To: oleg, linux, catalin.marinas, will
  Cc: linux-arm-kernel, linux-kernel, b10902118

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: b10902118 <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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] ARM: ptrace: minimize default bp_len for hw breakpoints to pass check
  2025-08-24 12:43 [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit b10902118
  2025-08-24 12:43 ` [PATCH 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together b10902118
  2025-08-24 12:43 ` [PATCH 2/3] arm64: ptrace: minimize default bp_len for hw breakpoints to pass check b10902118
@ 2025-08-24 12:43 ` b10902118
  2025-08-26 19:37 ` [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit Catalin Marinas
  2025-08-27  1:41 ` [PATCH v2 " Bill Tsui
  4 siblings, 0 replies; 9+ messages in thread
From: b10902118 @ 2025-08-24 12:43 UTC (permalink / raw)
  To: oleg, linux, catalin.marinas, will
  Cc: linux-arm-kernel, linux-kernel, b10902118

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: b10902118 <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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit
  2025-08-24 12:43 [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit b10902118
                   ` (2 preceding siblings ...)
  2025-08-24 12:43 ` [PATCH 3/3] ARM: " b10902118
@ 2025-08-26 19:37 ` Catalin Marinas
  2025-08-27  1:41 ` [PATCH v2 " Bill Tsui
  4 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2025-08-26 19:37 UTC (permalink / raw)
  To: b10902118; +Cc: oleg, linux, will, linux-arm-kernel, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit
  2025-08-24 12:43 [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit b10902118
                   ` (3 preceding siblings ...)
  2025-08-26 19:37 ` [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit Catalin Marinas
@ 2025-08-27  1:41 ` Bill Tsui
  2025-08-27  1:41   ` [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together Bill Tsui
                     ` (2 more replies)
  4 siblings, 3 replies; 9+ messages in thread
From: Bill Tsui @ 2025-08-27  1:41 UTC (permalink / raw)
  To: oleg, linux, catalin.marinas, will
  Cc: linux-arm-kernel, linux-kernel, Bill Tsui

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together
  2025-08-27  1:41 ` [PATCH v2 " Bill Tsui
@ 2025-08-27  1:41   ` Bill Tsui
  2025-08-27  1:41   ` [PATCH v2 2/3] arm64: ptrace: minimize default bp_len for hw breakpoints to pass check Bill Tsui
  2025-08-27  1:41   ` [PATCH v2 3/3] ARM: " Bill Tsui
  2 siblings, 0 replies; 9+ messages in thread
From: Bill Tsui @ 2025-08-27  1:41 UTC (permalink / raw)
  To: oleg, linux, catalin.marinas, will
  Cc: linux-arm-kernel, linux-kernel, Bill Tsui

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/3] arm64: ptrace: minimize default bp_len for hw breakpoints to pass check
  2025-08-27  1:41 ` [PATCH v2 " Bill Tsui
  2025-08-27  1:41   ` [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together Bill Tsui
@ 2025-08-27  1:41   ` Bill Tsui
  2025-08-27  1:41   ` [PATCH v2 3/3] ARM: " Bill Tsui
  2 siblings, 0 replies; 9+ messages in thread
From: Bill Tsui @ 2025-08-27  1:41 UTC (permalink / raw)
  To: oleg, linux, catalin.marinas, will
  Cc: linux-arm-kernel, linux-kernel, Bill Tsui

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] ARM: ptrace: minimize default bp_len for hw breakpoints to pass check
  2025-08-27  1:41 ` [PATCH v2 " Bill Tsui
  2025-08-27  1:41   ` [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together Bill Tsui
  2025-08-27  1:41   ` [PATCH v2 2/3] arm64: ptrace: minimize default bp_len for hw breakpoints to pass check Bill Tsui
@ 2025-08-27  1:41   ` Bill Tsui
  2 siblings, 0 replies; 9+ messages in thread
From: Bill Tsui @ 2025-08-27  1:41 UTC (permalink / raw)
  To: oleg, linux, catalin.marinas, will
  Cc: linux-arm-kernel, linux-kernel, Bill Tsui

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-08-27  1:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-24 12:43 [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit b10902118
2025-08-24 12:43 ` [PATCH 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together b10902118
2025-08-24 12:43 ` [PATCH 2/3] arm64: ptrace: minimize default bp_len for hw breakpoints to pass check b10902118
2025-08-24 12:43 ` [PATCH 3/3] ARM: " b10902118
2025-08-26 19:37 ` [PATCH 0/3] ARM/arm64: ptrace: fix unaligned hardware breakpoint validation for 32bit Catalin Marinas
2025-08-27  1:41 ` [PATCH v2 " Bill Tsui
2025-08-27  1:41   ` [PATCH v2 1/3] arm64: ptrace: fix hw_break_set() by setting addr and ctrl together Bill Tsui
2025-08-27  1:41   ` [PATCH v2 2/3] arm64: ptrace: minimize default bp_len for hw breakpoints to pass check Bill Tsui
2025-08-27  1:41   ` [PATCH v2 3/3] ARM: " Bill Tsui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).