[PATCH 1/2] powerpc/ptrace: Fix enforcement of DAWR contraints

Back when we first introduced the DAWR in this commit:
  4ae7ebe952 powerpc: Change hardware breakpoint to allow longer ranges

We screwed up the constraint making it a 1024 byte boundary rather
than a 512. This makes the check overly permissive. Fortunately GDB is
the only real user and it always did they right thing, so we never
noticed.

This fixes the constraint to 512 bytes.

Signed-off-by: Michael Neuling <redacted>
cc: <redacted> # v3.9+
---
 arch/powerpc/kernel/hw_breakpoint.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 4c1012b80d..80547dad37 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c

@@ -178,8 +178,8 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
 	if (cpu_has_feature(CPU_FTR_DAWR)) {
 		length_max = 512 ; /* 64 doublewords */
 		/* DAWR region can't cross 512 boundary */
-		if ((bp->attr.bp_addr >> 10) != 
-		    ((bp->attr.bp_addr + bp->attr.bp_len - 1) >> 10))
+		if ((bp->attr.bp_addr >> 9) !=
+		    ((bp->attr.bp_addr + bp->attr.bp_len - 1) >> 9))
 			return -EINVAL;
 	}
 	if (info->len >

-- 
2.14.1

In this change:
  e2a800beac powerpc/hw_brk: Fix off by one error when validating DAWR region end

We fixed setting the DAWR end point to its max value via
PPC_PTRACE_SETHWDEBUG. Unfortunately we broke PTRACE_SET_DEBUGREG when
setting a 512 byte aligned breakpoint.

PTRACE_SET_DEBUGREG currently sets the length of the breakpoint to
zero (memset() in hw_breakpoint_init()).  This worked with
arch_validate_hwbkpt_settings() before the above patch was applied but
is now broken if the breakpoint is 512byte aligned.

This sets the length of the breakpoint to 8 bytes when using
PTRACE_SET_DEBUGREG.

Signed-off-by: Michael Neuling <redacted>
Cc: stable@vger.kernel.org # 3.10+
---
 arch/powerpc/kernel/ptrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index d23cf632ed..0f63dd5972 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c

@@ -2443,6 +2443,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
 	/* Create a new breakpoint request if one doesn't exist already */
 	hw_breakpoint_init(&attr);
 	attr.bp_addr = hw_brk.address;
+	attr.bp_len = 8;
 	arch_bp_generic_fields(hw_brk.type,
 			       &attr.bp_type);
 

-- 
2.14.1

Michael Neuling [off-list ref] writes:

In this change:
  e2a800beac powerpc/hw_brk: Fix off by one error when validating DAWR region end

We fixed setting the DAWR end point to its max value via
PPC_PTRACE_SETHWDEBUG. Unfortunately we broke PTRACE_SET_DEBUGREG when
setting a 512 byte aligned breakpoint.

PTRACE_SET_DEBUGREG currently sets the length of the breakpoint to
zero (memset() in hw_breakpoint_init()).  This worked with
arch_validate_hwbkpt_settings() before the above patch was applied but
is now broken if the breakpoint is 512byte aligned.

This sets the length of the breakpoint to 8 bytes when using
PTRACE_SET_DEBUGREG.

Signed-off-by: Michael Neuling <redacted>
Cc: stable@vger.kernel.org # 3.10+

If this is "fixing" e2a800beac then I think v3.11 is right for the
stable tag?

$ git describe --contains --long e2a800beaca1
v3.11-rc1~94^2~4

cheers

On Fri, 2018-05-18 at 22:56 +1000, Michael Ellerman wrote:

Michael Neuling [off-list ref] writes:

quoted

In this change:
  e2a800beac powerpc/hw_brk: Fix off by one error when validating DAWR
region end
=20
We fixed setting the DAWR end point to its max value via
PPC_PTRACE_SETHWDEBUG. Unfortunately we broke PTRACE_SET_DEBUGREG when
setting a 512 byte aligned breakpoint.
=20
PTRACE_SET_DEBUGREG currently sets the length of the breakpoint to
zero (memset() in hw_breakpoint_init()).  This worked with
arch_validate_hwbkpt_settings() before the above patch was applied but
is now broken if the breakpoint is 512byte aligned.
=20
This sets the length of the breakpoint to 8 bytes when using
PTRACE_SET_DEBUGREG.
=20
Signed-off-by: Michael Neuling <redacted>
Cc: stable@vger.kernel.org # 3.10+

=20
If this is "fixing" e2a800beac then I think v3.11 is right for the
stable tag?
=20
$ git describe --contains --long e2a800beaca1
v3.11-rc1~94^2~4

You're right. I think read the output of gitk incorrectly.

Thanks.
Mikey

On Thu, 2018-05-17 at 05:37:14 UTC, Michael Neuling wrote:

Back when we first introduced the DAWR in this commit:
  4ae7ebe952 powerpc: Change hardware breakpoint to allow longer ranges

We screwed up the constraint making it a 1024 byte boundary rather
than a 512. This makes the check overly permissive. Fortunately GDB is
the only real user and it always did they right thing, so we never
noticed.

This fixes the constraint to 512 bytes.

Signed-off-by: Michael Neuling <redacted>
cc: <redacted> # v3.9+

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/cd6ef7eebf171bfcba7dc2df719c2a

cheers