Patchwork S390: Sync ptrace.h with kernel. [BZ #21539]

login
register
mail settings
Submitter Stefan Liebler
Date July 7, 2017, 10:22 a.m.
Message ID <bb27ad09-7f36-0fef-4669-94466a169bb1@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/21464/
State Superseded
Headers show

Comments

Stefan Liebler - July 7, 2017, 10:22 a.m.
On 07/04/2017 05:37 PM, Stefan Liebler wrote:
> On 07/04/2017 11:41 AM, Florian Weimer wrote:
>> On 07/04/2017 10:22 AM, Stefan Liebler wrote:
>>> +      /* Ptrace request 12 is done with zero data argument:
>>> +     -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
>>> +     header asm/ptrace.h defines this macro), the ptrace call is not 
>>> allowed
>>> +     to fail and has to continue the tracee until next taken branch.
>>
>> I think this is still bogus.  We can compile with newer kernel headers
>> than the host kernel, and this will cause the test to fail.
>>
>> Thanks,
>> Florian
>>
> 
> Okay.
> So I can check the return value of the second ptrace (req_singleblock, 
> pid, NULL, NULL) call at runtime to determine the kernel-support:
> 
>        errno = 0;
>        ret = ptrace (req_singleblock, pid, NULL, NULL);
>        if (ret == 0)
>      {
>        /* The kernel has support for PTRACE_SINGLEBLOCK ptrace request. */
>        TEST_VERIFY_EXIT (errno == 0);
>      }
>        else
>      {
>        /* The kernel (< 3.15) has no support for PTRACE_SINGLEBLOCK ptrace
>           request. */
>        TEST_VERIFY_EXIT (errno == EIO);
>        TEST_VERIFY_EXIT (ret == -1);
> 
>        /* Just continue tracee until it exits normally.  */
>        TEST_VERIFY_EXIT (ptrace (PTRACE_CONT, pid, NULL, NULL) == 0);
>      }
> 
> 
> 
> Then the test runs on kernels with / without support for 
> PTRACE_SINGLEBLOCK. The first ptrace call ensures that request 12 is not 
> interpreted as PTRACE_GETREGS.
> 
> Bye.
> Stefan
> 

Here is the complete patch with the change mentioned above.
Is this approach okay?

Bye.
Stefan
Florian Weimer - July 7, 2017, 10:45 a.m.
On 07/07/2017 12:22 PM, Stefan Liebler wrote:
> +      /* Ptrace request 12 is done with zero data argument:
> +	 -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
> +	 header asm/ptrace.h defines this macro), the ptrace call is not allowed
> +	 to fail and has to continue the tracee until next taken branch.
> +
> +	 -If the kernel (<3.15) has no support for PTRACE_SINGLEBLOCK, the
> +	 ptrace call has to fail with EIO. Then I continue the tracee with
> +	 PTRACE_CONT.
> +
> +	 -If the request 12 is interpreted as PTRACE_GETREGS, it will fail too.
> +	 It fails with EFAULT on intel / power as data argument is NULL.
> +	 According to the man-page: "Unfortunately, under Linux, different
> +	 variations of this fault will return EIO or EFAULT more or less
> +	 arbitrarily".
> +	 But if request 12 is interpreted as PTRACE_GETREGS, the first ptrace
> +	 call will touch the buffer which is detected by this test.  */

I think the comment is still a bit off.  I think it is only necessary to
retain the second two lines, the other things is already implied by the
short comments in the code below.

(I have not tested whether this actually works.  I assume you have
checked a couple of userspace/kernel permutations.)

Thanks,
Florian

Patch

commit 5319bad7263aa1dc69da37bd4873876189ee5e97
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Fri Jul 7 10:29:56 2017 +0200

    S390: Fix tst-ptrace-singleblock if kernel does not support PTRACE_SINGLEBLOCK.
    
    The request PTRACE_SINGLEBLOCK was introduced  in Linux 3.15.  Thus the ptrace call
    will fail on older kernels.
    Thus the test is now testing PTRACE_SINGLEBLOCK with data argument pointing to a
    buffer on stack which is assumed to fail.  If the request would be interpreted as
    PTRACE_GETREGS, then the ptrace call will not fail and the regs are written to buf.
    
    If we run with a kernel with support for PTRACE_SINGLEBLOCK a ptrace call with
    data=NULL, returns zero with no error.  If we run with a kernel without support for
    PTRACE_SINGLEBLOCK a ptrace call with data=NULL reports an error.
    In the latter case, the test is just continuing with PTRACE_CONT.
    
    ChangeLog:
    
    	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
    	Support running on kernels without PTRACE_SINGLEBLOCK.

diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
index 95a2f55..6e4ba00 100644
--- a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
+++ b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
@@ -26,6 +26,8 @@ 
 #include <elf.h>
 #include <support/xunistd.h>
 #include <support/check.h>
+#include <string.h>
+#include <errno.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
@@ -63,6 +65,10 @@  tracer_func (int pid)
   gregset_t regs2;
 
   int status;
+  int ret;
+#define MAX_CHARS_IN_BUF 4096
+  char buf[MAX_CHARS_IN_BUF + 1];
+  size_t buf_count;
 
   while (1)
     {
@@ -104,11 +110,69 @@  tracer_func (int pid)
 	 The s390 kernel has no support for PTRACE_GETREGS!
 	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
 
+	 The glibc sys/ptrace.h header contains the identifier
+	 PTRACE_SINGLEBLOCK in enum __ptrace_request.  In contrast, the kernel
+	 asm/ptrace.h header defines PTRACE_SINGLEBLOCK.
+
 	 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.  */
-      TEST_VERIFY_EXIT (ptrace (req_singleblock, pid, NULL, NULL) == 0);
+	 and this testcase will time out or the ptrace call will fail with
+	 different errors.  */
+
+      /* Ptrace request 12 is first done with data argument pointing to
+	 a buffer:
+	 -If request 12 is interpreted as PTRACE_GETREGS, it will store the regs
+	 to buffer without an error.
+
+	 -If request 12 is interpreted as PTRACE_SINGLEBLOCK, it will fail
+	 as data argument is used as signal-number and the address of
+	 buf is no valid signal.
+
+	 -If request 12 is not implemented, it will also fail.
+
+	 Here the test expects that the buffer is untouched and an error is
+	 returned.  */
+      memset (buf, 'a', MAX_CHARS_IN_BUF);
+      ret = ptrace (req_singleblock, pid, NULL, buf);
+      buf [MAX_CHARS_IN_BUF] = '\0';
+      buf_count = strspn (buf, "a");
+      TEST_VERIFY_EXIT (buf_count == MAX_CHARS_IN_BUF);
+      TEST_VERIFY_EXIT (ret == -1);
+
+      /* Ptrace request 12 is done with zero data argument:
+	 -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
+	 header asm/ptrace.h defines this macro), the ptrace call is not allowed
+	 to fail and has to continue the tracee until next taken branch.
+
+	 -If the kernel (<3.15) has no support for PTRACE_SINGLEBLOCK, the
+	 ptrace call has to fail with EIO. Then I continue the tracee with
+	 PTRACE_CONT.
+
+	 -If the request 12 is interpreted as PTRACE_GETREGS, it will fail too.
+	 It fails with EFAULT on intel / power as data argument is NULL.
+	 According to the man-page: "Unfortunately, under Linux, different
+	 variations of this fault will return EIO or EFAULT more or less
+	 arbitrarily".
+	 But if request 12 is interpreted as PTRACE_GETREGS, the first ptrace
+	 call will touch the buffer which is detected by this test.  */
+      errno = 0;
+      ret = ptrace (req_singleblock, pid, NULL, NULL);
+      if (ret == 0)
+	{
+	  /* The kernel has support for PTRACE_SINGLEBLOCK ptrace request. */
+	  TEST_VERIFY_EXIT (errno == 0);
+	}
+      else
+	{
+	  /* The kernel (< 3.15) has no support for PTRACE_SINGLEBLOCK ptrace
+	     request. */
+	  TEST_VERIFY_EXIT (errno == EIO);
+	  TEST_VERIFY_EXIT (ret == -1);
+
+	  /* Just continue tracee until it exits normally.  */
+	  TEST_VERIFY_EXIT (ptrace (PTRACE_CONT, pid, NULL, NULL) == 0);
+	}
     }
 }