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

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

[Michael Neuling]

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 <mikey@neuling.org>
cc: <stable@vger.kernel.org> # 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

[PATCH 2/2] powerpc/ptrace: Fix setting 512B aligned breakpoints with PTRACE_SET_DEBUGREG

[Michael Neuling]

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 <mikey@neuling.org>
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

Re: [PATCH 2/2] powerpc/ptrace: Fix setting 512B aligned breakpoints with PTRACE_SET_DEBUGREG

[Michael Ellerman]

Michael Neuling <mikey@neuling.org> 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 <mikey@neuling.org>
> 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

Re: [PATCH 2/2] powerpc/ptrace: Fix setting 512B aligned breakpoints with PTRACE_SET_DEBUGREG

[Michael Neuling]

On Fri, 2018-05-18 at 22:56 +1000, Michael Ellerman wrote:
> Michael Neuling <mikey@neuling.org> writes:
> > 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 <mikey@neuling.org>
> > 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

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

[Michael Ellerman]

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 <mikey@neuling.org>
> cc: <stable@vger.kernel.org> # v3.9+

Series applied to powerpc next, thanks.

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

cheers