[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

5–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?

17

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

5–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?

17

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

5–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) {
+      __sanitizer_iovec *iovec = (__sanitizer_iovec*)data;
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, iovec, sizeof(*iovec));
+      local_iovec = *iovec;
+      if (request == ptrace_setregset)
+        COMMON_INTERCEPTOR_READ_RANGE(ctx, iovec->iov_base, 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);
+      __sanitizer_iovec *iovec = (__sanitizer_iovec*)data;
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, iovec, sizeof(*iovec));
+      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
 
 #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
+
+#if defined(__i386__) || defined(__x86_64__)
+typedef user_regs_struct   regs_struct;
+typedef user_fpregs_struct fpregs_struct;
+#if defined(__i386__)
+#define REG_IP  eip
+#else
+#define REG_IP  rip
+#endif
+#define PRINT_REG_PC(__regs)    printf ("%lx\n", (unsigned long) (__regs.REG_IP))
+#define PRINT_REG_FP(__fpregs)  printf ("%lx\n", (unsigned long) (__fpregs.cwd))
+#define __PTRACE_FPREQUEST PTRACE_GETFPREGS
+
+#elif defined(__aarch64__)
+typedef struct user_pt_regs      regs_struct;
+typedef struct user_fpsimd_state fpregs_struct;
+#define PRINT_REG_PC(__regs)    printf ("%x\n", (unsigned) (__regs.pc))
+#define PRINT_REG_FP(__fpregs)  printf ("%x\n", (unsigned) (__fpregs.fpsr))
+#define ARCH_IOVEC_FOR_GETREGSET
+
+#elif defined(__powerpc64__)
+typedef struct pt_regs regs_struct;
+typedef elf_fpregset_t fpregs_struct;
+#define PRINT_REG_PC(__regs)    printf ("%lx\n", (unsigned long) (__regs.nip))
+#define PRINT_REG_FP(__fpregs)  printf ("%lx\n", (elf_greg_t)fpregs[32])
+#define ARCH_IOVEC_FOR_GETREGSET
+
+#elif defined(__mips64)
+typedef struct pt_regs regs_struct;
+typedef elf_fpregset_t fpregs_struct;
+#define PRINT_REG_PC(__regs)    printf ("%lx\n", (unsigned long) (__regs.cp0_epc))
+#define PRINT_REG_FP(__fpregs)  printf ("%lx\n", (elf_greg_t) (__fpregs[32]))
+#define __PTRACE_FPREQUEST PTRACE_GETFPREGS
+
+#elif defined(__arm__)
+# include <asm/ptrace.h>
+# include <sys/procfs.h>
+typedef struct pt_regs regs_struct;
+typedef char fpregs_struct[ARM_VFPREGS_SIZE];
+#define PRINT_REG_PC(__regs)    printf ("%x\n", (unsigned) (__regs.ARM_pc))
+#define PRINT_REG_FP(__fpregs)  printf ("%x\n", (unsigned) (__fpregs + 32 * 8))
+#define __PTRACE_FPREQUEST PTRACE_GETVFPREGS
+#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;
+    regs_struct regs;
+    regs_struct* 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
-    res = ptrace(PTRACE_GETREGS, pid, NULL, pregs);
+
+#ifdef ARCH_IOVEC_FOR_GETREGSET
+# define __PTRACE_REQUEST  PTRACE_GETREGSET
+# define __PTRACE_ARGS     (void*)NT_PRSTATUS, (void*)&regset_io
+    regset_io.iov_base = pregs;
+    regset_io.iov_len = sizeof(regs_struct);
+#else
+# define __PTRACE_REQUEST  PTRACE_GETREGS
+# define __PTRACE_ARGS     NULL, pregs
+#endif
+    res = ptrace((enum __ptrace_request)__PTRACE_REQUEST, pid, __PTRACE_ARGS);
     // CHECK: AddressSanitizer: stack-buffer-overflow
     // CHECK: {{.*ptrace.cc:}}[[@LINE-2]]
     assert(!res);
-#ifdef __x86_64__
-    printf("%lx\n", (unsigned long)regs.rip);
+    PRINT_REG_PC(regs);
+
+    fpregs_struct fpregs;
+#ifdef ARCH_IOVEC_FOR_GETREGSET
+# define __PTRACE_FPREQUEST  PTRACE_GETREGSET
+# define __PTRACE_FPARGS     (void*)NT_PRSTATUS, (void*)&regset_io
+    regset_io.iov_base = &fpregs;
+    regset_io.iov_len = sizeof(fpregs_struct);
+    res = ptrace((enum __ptrace_request)PTRACE_GETREGSET, pid, (void*)NT_FPREGSET,
+                 (void*)&regset_io);
 #else
-    printf("%lx\n", regs.eip);
+# define __PTRACE_FPARGS     NULL, &fpregs
 #endif
-
-    user_fpregs_struct fpregs;
-    res = ptrace(PTRACE_GETFPREGS, pid, NULL, &fpregs);
+    res = ptrace((enum __ptrace_request)__PTRACE_FPREQUEST, pid, __PTRACE_FPARGS);
     assert(!res);
-    printf("%lx\n", (unsigned long)fpregs.cwd);
+    PRINT_REG_FP(fpregs);
 
-#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;
 }