| Message ID | 20201119160247.GB5188@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 324a69467f12652b21b17f9644faa967d3d8bbdf |
| Headers | show |
| Series | powerpc/ptrace: Hard wire PT_SOFTE value to 1 in gpr_get() too | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (9d1aa2f025c6cc516125c42c70f6a9ce087c49ea) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 40 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On 11/19, Oleg Nesterov wrote: > > This is not consistent and this breaks the user-regs-peekpoke test > from https://sourceware.org/systemtap/wiki/utrace/tests/ See the test-case below. Oleg. /* 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 <sys/ptrace.h> #ifdef __ia64__ #undef ia64_fpreg #undef pt_all_user_regs #endif /* __ia64__ */ #include <linux/ptrace.h> #include <sys/types.h> #include <sys/user.h> #if defined __i386__ || defined __x86_64__ #include <sys/debugreg.h> #endif #include <asm/unistd.h> #include <assert.h> #include <errno.h> #include <error.h> #include <unistd.h> #include <signal.h> #include <stdlib.h> #include <stdio.h> #include <sys/wait.h> #include <sys/time.h> #include <string.h> #include <stddef.h> /* 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, &status, 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, &u2.user, NULL); # else l = ptrace (PTRACE_GETREGS, child, NULL, &u2.user); # 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, &u.user, NULL); # else l = ptrace (PTRACE_SETREGS, child, NULL, &u.user); # endif assert (l == 0); /* Peek into U2. */ # ifdef __sparc__ l = ptrace (PTRACE_GETREGS, child, &u2.user, NULL); # else l = ptrace (PTRACE_GETREGS, child, NULL, &u2.user); # endif assert (l == 0); /* Verify it matches. */ if (memcmp (&u.user, &u2.user, sizeof u.byte) != 0) { for (start = 0; start + REGALIGN <= sizeof u.byte; start += REGALIGN) if (*(unsigned long *) (u.byte + start) != *(unsigned long *) (u2.byte + start)) printf ("\ mismatch at offset %#x: SETREGS wrote %lx GETREGS read %lx\n", start, *(unsigned long *) (u.byte + start), *(unsigned long *) (u2.byte + start)); return 1; } /* Reverse the pattern. */ for (i = 0; i < sizeof u.byte; i++) u.byte[i] ^= -1; #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, &u.user, NULL); # else l = ptrace (PTRACE_SETREGS, child, NULL, &u.user); # endif assert (l == 0); /* Peek into U2. */ # ifdef __sparc__ l = ptrace (PTRACE_GETREGS, child, &u2.user, NULL); # else l = ptrace (PTRACE_GETREGS, child, NULL, &u2.user); # endif assert (l == 0); /* Verify it matches. */ if (memcmp (&u.user, &u2.user, sizeof u.byte) != 0) { for (start = 0; start + REGALIGN <= sizeof u.byte; start += REGALIGN) if (*(unsigned long *) (u.byte + start) != *(unsigned long *) (u2.byte + start)) printf ("\ mismatch at offset %#x: SETREGS wrote %lx GETREGS read %lx\n", start, *(unsigned long *) (u.byte + start), *(unsigned long *) (u2.byte + start)); return 1; } /* Now try poking arbitrary ranges and verifying it reads back right. We expect the U area is already a random enough pattern. */ for (start = 0; start + REGALIGN <= sizeof u.byte; start += REGALIGN) { for (i = start; i < start + REGALIGN; i++) u.byte[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; if (start > offsetof (struct pt_regs, ccr)) break; #endif /* __powerpc__ */ /* Poke U. */ l = ptrace (PTRACE_POKEUSER, child, (void *) (unsigned long) start, (void *) *(unsigned long *) (u.byte + start)); if (l != 0) error (1, errno, "PTRACE_POKEUSER at %x", start); /* Peek into U2. */ # ifdef __sparc__ l = ptrace (PTRACE_GETREGS, child, &u2.user, NULL); # else l = ptrace (PTRACE_GETREGS, child, NULL, &u2.user); # endif assert (l == 0); /* Verify it matches. */ if (memcmp (&u.user, &u2.user, sizeof u.byte) != 0) { printf ("mismatch at offset %#x: poked %lx but GETREGS read %lx\n", start, *(unsigned long *) (u.byte + start), *(unsigned long *) (u2.byte + start)); return 1; } } /* Now try peeking arbitrary ranges and verifying it is the same. We expect the U area is already a random enough pattern. */ for (start = 0; start + REGALIGN <= sizeof u.byte; start += REGALIGN) { /* Peek for the U comparation. */ errno = 0; l = ptrace (PTRACE_PEEKUSER, child, (void *) (unsigned long) start, NULL); assert_perror (errno); /* Verify it matches. */ if (*(unsigned long *) (u.byte + start) != l) { printf ("mismatch at offset %#x: poked %lx but peeked %lx\n", start, *(unsigned long *) (u.byte + start), l); return 1; } } return 0; } #endif /* PTRACE_SETREGS */
Le 19/11/2020 à 17:02, Oleg Nesterov a écrit : > 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 the user-regs-peekpoke test > from https://sourceware.org/systemtap/wiki/utrace/tests/ > > Reported-by: Jan Kratochvil <jan.kratochvil@redhat.com> > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > --- > arch/powerpc/kernel/ptrace/ptrace-tm.c | 8 +++++++- > arch/powerpc/kernel/ptrace/ptrace-view.c | 8 +++++++- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kernel/ptrace/ptrace-tm.c b/arch/powerpc/kernel/ptrace/ptrace-tm.c > index f8fcbd85d4cb..d0d339f86e61 100644 > --- a/arch/powerpc/kernel/ptrace/ptrace-tm.c > +++ b/arch/powerpc/kernel/ptrace/ptrace-tm.c > @@ -87,6 +87,10 @@ int tm_cgpr_get(struct task_struct *target, const struct user_regset *regset, > struct membuf to) > { > struct membuf to_msr = membuf_at(&to, offsetof(struct pt_regs, msr)); > +#ifdef CONFIG_PPC64 > + struct membuf to_softe = membuf_at(&to, > + offsetof(struct pt_regs, softe)); Should fit on a single line I think. > +#endif > > if (!cpu_has_feature(CPU_FTR_TM)) > return -ENODEV; > @@ -102,7 +106,9 @@ int tm_cgpr_get(struct task_struct *target, const struct user_regset *regset, > sizeof(struct user_pt_regs)); > > membuf_store(&to_msr, get_user_ckpt_msr(target)); > - > +#ifdef CONFIG_PPC64 > + membuf_store(&to_softe, 0x1ul); > +#endif > return membuf_zero(&to, ELF_NGREG * sizeof(unsigned long) - > sizeof(struct user_pt_regs)); > } > diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c b/arch/powerpc/kernel/ptrace/ptrace-view.c > index 39686ede40b3..f554ccfcbfae 100644 > --- a/arch/powerpc/kernel/ptrace/ptrace-view.c > +++ b/arch/powerpc/kernel/ptrace/ptrace-view.c > @@ -218,6 +218,10 @@ static int gpr_get(struct task_struct *target, const struct user_regset *regset, > struct membuf to) > { > struct membuf to_msr = membuf_at(&to, offsetof(struct pt_regs, msr)); > +#ifdef CONFIG_PPC64 > + struct membuf to_softe = membuf_at(&to, > + offsetof(struct pt_regs, softe)); Should fit on a single line I think. > +#endif > int i; > > if (target->thread.regs == NULL) > @@ -233,7 +237,9 @@ static int gpr_get(struct task_struct *target, const struct user_regset *regset, > sizeof(struct user_pt_regs)); > > membuf_store(&to_msr, get_user_msr(target)); > - > +#ifdef CONFIG_PPC64 > + membuf_store(&to_softe, 0x1ul); > +#endif > return membuf_zero(&to, ELF_NGREG * sizeof(unsigned long) - > sizeof(struct user_pt_regs)); > } > Christophe
Quoting Oleg Nesterov <oleg@redhat.com>: > 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 the user-regs-peekpoke test > from https://sourceware.org/systemtap/wiki/utrace/tests/ > > Reported-by: Jan Kratochvil <jan.kratochvil@redhat.com> > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > --- > arch/powerpc/kernel/ptrace/ptrace-tm.c | 8 +++++++- > arch/powerpc/kernel/ptrace/ptrace-view.c | 8 +++++++- > 2 files changed, 14 insertions(+), 2 deletions(-) > I think the following should work, and not require the first patch (compile tested only). diff --git a/arch/powerpc/kernel/ptrace/ptrace-tm.c b/arch/powerpc/kernel/ptrace/ptrace-tm.c index 54f2d076206f..f779b3bc0279 100644 --- a/arch/powerpc/kernel/ptrace/ptrace-tm.c +++ b/arch/powerpc/kernel/ptrace/ptrace-tm.c @@ -104,8 +104,14 @@ int tm_cgpr_get(struct task_struct *target, const struct user_regset *regset, offsetof(struct pt_regs, msr) + sizeof(long)); membuf_write(&to, &target->thread.ckpt_regs.orig_gpr3, - sizeof(struct user_pt_regs) - - offsetof(struct pt_regs, orig_gpr3)); + offsetof(struct pt_regs, softe) - offsetof(struct pt_regs, orig_gpr3)); + membuf_store(&to, 1UL); + + BUILD_BUG_ON(offsetof(struct pt_regs, trap) != + offsetof(struct pt_regs, softe) + sizeof(long)); + + membuf_write(&to, &target->thread.ckpt_regs.trap, + sizeof(struct user_pt_regs) - offsetof(struct pt_regs, trap)); return membuf_zero(&to, ELF_NGREG * sizeof(unsigned long) - sizeof(struct user_pt_regs)); } diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c b/arch/powerpc/kernel/ptrace/ptrace-view.c index 7e6478e7ed07..736bfbf33890 100644 --- a/arch/powerpc/kernel/ptrace/ptrace-view.c +++ b/arch/powerpc/kernel/ptrace/ptrace-view.c @@ -234,9 +234,21 @@ 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 + membuf_write(&to, &target->thread.regs->orig_gpr3, + offsetof(struct pt_regs, softe) - offsetof(struct pt_regs, orig_gpr3)); + membuf_store(&to, 1UL); + + BUILD_BUG_ON(offsetof(struct pt_regs, trap) != + offsetof(struct pt_regs, softe) + sizeof(long)); + + membuf_write(&to, &target->thread.regs->trap, + sizeof(struct user_pt_regs) - offsetof(struct pt_regs, trap)); +#else membuf_write(&to, &target->thread.regs->orig_gpr3, sizeof(struct user_pt_regs) - offsetof(struct pt_regs, orig_gpr3)); +#endif return membuf_zero(&to, ELF_NGREG * sizeof(unsigned long) - sizeof(struct user_pt_regs)); } --- Christophe
On 11/19, Christophe Leroy wrote: > > I think the following should work, and not require the first patch (compile > tested only). > > --- a/arch/powerpc/kernel/ptrace/ptrace-view.c > +++ b/arch/powerpc/kernel/ptrace/ptrace-view.c > @@ -234,9 +234,21 @@ 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 > + membuf_write(&to, &target->thread.regs->orig_gpr3, > + offsetof(struct pt_regs, softe) - offsetof(struct pt_regs, > orig_gpr3)); > + membuf_store(&to, 1UL); > + > + BUILD_BUG_ON(offsetof(struct pt_regs, trap) != > + offsetof(struct pt_regs, softe) + sizeof(long)); > + > + membuf_write(&to, &target->thread.regs->trap, > + sizeof(struct user_pt_regs) - offsetof(struct pt_regs, trap)); > +#else > membuf_write(&to, &target->thread.regs->orig_gpr3, > sizeof(struct user_pt_regs) - > offsetof(struct pt_regs, orig_gpr3)); > +#endif > return membuf_zero(&to, ELF_NGREG * sizeof(unsigned long) - > sizeof(struct user_pt_regs)); > } Probably yes. This mirrors the previous patch I sent (https://lore.kernel.org/lkml/20190917143753.GA12300@redhat.com/) and this is exactly what I tried to avoid, we can make a simpler fix now. But let me repeat, I agree with any fix even if imp my version simplifies the code, just commit this change and lets forget this problem. Oleg.
Christophe, et al, So what? Are you going to push your change or should I re-send 1-2 without whitespace cleanups? On 11/19, Oleg Nesterov wrote: > > On 11/19, Christophe Leroy wrote: > > > > I think the following should work, and not require the first patch (compile > > tested only). > > > > --- a/arch/powerpc/kernel/ptrace/ptrace-view.c > > +++ b/arch/powerpc/kernel/ptrace/ptrace-view.c > > @@ -234,9 +234,21 @@ 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 > > + membuf_write(&to, &target->thread.regs->orig_gpr3, > > + offsetof(struct pt_regs, softe) - offsetof(struct pt_regs, > > orig_gpr3)); > > + membuf_store(&to, 1UL); > > + > > + BUILD_BUG_ON(offsetof(struct pt_regs, trap) != > > + offsetof(struct pt_regs, softe) + sizeof(long)); > > + > > + membuf_write(&to, &target->thread.regs->trap, > > + sizeof(struct user_pt_regs) - offsetof(struct pt_regs, trap)); > > +#else > > membuf_write(&to, &target->thread.regs->orig_gpr3, > > sizeof(struct user_pt_regs) - > > offsetof(struct pt_regs, orig_gpr3)); > > +#endif > > return membuf_zero(&to, ELF_NGREG * sizeof(unsigned long) - > > sizeof(struct user_pt_regs)); > > } > > Probably yes. > > This mirrors the previous patch I sent (https://lore.kernel.org/lkml/20190917143753.GA12300@redhat.com/) > and this is exactly what I tried to avoid, we can make a simpler fix now. > > But let me repeat, I agree with any fix even if imp my version simplifies the code, just > commit this change and lets forget this problem. > > Oleg.
Oleg Nesterov <oleg@redhat.com> writes: > Christophe, et al, > > So what? > > Are you going to push your change or should I re-send 1-2 without > whitespace cleanups? I'll take your 1 & 2 and fixup the whitespace issues when applying. cheers > On 11/19, Oleg Nesterov wrote: >> >> On 11/19, Christophe Leroy wrote: >> > >> > I think the following should work, and not require the first patch (compile >> > tested only). >> > >> > --- a/arch/powerpc/kernel/ptrace/ptrace-view.c >> > +++ b/arch/powerpc/kernel/ptrace/ptrace-view.c >> > @@ -234,9 +234,21 @@ 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 >> > + membuf_write(&to, &target->thread.regs->orig_gpr3, >> > + offsetof(struct pt_regs, softe) - offsetof(struct pt_regs, >> > orig_gpr3)); >> > + membuf_store(&to, 1UL); >> > + >> > + BUILD_BUG_ON(offsetof(struct pt_regs, trap) != >> > + offsetof(struct pt_regs, softe) + sizeof(long)); >> > + >> > + membuf_write(&to, &target->thread.regs->trap, >> > + sizeof(struct user_pt_regs) - offsetof(struct pt_regs, trap)); >> > +#else >> > membuf_write(&to, &target->thread.regs->orig_gpr3, >> > sizeof(struct user_pt_regs) - >> > offsetof(struct pt_regs, orig_gpr3)); >> > +#endif >> > return membuf_zero(&to, ELF_NGREG * sizeof(unsigned long) - >> > sizeof(struct user_pt_regs)); >> > } >> >> Probably yes. >> >> This mirrors the previous patch I sent (https://lore.kernel.org/lkml/20190917143753.GA12300@redhat.com/) >> and this is exactly what I tried to avoid, we can make a simpler fix now. >> >> But let me repeat, I agree with any fix even if imp my version simplifies the code, just >> commit this change and lets forget this problem. >> >> Oleg.
diff --git a/arch/powerpc/kernel/ptrace/ptrace-tm.c b/arch/powerpc/kernel/ptrace/ptrace-tm.c index f8fcbd85d4cb..d0d339f86e61 100644 --- a/arch/powerpc/kernel/ptrace/ptrace-tm.c +++ b/arch/powerpc/kernel/ptrace/ptrace-tm.c @@ -87,6 +87,10 @@ int tm_cgpr_get(struct task_struct *target, const struct user_regset *regset, struct membuf to) { struct membuf to_msr = membuf_at(&to, offsetof(struct pt_regs, msr)); +#ifdef CONFIG_PPC64 + struct membuf to_softe = membuf_at(&to, + offsetof(struct pt_regs, softe)); +#endif if (!cpu_has_feature(CPU_FTR_TM)) return -ENODEV; @@ -102,7 +106,9 @@ int tm_cgpr_get(struct task_struct *target, const struct user_regset *regset, sizeof(struct user_pt_regs)); membuf_store(&to_msr, get_user_ckpt_msr(target)); - +#ifdef CONFIG_PPC64 + membuf_store(&to_softe, 0x1ul); +#endif return membuf_zero(&to, ELF_NGREG * sizeof(unsigned long) - sizeof(struct user_pt_regs)); } diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c b/arch/powerpc/kernel/ptrace/ptrace-view.c index 39686ede40b3..f554ccfcbfae 100644 --- a/arch/powerpc/kernel/ptrace/ptrace-view.c +++ b/arch/powerpc/kernel/ptrace/ptrace-view.c @@ -218,6 +218,10 @@ static int gpr_get(struct task_struct *target, const struct user_regset *regset, struct membuf to) { struct membuf to_msr = membuf_at(&to, offsetof(struct pt_regs, msr)); +#ifdef CONFIG_PPC64 + struct membuf to_softe = membuf_at(&to, + offsetof(struct pt_regs, softe)); +#endif int i; if (target->thread.regs == NULL) @@ -233,7 +237,9 @@ static int gpr_get(struct task_struct *target, const struct user_regset *regset, sizeof(struct user_pt_regs)); membuf_store(&to_msr, get_user_msr(target)); - +#ifdef CONFIG_PPC64 + membuf_store(&to_softe, 0x1ul); +#endif return membuf_zero(&to, ELF_NGREG * sizeof(unsigned long) - sizeof(struct user_pt_regs)); }
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 the user-regs-peekpoke test from https://sourceware.org/systemtap/wiki/utrace/tests/ Reported-by: Jan Kratochvil <jan.kratochvil@redhat.com> Signed-off-by: Oleg Nesterov <oleg@redhat.com> --- arch/powerpc/kernel/ptrace/ptrace-tm.c | 8 +++++++- arch/powerpc/kernel/ptrace/ptrace-view.c | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-)