Re: [PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too
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, &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
Re: PPC upstream kernel ignored DABR bug
On Wed, 28 Nov 2007 13:28:48 +0100, Arnd Bergmann wrote: > On Wednesday 28 November 2007, Jan Kratochvil wrote: > > Please be aware DABR works fine if the same code runs just 1 (always) or > > 2 (sometimes) threads. It starts failing with too many threads running: > > > > $ ./dabr-lost > > TID 32725: DABR 0x1001279f NIP 0xfecf41c > > TID 32726: DABR 0x1001279f NIP 0xfecf41c > > TID 32725: hitting the variable > > variable found = -1, caught TID = 32725 > > TID 32726: hitting the variable > > variable found = -1, caught TID = 32726 > > The kernel bug did not get reproduced - increase THREADS. > > > > As I did not find any code in that kernel touching DABRX its value should > > not > > be dependent on the number of threads running. > > > > Right, this is a different problem from the one reported by Uli. > From what I can tell, your problem is that you set the DABR only > in one thread, so the other threads don't see it. DABR is saved > in the thread_struct, so setting it in one thread doesn't have > an impact on any other thread. It even prints out above: TID 32725: DABR 0x1001279f NIP 0xfecf41c TID 32726: DABR 0x1001279f NIP 0xfecf41c that it wrote DABR in both the threads and it has also successfully read it back from each thread specifically (according to its thread-specific TID). for (threadi = 0; threadi < THREADS; threadi++) { pid_t tid = thread[threadi]; setup (tid); ... } static void setup (pid_t tid) { ... l = ptrace (PTRACE_SET_DEBUGREG, tid, NULL, (void *) dabr); ... } Also if I would not set DABR specifically for each thread it would not work in 90% of cases for `THREADS == 2'. And it would not work for `THREADS == 4' if they are busylooping (therefore not in a syscall). TID 596: DABR 0x100127a7 NIP 0x1dbc TID 597: DABR 0x100127a7 NIP 0x1db0 TID 598: DABR 0x100127a7 NIP 0x1dac TID 599: DABR 0x100127a7 NIP 0x1dbc TID 596: hitting the variable variable found = -1, caught TID = 596 TID 599: hitting the variable variable found = -1, caught TID = 599 TID 597: hitting the variable variable found = -1, caught TID = 597 TID 598: hitting the variable variable found = -1, caught TID = 598 The kernel bug got workarounded by WORKAROUND_SET_DABR_IN_SYSCALL. (I found out now WORKAROUND_SET_DABR_IN_SYSCALL only reduces the probability of the failure, it is not a 100% workaround of the problem in the testcase.) There is some tricky kernel code around it but I did not try to debug it: struct task_struct *__switch_to(struct task_struct *prev, struct task_struct *new) { ... if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr)) { set_dabr(new->thread.dabr); __get_cpu_var(current_dabr) = new->thread.dabr; } ... } Regards, Jan ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: PPC upstream kernel ignored DABR bug
On Tue, 27 Nov 2007 23:35:36 +0100, Arnd Bergmann wrote: > On Monday 26 November 2007, Jan Kratochvil wrote: > > Hi, > > > > this testcase: > > http://people.redhat.com/jkratoch/dabr-lost.c > > > > reproduces a PPC DABR kernel bug. The variable `variable' should not get > > modified as the thread modifying it should be caught by its DABR: > > > > $ ./dabr-lost > > TID 30914: DABR 0x10012a77 NIP 0x80f6ebb318 > > TID 30915: DABR 0x10012a77 NIP 0x80f6ebb318 > > TID 30916: DABR 0x10012a77 NIP 0x80f6ebb318 > > TID 30914: hitting the variable > > TID 30915: hitting the variable > > TID 30916: hitting the variable > > variable found = 30916, caught TID = 30914 > > TID 30916: DABR 0x10012a77 > > Variable got modified by a thread which has DABR still set! > > > > This sounds like a bug recently reported by Uli Weigand. BenH > said he'd take a look, but it probably fell under the table. > The problem found by Uli is that on certain processors (Cell/B.E. > in his case), the DABRX register needs to be set in order for > the DABR to take effect. Please be aware DABR works fine if the same code runs just 1 (always) or 2 (sometimes) threads. It starts failing with too many threads running: $ ./dabr-lost TID 32725: DABR 0x1001279f NIP 0xfecf41c TID 32726: DABR 0x1001279f NIP 0xfecf41c TID 32725: hitting the variable variable found = -1, caught TID = 32725 TID 32726: hitting the variable variable found = -1, caught TID = 32726 The kernel bug did not get reproduced - increase THREADS. As I did not find any code in that kernel touching DABRX its value should not be dependent on the number of threads running. Regards, Lace ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
PPC upstream kernel ignored DABR bug
Hi, this testcase: http://people.redhat.com/jkratoch/dabr-lost.c reproduces a PPC DABR kernel bug. The variable `variable' should not get modified as the thread modifying it should be caught by its DABR: $ ./dabr-lost TID 30914: DABR 0x10012a77 NIP 0x80f6ebb318 TID 30915: DABR 0x10012a77 NIP 0x80f6ebb318 TID 30916: DABR 0x10012a77 NIP 0x80f6ebb318 TID 30914: hitting the variable TID 30915: hitting the variable TID 30916: hitting the variable variable found = 30916, caught TID = 30914 TID 30916: DABR 0x10012a77 Variable got modified by a thread which has DABR still set! At the `variable found =' line the parent ptracer found the TID thread 30916 wrote the value into the variable - despite it had DABR alrady set before. As the behavior is dependent on the current weather I expect the scheduling matters there. It is important the target thread is in the `nanosleep' syscall. If you define WORKAROUND_SET_DABR_IN_SYSCALL in the testcase it busyloops in the userland and the bug gets no longer reproduced. I got it reproduced on a utrace-patched kernel on dual-CPU Power5 and Roland McGrath reported it reproduced on the vanilla upstream kernel on a Mac G5. Regards, Jan Kratochvil ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev