Re: [PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too

2020-06-11 Thread Jan Kratochvil 
On Thu, 11 Jun 2020 12:58:31 +0200, Oleg Nesterov wrote:
> On 06/11, Madhavan Srinivasan wrote:
> > On 6/10/20 8:37 PM, Oleg Nesterov wrote:
> > > > This is not consistent and this breaks
> > > > http://sourceware.org/systemtap/wiki/utrace/tests/user-regs-peekpoke
> 
> this is 404.

Attaching the testcase, the CVS web interface no longer works on
sourceware.org.


Jan
/* Test case for PTRACE_SETREGS modifying the requested ragisters.
   x86* counterpart of the s390* testcase `user-area-access.c'.

   This software is provided 'as-is', without any express or implied
   warranty.  In no event will the authors be held liable for any damages
   arising from the use of this software.

   Permission is granted to anyone to use this software for any purpose,
   including commercial applications, and to alter it and redistribute it
   freely.  */

/* FIXME: EFLAGS should be tested restricted on the appropriate bits.  */

#define _GNU_SOURCE 1

#if defined __powerpc__ || defined __sparc__
# define user_regs_struct pt_regs
#endif

#ifdef __ia64__
#define ia64_fpreg ia64_fpreg_DISABLE
#define pt_all_user_regs pt_all_user_regs_DISABLE
#endif  /* __ia64__ */
#include 
#ifdef __ia64__
#undef ia64_fpreg
#undef pt_all_user_regs
#endif  /* __ia64__ */
#include 
#include 
#include 
#if defined __i386__ || defined __x86_64__
#include 
#endif
#include 

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

/* ia64 has PTRACE_SETREGS but it has no USER_REGS_STRUCT.  */
#if !defined PTRACE_SETREGS || defined __ia64__

int
main (void)
{
  return 77;
}

#else   /* PTRACE_SETREGS */

/* The minimal alignment we use for the random access ranges.  */
#define REGALIGN (sizeof (long))

static pid_t child;

static void
cleanup (void)
{
  if (child > 0)
kill (child, SIGKILL);
  child = 0;
}

static void
handler_fail (int signo)
{
  cleanup ();
  signal (SIGABRT, SIG_DFL);
  abort ();
}

int
main (void)
{
  long l;
  int status, i;
  pid_t pid;
  union
{
  struct user_regs_struct user;
  unsigned char byte[sizeof (struct user_regs_struct)];
} u, u2;
  int start;

  setbuf (stdout, NULL);
  atexit (cleanup);
  signal (SIGABRT, handler_fail);
  signal (SIGALRM, handler_fail);
  signal (SIGINT, handler_fail);
  i = alarm (10);
  assert (i == 0);

  child = fork ();
  switch (child)
{
case -1:
  assert_perror (errno);
  assert (0);

case 0:
  l = ptrace (PTRACE_TRACEME, 0, NULL, NULL);
  assert (l == 0);

  // Prevent rt_sigprocmask() call called by glibc after raise().
  syscall (__NR_tkill, getpid (), SIGSTOP);
  assert (0);

default:
  break;
}

  pid = waitpid (child, , 0);
  assert (pid == child);
  assert (WIFSTOPPED (status));
  assert (WSTOPSIG (status) == SIGSTOP);

  /* Fetch U2 from the inferior.  */
  errno = 0;
# ifdef __sparc__
  l = ptrace (PTRACE_GETREGS, child, , NULL);
# else
  l = ptrace (PTRACE_GETREGS, child, NULL, );
# endif
  assert_perror (errno);
  assert (l == 0);

  /* Initialize U with a pattern.  */
  for (i = 0; i < sizeof u.byte; i++)
u.byte[i] = i;
#ifdef __x86_64__
  /* non-EFLAGS modifications fail with EIO,  EFLAGS gets back different.  */
  u.user.eflags = u2.user.eflags;
  u.user.cs = u2.user.cs;
  u.user.ds = u2.user.ds;
  u.user.es = u2.user.es;
  u.user.fs = u2.user.fs;
  u.user.gs = u2.user.gs;
  u.user.ss = u2.user.ss;
  u.user.fs_base = u2.user.fs_base;
  u.user.gs_base = u2.user.gs_base;
  /* RHEL-4 refuses to set too high (and invalid) PC values.  */
  u.user.rip = (unsigned long) handler_fail;
  /* 2.6.25 always truncates and sign-extends orig_rax.  */
  u.user.orig_rax = (int) u.user.orig_rax;
#endif  /* __x86_64__ */
#ifdef __i386__
  /* These values get back different.  */
  u.user.xds = u2.user.xds;
  u.user.xes = u2.user.xes;
  u.user.xfs = u2.user.xfs;
  u.user.xgs = u2.user.xgs;
  u.user.xcs = u2.user.xcs;
  u.user.eflags = u2.user.eflags;
  u.user.xss = u2.user.xss;
  /* RHEL-4 refuses to set too high (and invalid) PC values.  */
  u.user.eip = (unsigned long) handler_fail;
#endif  /* __i386__ */
#ifdef __powerpc__
  /* These fields are constrained.  */
  u.user.msr = u2.user.msr;
# ifdef __powerpc64__
  u.user.softe = u2.user.softe;
# else
  u.user.mq = u2.user.mq;
# endif /* __powerpc64__ */
  u.user.trap = u2.user.trap;
  u.user.dar = u2.user.dar;
  u.user.dsisr = u2.user.dsisr;
  u.user.result = u2.user.result;
#endif  /* __powerpc__ */

  /* Poke U.  */
# ifdef __sparc__
  l = ptrace (PTRACE_SETREGS, child, , NULL);
# else
  l = ptrace (PTRACE_SETREGS, child, NULL, );
# endif
  assert (l == 0);

  /* Peek into U2.  */
# ifdef __sparc__
  l = ptrace (PTRACE_GETREGS, child, , NULL);
# else
  l = ptrace (PTRACE_GETREGS, child, NULL, );
# endif
  assert (l == 0);

  /* Verify it matches.  */
  if (memcmp (, , sizeof u.byte) != 0)
{
  for (start = 0; start + REGALIGN <= sizeof u.byte; start += REGALIGN)
if (*(unsigned long *) (u.byte +

Re: [PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too

2020-06-11 Thread Oleg Nesterov 
On 06/11, Madhavan Srinivasan wrote:
>
>
> On 6/10/20 8:37 PM, Oleg Nesterov wrote:
> >Hi,
> >
> >looks like this patch was forgotten.
>
> yep, I missed this. But mpe did have comments for the patch.
>
> https://lkml.org/lkml/2019/9/19/107

Yes, and I thought that I have replied... apparently not, sorry!

So let me repeat, I am fine either way, I do not understand this
ppc-specific code and I can't really test this change.

Let me quote that email from Michael:

> We could do it like below. I'm 50/50 though on whether it's worth it, 
or
> if we should just go with the big ifdef like in your patch.

up to you ;)

Hmm. And yes,

> >>This is not consistent and this breaks
> >>http://sourceware.org/systemtap/wiki/utrace/tests/user-regs-peekpoke

this is 404.

Jan, could correct the link above?

Oleg.

Re: [PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too

2020-06-11 Thread Madhavan Srinivasan 




On 6/10/20 8:37 PM, Oleg Nesterov wrote:

Hi,

looks like this patch was forgotten.


yep, I missed this. But mpe did have comments for the patch.

https://lkml.org/lkml/2019/9/19/107

Maddy


Do you think this should be fixed or should we document that
PTRACE_GETREGS is not consistent with PTRACE_PEEKUSER on ppc64?


On 09/17, Oleg Nesterov wrote:

I don't have a ppc machine, this patch wasn't even compile tested,
could you please review?

The commit a8a4b03ab95f ("powerpc: Hard wire PT_SOFTE value to 1 in
ptrace & signals") changed ptrace_get_reg(PT_SOFTE) to report 0x1,
but PTRACE_GETREGS still copies pt_regs->softe as is.

This is not consistent and this breaks
http://sourceware.org/systemtap/wiki/utrace/tests/user-regs-peekpoke

Reported-by: Jan Kratochvil 
Signed-off-by: Oleg Nesterov 
---
  arch/powerpc/kernel/ptrace.c | 25 +
  1 file changed, 25 insertions(+)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 8c92feb..291acfb 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -363,11 +363,36 @@ static int gpr_get(struct task_struct *target, const 
struct user_regset *regset,
BUILD_BUG_ON(offsetof(struct pt_regs, orig_gpr3) !=
 offsetof(struct pt_regs, msr) + sizeof(long));
  
+#ifdef CONFIG_PPC64

+   if (!ret)
+   ret = user_regset_copyout(, , , ,
+ >thread.regs->orig_gpr3,
+ offsetof(struct pt_regs, orig_gpr3),
+ offsetof(struct pt_regs, softe));
+
+   if (!ret) {
+   unsigned long softe = 0x1;
+   ret = user_regset_copyout(, , , , ,
+ offsetof(struct pt_regs, softe),
+ offsetof(struct pt_regs, softe) +
+ sizeof(softe));
+   }
+
+   BUILD_BUG_ON(offsetof(struct pt_regs, trap) !=
+offsetof(struct pt_regs, softe) + sizeof(long));
+
+   if (!ret)
+   ret = user_regset_copyout(, , , ,
+ >thread.regs->trap,
+ offsetof(struct pt_regs, trap),
+ sizeof(struct user_pt_regs));
+#else
if (!ret)
ret = user_regset_copyout(, , , ,
  >thread.regs->orig_gpr3,
  offsetof(struct pt_regs, orig_gpr3),
  sizeof(struct user_pt_regs));
+#endif
if (!ret)
ret = user_regset_copyout_zero(, , , ,
   sizeof(struct user_pt_regs), -1);
--
2.5.0

Re: [PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too

2020-06-10 Thread Oleg Nesterov 
Hi,

looks like this patch was forgotten.

Do you think this should be fixed or should we document that
PTRACE_GETREGS is not consistent with PTRACE_PEEKUSER on ppc64?


On 09/17, Oleg Nesterov wrote:
>
> I don't have a ppc machine, this patch wasn't even compile tested,
> could you please review?
> 
> The commit a8a4b03ab95f ("powerpc: Hard wire PT_SOFTE value to 1 in
> ptrace & signals") changed ptrace_get_reg(PT_SOFTE) to report 0x1,
> but PTRACE_GETREGS still copies pt_regs->softe as is.
> 
> This is not consistent and this breaks
> http://sourceware.org/systemtap/wiki/utrace/tests/user-regs-peekpoke
> 
> Reported-by: Jan Kratochvil 
> Signed-off-by: Oleg Nesterov 
> ---
>  arch/powerpc/kernel/ptrace.c | 25 +
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 8c92feb..291acfb 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -363,11 +363,36 @@ static int gpr_get(struct task_struct *target, const 
> struct user_regset *regset,
>   BUILD_BUG_ON(offsetof(struct pt_regs, orig_gpr3) !=
>offsetof(struct pt_regs, msr) + sizeof(long));
>  
> +#ifdef CONFIG_PPC64
> + if (!ret)
> + ret = user_regset_copyout(, , , ,
> +   >thread.regs->orig_gpr3,
> +   offsetof(struct pt_regs, orig_gpr3),
> +   offsetof(struct pt_regs, softe));
> +
> + if (!ret) {
> + unsigned long softe = 0x1;
> + ret = user_regset_copyout(, , , , ,
> +   offsetof(struct pt_regs, softe),
> +   offsetof(struct pt_regs, softe) +
> +   sizeof(softe));
> + }
> +
> + BUILD_BUG_ON(offsetof(struct pt_regs, trap) !=
> +  offsetof(struct pt_regs, softe) + sizeof(long));
> +
> + if (!ret)
> + ret = user_regset_copyout(, , , ,
> +   >thread.regs->trap,
> +   offsetof(struct pt_regs, trap),
> +   sizeof(struct user_pt_regs));
> +#else
>   if (!ret)
>   ret = user_regset_copyout(, , , ,
> >thread.regs->orig_gpr3,
> offsetof(struct pt_regs, orig_gpr3),
> sizeof(struct user_pt_regs));
> +#endif
>   if (!ret)
>   ret = user_regset_copyout_zero(, , , ,
>  sizeof(struct user_pt_regs), -1);
> -- 
> 2.5.0
>

[PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too

2019-09-17 Thread Oleg Nesterov 
I don't have a ppc machine, this patch wasn't even compile tested,
could you please review?

The commit a8a4b03ab95f ("powerpc: Hard wire PT_SOFTE value to 1 in
ptrace & signals") changed ptrace_get_reg(PT_SOFTE) to report 0x1,
but PTRACE_GETREGS still copies pt_regs->softe as is.

This is not consistent and this breaks
http://sourceware.org/systemtap/wiki/utrace/tests/user-regs-peekpoke

Reported-by: Jan Kratochvil 
Signed-off-by: Oleg Nesterov 
---
 arch/powerpc/kernel/ptrace.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 8c92feb..291acfb 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -363,11 +363,36 @@ static int gpr_get(struct task_struct *target, const 
struct user_regset *regset,
BUILD_BUG_ON(offsetof(struct pt_regs, orig_gpr3) !=
 offsetof(struct pt_regs, msr) + sizeof(long));
 
+#ifdef CONFIG_PPC64
+   if (!ret)
+   ret = user_regset_copyout(, , , ,
+ >thread.regs->orig_gpr3,
+ offsetof(struct pt_regs, orig_gpr3),
+ offsetof(struct pt_regs, softe));
+
+   if (!ret) {
+   unsigned long softe = 0x1;
+   ret = user_regset_copyout(, , , , ,
+ offsetof(struct pt_regs, softe),
+ offsetof(struct pt_regs, softe) +
+ sizeof(softe));
+   }
+
+   BUILD_BUG_ON(offsetof(struct pt_regs, trap) !=
+offsetof(struct pt_regs, softe) + sizeof(long));
+
+   if (!ret)
+   ret = user_regset_copyout(, , , ,
+ >thread.regs->trap,
+ offsetof(struct pt_regs, trap),
+ sizeof(struct user_pt_regs));
+#else
if (!ret)
ret = user_regset_copyout(, , , ,
  >thread.regs->orig_gpr3,
  offsetof(struct pt_regs, orig_gpr3),
  sizeof(struct user_pt_regs));
+#endif
if (!ret)
ret = user_regset_copyout_zero(, , , ,
   sizeof(struct user_pt_regs), -1);
-- 
2.5.0