[sanitizer] Fix ptrace interceptor for aarch64

[zatrazz]

Summary

This patch fixes the ptrace interceptor for aarch64. The PTRACE_GETREGSET
ptrace syscall with with invalid memory might zero the iovec::iov_base
field and then masking the subsequent check after the syscall (since it
will be 0 and it will not trigger an invalid access). The fix is to copy
the value on a local variable and use its value on the check.

The patch also adds more coverage on the Linux/ptrace.cc testcase by addding
check for PTRACE_GETREGSET for both general and floating registers (aarch64
definitions added only).

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors.inc

2464

Hm, we don't check that __sanitizer_iovec in data is accessible. Would you mind adding a READ_RANGE for that before copying it out?

test/asan/TestCases/Linux/ptrace.cc

6

This is impossible. You can not build compiler-rt to support all 3 of those at one time.
UNSUPPORTED: AddressSanitizer-x86_64-linux :: TestCases/Linux/ptrace.cc
UNSUPPORTED: AddressSanitizer-i386-linux :: TestCases/Linux/ptrace.cc

Not sure what's the best solution here. Maybe #ifdef-out the entire test on unsupported platforms?

18

Does it mean x86_64 run into the same problem soon?

[zatrazz]

lib/sanitizer_common/sanitizer_common_interceptors.inc

2464

Right, I will do it.

test/asan/TestCases/Linux/ptrace.cc

6

Right, I misunderstood the idea of REQUIRES. If there is no way to conditionally select the requirement (x86_64 or i386 or aarch64) the way I see to enable this test is to remove the 'requires', enable on all the architectures and then XFAIL on the ones that do not have the instrumented ptrace (since the CHECK: will be still being evaluated even if we #ifdef-out the tests). Suggestions?

18

The REQUIRES issue you noted above in fact showed this is not really necessary for the test itself. I will fix it and also another nits I found in this testcase.

[eugenis]

test/asan/TestCases/Linux/ptrace.cc

6

How about listing targets this test does not run on as UNSUPPORTED: ?

[zatrazz]

I updated the patch with additional checks for the iovec pointer itself and add
more tests for the test/asan/TestCases/Linux/ptrace.cc taking in consideration
all the current supported platforms. I tested on aarch64, arm*, powerpc64**,
x86_64, and i386. I do not have access to a mips64 machine, but I based the
changes on current code from ./test/sanitizer_common/TestCases/Linux/ptrace.cc.

For arm the test now fails due missing ptrace instrumentation for the architecture,
however I have a patch ready to fix it (so we might XFAIL arm-linux-gnueabi{hf}
for now). For powercp64 I had to use PTRACE_GETREGSET because
PTRACE_GETREGS was not supported on LE system I have access (although
the ptrace itself does not fail).

lib/sanitizer_common/sanitizer_common_interceptors.inc

 #else
 #define INIT_READDIR64
 #endif
 
 #if SANITIZER_INTERCEPT_PTRACE
 INTERCEPTOR(uptr, ptrace, int request, int pid, void *addr, void *data) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, ptrace, request, pid, addr, data);
+  __sanitizer_iovec local_iovec;
 
   if (data) {
     if (request == ptrace_setregs)
       COMMON_INTERCEPTOR_READ_RANGE(ctx, data, struct_user_regs_struct_sz);
     else if (request == ptrace_setfpregs)
       COMMON_INTERCEPTOR_READ_RANGE(ctx, data, struct_user_fpregs_struct_sz);
     else if (request == ptrace_setfpxregs)
       COMMON_INTERCEPTOR_READ_RANGE(ctx, data, struct_user_fpxregs_struct_sz);
     else if (request == ptrace_setsiginfo)
       COMMON_INTERCEPTOR_READ_RANGE(ctx, data, siginfo_t_sz);
-    else if (request == ptrace_setregset) {
-      __sanitizer_iovec *iov = (__sanitizer_iovec *)data;
-      COMMON_INTERCEPTOR_READ_RANGE(ctx, iov->iov_base, iov->iov_len);
+    // Some kernel might zero the iovec::iov_base in case of invalid
+    // write access.  In this case copy the invalid address for further
+    // inspection.
+    else if (request == ptrace_setregset || request == ptrace_getregset) {
+      local_iovec = *(__sanitizer_iovec *)data;
+      if (request == ptrace_setregset)
+        COMMON_INTERCEPTOR_READ_RANGE(ctx, local_iovec.iov_base,
+                                      local_iovec.iov_len);
     }
   }
 
   // FIXME: under ASan the call below may write to freed memory and corrupt
   // its metadata. See
   // https://code.google.com/p/address-sanitizer/issues/detail?id=321.
   uptr res = REAL(ptrace)(request, pid, addr, data);
 
   if (!res && data) {
     // Note that PEEK* requests assign different meaning to the return value.
     // This function does not handle them (nor does it need to).
     if (request == ptrace_getregs)
       COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, struct_user_regs_struct_sz);
     else if (request == ptrace_getfpregs)
       COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, struct_user_fpregs_struct_sz);
     else if (request == ptrace_getfpxregs)
       COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, struct_user_fpxregs_struct_sz);
     else if (request == ptrace_getsiginfo)
       COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, siginfo_t_sz);
     else if (request == ptrace_geteventmsg)
       COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, sizeof(unsigned long));
     else if (request == ptrace_getregset) {
-      __sanitizer_iovec *iov = (__sanitizer_iovec *)data;
-      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, iov->iov_base, iov->iov_len);
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, local_iovec.iov_base,
+                                     local_iovec.iov_len);
     }
   }
   return res;
 }
 
 #define INIT_PTRACE COMMON_INTERCEPT_FUNCTION(ptrace);
 #else
 #define INIT_PTRACE

test/asan/TestCases/Linux/ptrace.cc

 // FIXME: https://code.google.com/p/address-sanitizer/issues/detail?id=316
 // XFAIL: android
 //
 // RUN: %clangxx_asan -O0 %s -o %t && %run %t
 // RUN: %clangxx_asan -DPOSITIVE -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s
-// REQUIRES: x86_64-supported-target,i386-supported-target
+// REQUIRES: x86_64-supported-target,i386-supported-target,aarch64-supported-target
 
 #include <assert.h>
 #include <stdio.h>
 #include <sys/ptrace.h>
 #include <sys/types.h>
 #include <sys/user.h>
 #include <sys/wait.h>
 #include <unistd.h>
+#include <sys/uio.h> // for iovec
+#include <elf.h> // for NT_PRSTATUS
+#ifdef __aarch64__
+// GLIBC 2.20+ sys/user does not include asm/ptrace.h
+ #include <asm/ptrace.h>
+#endif
+
+#if defined(__i386__) || defined(__x86_64__)
+typedef user_regs_struct   regs_struct;
+typedef user_fpregs_struct fpregs_struct;
+#if defined(__i386__)
+#define REG_PC  eip
+#else
+#define REG_PC  rip
+#endif
+#define REG_FPS cwd
+
+#elif defined(__aarch64__)
+typedef struct user_pt_regs      regs_struct;
+typedef struct user_fpsimd_state fpregs_struct;
+#define REG_PC  pc
+#define REG_FPS fpsr
+#define ARCH_IOVEC_FOR_GETREGSET
+#endif
+
 
 int main(void) {
   pid_t pid;
   pid = fork();
   if (pid == 0) { // child
     ptrace(PTRACE_TRACEME, 0, NULL, NULL);
     execl("/bin/true", "true", NULL);
   } else {
     wait(NULL);
-    user_regs_struct regs;
+    user_pt_regs regs;
+    user_pt_regs* volatile pregs = &regs;
+#ifdef ARCH_IOVEC_FOR_GETREGSET
+    struct iovec regset_io;
+#endif
     int res;
-    user_regs_struct * volatile pregs = &regs;
+
 #ifdef POSITIVE
     ++pregs;
 #endif
+
+#ifdef ARCH_IOVEC_FOR_GETREGSET
+    regset_io.iov_base = pregs;
+    regset_io.iov_len = sizeof(regs_struct);
+    res = ptrace(PTRACE_GETREGSET, pid, (void*)NT_PRSTATUS,
+                 (void*)&regset_io);
+#else
     res = ptrace(PTRACE_GETREGS, pid, NULL, pregs);
+#endif
     // CHECK: AddressSanitizer: stack-buffer-overflow
     // CHECK: {{.*ptrace.cc:}}[[@LINE-2]]
     assert(!res);
-#ifdef __x86_64__
-    printf("%lx\n", (unsigned long)regs.rip);
-#else
-    printf("%lx\n", regs.eip);
-#endif
+    printf("%lx\n", (unsigned long)regs.REG_PC);
 
-    user_fpregs_struct fpregs;
+    fpregs_struct fpregs;
+#ifdef ARCH_IOVEC_FOR_GETREGSET
+    regset_io.iov_base = &fpregs;
+    regset_io.iov_len = sizeof(regs_struct);
+    res = ptrace(PTRACE_GETREGSET, pid, (void*)NT_FPREGSET,
+                 (void*)&regset_io);
+#else
     res = ptrace(PTRACE_GETFPREGS, pid, NULL, &fpregs);
+#endif
     assert(!res);
-    printf("%lx\n", (unsigned long)fpregs.cwd);
+    printf("%lx\n", (unsigned long)fpregs.REG_FPS);
 
-#ifndef __x86_64__
+#ifdef __i386__
     user_fpxregs_struct fpxregs;
     res = ptrace(PTRACE_GETFPXREGS, pid, NULL, &fpxregs);
     assert(!res);
     printf("%lx\n", (unsigned long)fpxregs.mxcsr);
 #endif
 
     ptrace(PTRACE_CONT, pid, NULL, NULL);
     wait(NULL);
   }
   return 0;
 }