Message ID | bda685cb-c5c4-ab27-47e0-1e95223275f9@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On Jun 06 2017, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: > Hi, > > this patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS > and PTRACE_SETFPREGS as these requests does not exist on s390 kernel. Since it is an API change it should probably get a NEWS entry. > +static void > +tracer_func (int pid) > +{ > + unsigned long last_break; > + ptrace_area parea; > + gregset_t regs; > + int status; > + > + while (1) > + { > + /* Wait for the tracee to be stopped or exited. */ > + wait (&status); Doesn't that need to use WUNTRACED? Andreas.
On Tue, Jun 06, 2017 at 12:44:04PM +0200, Andreas Schwab wrote: > On Jun 06 2017, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: [...] > > +static void > > +tracer_func (int pid) > > +{ > > + unsigned long last_break; > > + ptrace_area parea; > > + gregset_t regs; > > + int status; > > + > > + while (1) > > + { > > + /* Wait for the tracee to be stopped or exited. */ > > + wait (&status); > > Doesn't that need to use WUNTRACED? No, it doesn't: as the tracee has called PTRACE_TRACEME before raising SIGSTOP, there is going to be a ptrace signal delivery stop.
On 06/06/2017 12:44 PM, Andreas Schwab wrote: > On Jun 06 2017, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: > >> Hi, >> >> this patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS >> and PTRACE_SETFPREGS as these requests does not exist on s390 kernel. > > Since it is an API change it should probably get a NEWS entry. > I've added this NEWS entry: * The s390 specific ptrace requests are adjusted to the kernel ones. Request 12 is now used for PTRACE_SINGLEBLOCK instead of PTRACE_GETREGS. The requests PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS and PTRACE_SETFPREGS were removed as those are not supported by the s390 kernel. The requests PTRACE_SINGLEBLOCK, PTRACE_SECCOMP_GET_FILTER, PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA, PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE and PTRACE_TE_ABORT_RAND were added as those are supported by the s390 kernel.
On 06/06/2017 01:56 PM, Stefan Liebler wrote: > On 06/06/2017 12:44 PM, Andreas Schwab wrote: >> On Jun 06 2017, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: >> >>> Hi, >>> >>> this patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS >>> and PTRACE_SETFPREGS as these requests does not exist on s390 kernel. >> >> Since it is an API change it should probably get a NEWS entry. >> > I've added this NEWS entry: > * The s390 specific ptrace requests are adjusted to the kernel ones. > Request 12 is now used for PTRACE_SINGLEBLOCK instead of > PTRACE_GETREGS. The requests PTRACE_GETREGS, PTRACE_SETREGS, > PTRACE_GETFPREGS and PTRACE_SETFPREGS were removed as those are not > supported by the s390 kernel. The requests PTRACE_SINGLEBLOCK, > PTRACE_SECCOMP_GET_FILTER, PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA, > PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE and > PTRACE_TE_ABORT_RAND were added as those are supported by the s390 > kernel. > Any objection? Otherwise I'll commit the patch tomorrow. Bye Stefan
On Tue, Jun 06, 2017 at 12:17:33PM +0200, Stefan Liebler wrote: [...] > diff --git a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h > index 7caf101..88079fc 100644 > --- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h > +++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h > @@ -89,25 +89,9 @@ enum __ptrace_request > PTRACE_SINGLESTEP = 9, > #define PT_STEP PTRACE_SINGLESTEP > > - /* Get all general purpose registers used by a processes. > - This is not supported on all machines. */ > - PTRACE_GETREGS = 12, > -#define PT_GETREGS PTRACE_GETREGS > - > - /* Set all general purpose registers used by a processes. > - This is not supported on all machines. */ > - PTRACE_SETREGS = 13, > -#define PT_SETREGS PTRACE_SETREGS > - > - /* Get all floating point registers used by a processes. > - This is not supported on all machines. */ > - PTRACE_GETFPREGS = 14, > -#define PT_GETFPREGS PTRACE_GETFPREGS > - > - /* Set all floating point registers used by a processes. > - This is not supported on all machines. */ > - PTRACE_SETFPREGS = 15, > -#define PT_SETFPREGS PTRACE_SETFPREGS > + /* Execute process until next taken branch. */ > + PTRACE_SINGLEBLOCK = 12, > +#define PT_STEPBLOCK PTRACE_SINGLEBLOCK > > /* Attach to a process that is already running. */ > PTRACE_ATTACH = 16, > @@ -167,8 +151,26 @@ enum __ptrace_request > PTRACE_SETSIGMASK = 0x420b, > #define PTRACE_SETSIGMASK PTRACE_SETSIGMASK > > - PTRACE_SECCOMP_GET_FILTER = 0x420c > + PTRACE_SECCOMP_GET_FILTER = 0x420c, > #define PTRACE_SECCOMP_GET_FILTER PTRACE_SECCOMP_GET_FILTER > + > + PTRACE_PEEKUSR_AREA = 0x5000, > +#define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA > + > + PTRACE_POKEUSR_AREA = 0x5001, > +#define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA > + > + PTRACE_GET_LAST_BREAK = 0x5006, > +#define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK > + > + PTRACE_ENABLE_TE = 0x5009, > +#define PTRACE_ENABLE_TE PTRACE_ENABLE_TE > + > + PTRACE_DISABLE_TE = 0x5010, > +#define PTRACE_DISABLE_TE PTRACE_DISABLE_TE > + > + PTRACE_TE_ABORT_RAND = 0x5011 > +#define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND > }; The sys/ptrace.h part of the change looks fine. > diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c > new file mode 100644 > index 0000000..b384562 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c > @@ -0,0 +1,110 @@ > +/* Testing s390x PTRACE_SINGLEBLOCK ptrace request. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> > +#include <sys/wait.h> > +#include <sys/types.h> > +#include <support/xunistd.h> > + > +/* Ensure that we use the PTRACE_SINGLEBLOCK definition from glibc ptrace.h > + in tracer_func. We need the kernel ptrace.h for structs ptrace_area > + and gregset_t. */ > +#include <sys/ptrace.h> > +static const enum __ptrace_request req_singleblock = PTRACE_SINGLEBLOCK; > +#include <asm/ptrace.h> > + > +static void > +tracee_func (int pid) > +{ > + /* Dump the mapping information for manual inspection of the printed > + tracee addresses. */ > + char str[80]; > + sprintf (str, "cat /proc/%d/maps", pid); > + puts (str); > + system (str); > + fflush (stdout); > + > + ptrace (PTRACE_TRACEME); > + /* Stop tracee. Afterwards the tracer_func can operate. */ > + kill (pid, SIGSTOP); > + > + puts ("The PTRACE_SINGLEBLOCK of the tracer will stop after: " > + "brasl %r14,<puts@plt>!"); > +} > + > +static void > +tracer_func (int pid) > +{ > + unsigned long last_break; > + ptrace_area parea; > + gregset_t regs; > + int status; > + > + while (1) > + { > + /* Wait for the tracee to be stopped or exited. */ > + wait (&status); > + if (WIFEXITED (status)) > + break; > + > + /* Get information about tracee: gprs, last breaking address. */ > + parea.len = sizeof (regs); > + parea.process_addr = (unsigned long) ®s; > + parea.kernel_addr = 0; > + ptrace (PTRACE_PEEKUSR_AREA, pid, &parea); Note that you can verify whether PTRACE_PEEKUSR_AREA has returned the expected result by comparing registers with those returned by PTRACE_GETREGSET. The latter is implemented on s390 since linux 2.6.27 so its use in glibc is safe. > + ptrace (PTRACE_GET_LAST_BREAK, pid, NULL, &last_break); As these ptrace calls are expected to succeed, you might want to check their return code. > + > + printf ("child IA: %p last_break: %p\n", > + (void *) regs[1], (void *) last_break); > + > + /* Execute tracee until next taken branch. > + > + Note: > + Before the commit which introduced this testcase, > + <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h > + uses ptrace-request 12 for PTRACE_GETREGS, > + but <kernel>/include/uapi/linux/ptrace.h > + uses 12 for PTRACE_SINGLEBLOCK. > + > + The s390 kernel has no support for PTRACE_GETREGS! > + Thus glibc ptrace.h is adjusted to match kernel ptrace.h. > + > + This test ensures, that PTRACE_SINGLEBLOCK defined in glibc > + works as expected. If the kernel would interpret it as > + PTRACE_GETREGS, then the tracee will not make any progress > + and this testcase will time out. */ > + ptrace (req_singleblock, pid, NULL, NULL); Likewise.
commit c24a9b5367e91cf36a5a309eced8d0e13bc951e4 Author: Stefan Liebler <stli@linux.vnet.ibm.com> Date: Fri Jun 2 15:31:34 2017 +0200 S390: Sync ptrace.h with kernel. [BZ #21539] This patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS and PTRACE_SETFPREGS as these requests does not exist on s390 kernel. But the kernel has support for PTRACE_SINGLEBLOCK, PTRACE_SECCOMP_GET_FILTER, PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA, PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE and PTRACE_TE_ABORT_RAND. Thus those are defined now. The current kernel s390 specific ptrace.h file also defines PTRACE_PEEKTEXT_AREA, PTRACE_PEEKDATA_AREA, PTRACE_POKETEXT_AREA, PTRACE_POKEDATA_AREA, PTRACE_PEEK_SYSTEM_CALL, PTRACE_POKE_SYSTEM_CALL and PTRACE_PROT, but those requests are not supported. Thus those defines are skipped in glibc ptrace.h. There were old includes of ptrace.h in sysdeps/s390/fpu/fesetenv.c. The ptrace feature isn't used there anymore, thus I removed the includes. Before this patch, <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h uses ptrace-request 12 for PTRACE_GETREGS, but <kernel>/include/uapi/linux/ptrace.h uses 12 for PTRACE_SINGLEBLOCK. The s390 kernel has never had support for PTRACE_GETREGS! Thus glibc ptrace.h is adjusted to match kernel ptrace.h. The new s390 specific test ensures, that PTRACE_SINGLEBLOCK defined in glibc works as expected. If the kernel would interpret it as PTRACE_GETREGS, then the testcase will not make any progress and will time out. ChangeLog: [BZ #21539] * sysdeps/unix/sysv/linux/s390/sys/ptrace.h (PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS, PTRACE_SETFPREGS): Remove enum constant. (PT_GETREGS, PT_SETREGS, PT_GETFPREGS, T_SETFPREGS): Remove defines. (PTRACE_SINGLEBLOCK): New enum constant. (PT_STEPBLOCK): New define. (PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA, PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE, PTRACE_TE_ABORT_RAND): New enum constant and define. * sysdeps/s390/fpu/fesetenv.c: Remove ptrace.h includes. * sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c: New file. * sysdeps/unix/sysv/linux/s390/Makefile: Add test. diff --git a/sysdeps/s390/fpu/fesetenv.c b/sysdeps/s390/fpu/fesetenv.c index 4c9bcf0..0f64a3f 100644 --- a/sysdeps/s390/fpu/fesetenv.c +++ b/sysdeps/s390/fpu/fesetenv.c @@ -20,8 +20,6 @@ #include <fenv_libc.h> #include <fpu_control.h> #include <stddef.h> -#include <asm/ptrace.h> -#include <sys/ptrace.h> #include <unistd.h> int diff --git a/sysdeps/unix/sysv/linux/s390/Makefile b/sysdeps/unix/sysv/linux/s390/Makefile index 3867c33..f30a6bb 100644 --- a/sysdeps/unix/sysv/linux/s390/Makefile +++ b/sysdeps/unix/sysv/linux/s390/Makefile @@ -29,3 +29,7 @@ CFLAGS-elision-trylock.c = $(elision-CFLAGS) CFLAGS-elision-unlock.c = $(elision-CFLAGS) endif endif + +ifeq ($(subdir),misc) +tests += tst-ptrace-singleblock +endif diff --git a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h index 7caf101..88079fc 100644 --- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h +++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h @@ -89,25 +89,9 @@ enum __ptrace_request PTRACE_SINGLESTEP = 9, #define PT_STEP PTRACE_SINGLESTEP - /* Get all general purpose registers used by a processes. - This is not supported on all machines. */ - PTRACE_GETREGS = 12, -#define PT_GETREGS PTRACE_GETREGS - - /* Set all general purpose registers used by a processes. - This is not supported on all machines. */ - PTRACE_SETREGS = 13, -#define PT_SETREGS PTRACE_SETREGS - - /* Get all floating point registers used by a processes. - This is not supported on all machines. */ - PTRACE_GETFPREGS = 14, -#define PT_GETFPREGS PTRACE_GETFPREGS - - /* Set all floating point registers used by a processes. - This is not supported on all machines. */ - PTRACE_SETFPREGS = 15, -#define PT_SETFPREGS PTRACE_SETFPREGS + /* Execute process until next taken branch. */ + PTRACE_SINGLEBLOCK = 12, +#define PT_STEPBLOCK PTRACE_SINGLEBLOCK /* Attach to a process that is already running. */ PTRACE_ATTACH = 16, @@ -167,8 +151,26 @@ enum __ptrace_request PTRACE_SETSIGMASK = 0x420b, #define PTRACE_SETSIGMASK PTRACE_SETSIGMASK - PTRACE_SECCOMP_GET_FILTER = 0x420c + PTRACE_SECCOMP_GET_FILTER = 0x420c, #define PTRACE_SECCOMP_GET_FILTER PTRACE_SECCOMP_GET_FILTER + + PTRACE_PEEKUSR_AREA = 0x5000, +#define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA + + PTRACE_POKEUSR_AREA = 0x5001, +#define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA + + PTRACE_GET_LAST_BREAK = 0x5006, +#define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK + + PTRACE_ENABLE_TE = 0x5009, +#define PTRACE_ENABLE_TE PTRACE_ENABLE_TE + + PTRACE_DISABLE_TE = 0x5010, +#define PTRACE_DISABLE_TE PTRACE_DISABLE_TE + + PTRACE_TE_ABORT_RAND = 0x5011 +#define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND }; diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c new file mode 100644 index 0000000..b384562 --- /dev/null +++ b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c @@ -0,0 +1,110 @@ +/* Testing s390x PTRACE_SINGLEBLOCK ptrace request. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <sys/wait.h> +#include <sys/types.h> +#include <support/xunistd.h> + +/* Ensure that we use the PTRACE_SINGLEBLOCK definition from glibc ptrace.h + in tracer_func. We need the kernel ptrace.h for structs ptrace_area + and gregset_t. */ +#include <sys/ptrace.h> +static const enum __ptrace_request req_singleblock = PTRACE_SINGLEBLOCK; +#include <asm/ptrace.h> + +static void +tracee_func (int pid) +{ + /* Dump the mapping information for manual inspection of the printed + tracee addresses. */ + char str[80]; + sprintf (str, "cat /proc/%d/maps", pid); + puts (str); + system (str); + fflush (stdout); + + ptrace (PTRACE_TRACEME); + /* Stop tracee. Afterwards the tracer_func can operate. */ + kill (pid, SIGSTOP); + + puts ("The PTRACE_SINGLEBLOCK of the tracer will stop after: " + "brasl %r14,<puts@plt>!"); +} + +static void +tracer_func (int pid) +{ + unsigned long last_break; + ptrace_area parea; + gregset_t regs; + int status; + + while (1) + { + /* Wait for the tracee to be stopped or exited. */ + wait (&status); + if (WIFEXITED (status)) + break; + + /* Get information about tracee: gprs, last breaking address. */ + parea.len = sizeof (regs); + parea.process_addr = (unsigned long) ®s; + parea.kernel_addr = 0; + ptrace (PTRACE_PEEKUSR_AREA, pid, &parea); + ptrace (PTRACE_GET_LAST_BREAK, pid, NULL, &last_break); + + printf ("child IA: %p last_break: %p\n", + (void *) regs[1], (void *) last_break); + + /* Execute tracee until next taken branch. + + Note: + Before the commit which introduced this testcase, + <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h + uses ptrace-request 12 for PTRACE_GETREGS, + but <kernel>/include/uapi/linux/ptrace.h + uses 12 for PTRACE_SINGLEBLOCK. + + The s390 kernel has no support for PTRACE_GETREGS! + Thus glibc ptrace.h is adjusted to match kernel ptrace.h. + + This test ensures, that PTRACE_SINGLEBLOCK defined in glibc + works as expected. If the kernel would interpret it as + PTRACE_GETREGS, then the tracee will not make any progress + and this testcase will time out. */ + ptrace (req_singleblock, pid, NULL, NULL); + } +} + +static int +do_test (void) +{ + int pid; + pid = xfork (); + if (pid) + tracer_func (pid); + else + tracee_func (getpid ()); + + return EXIT_SUCCESS; +} + +#include <support/test-driver.c>