Skip to content

[sanitizer_common] Handle ptrace on Linux/sparc64 #109310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

rorth
Copy link
Collaborator

@rorth rorth commented Sep 19, 2024

When ASan testing is enabled on SPARC as per PR #107405, the

  AddressSanitizer-sparc-linux :: TestCases/Linux/ptrace.cpp

FAILs on Linux/sparc64. This happens because the ptrace interceptor has no support for that target at all.

This patch adds the missing parts and accounts for a couple of issues specific to this target:

  • In some cases, SPARC just needs to be included in the list of supported targets.
  • Besides, the types used by the PTRACE_GETREGS and PTRACE_GETFPREGS requests need to be filled in.
  • ptrace has a weird quirk on this target: for a couple of requests, the meaning of the data and addr args is reversed. All of the Linux/ptrace.cpp test and the interceptor, pre-syscall and post-syscall hooks need to account for that swap in their checks.

Tested on sparc64-unknown-linux-gnu and x86_64-pc-linux-gnu.

When ASan testing is enabled on SPARC as per PR llvm#107405, the
```
  AddressSanitizer-sparc-linux :: TestCases/Linux/ptrace.cpp
```
`FAIL`s on Linux/sparc64.  This happens because the `ptrace` interceptor
has no support for that target at all.

This patch adds the missing parts and accounts for a couple of issues
specific to this target:
- In some cases, SPARC just needs to be included in the list of supported
  targets.
- Besides, the Linux/sparc64 types used by the `PTRACE_GETREGS` and
  `PTRACE_GETFPREGS` need to be filled in.
- `ptrace` has a weird quirk on this target: for a couple of requests, the
  meaning of the `data` and `addr` args is reversed.  All of the
  `Linux/ptrace.cpp` test and the interceptor, pre-syscall and post-syscall
  hooks need to account for that swap in their checks.

Tested on `sparc64-unknown-linux-gnu` and `x86_64-pc-linux-gnu`.
@llvmbot
Copy link
Member

llvmbot commented Sep 19, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Rainer Orth (rorth)

Changes

When ASan testing is enabled on SPARC as per PR #107405, the

  AddressSanitizer-sparc-linux :: TestCases/Linux/ptrace.cpp

FAILs on Linux/sparc64. This happens because the ptrace interceptor has no support for that target at all.

This patch adds the missing parts and accounts for a couple of issues specific to this target:

  • In some cases, SPARC just needs to be included in the list of supported targets.
  • Besides, the types used by the PTRACE_GETREGS and PTRACE_GETFPREGS requests need to be filled in.
  • ptrace has a weird quirk on this target: for a couple of requests, the meaning of the data and addr args is reversed. All of the Linux/ptrace.cpp test and the interceptor, pre-syscall and post-syscall hooks need to account for that swap in their checks.

Tested on sparc64-unknown-linux-gnu and x86_64-pc-linux-gnu.


Full diff: https://github.com/llvm/llvm-project/pull/109310.diff

6 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc (+32-18)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc (+31-15)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h (+3-2)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp (+19-14)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h (+5-4)
  • (modified) compiler-rt/test/asan/TestCases/Linux/ptrace.cpp (+21-2)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
index e09a4a8ae25fd8..99913e150d2937 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -3430,23 +3430,34 @@ INTERCEPTOR(uptr, ptrace, int request, int pid, void *addr, void *data) {
   COMMON_INTERCEPTOR_ENTER(ctx, ptrace, request, pid, addr, data);
   __sanitizer_iovec local_iovec;
 
-  if (data) {
+  void *data_arg = data;
+#  if defined(__sparc__)
+  // The meanings of addr and data are reversed for a few requests on
+  // Linux/sparc64.
+  if (request == ptrace_getregs || request == ptrace_getfpregs ||
+      request == ptrace_setregs || request == ptrace_setfpregs)
+    data_arg = addr;
+#  endif
+  if (data_arg) {
     if (request == ptrace_setregs) {
-      COMMON_INTERCEPTOR_READ_RANGE(ctx, data, struct_user_regs_struct_sz);
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, data_arg, struct_user_regs_struct_sz);
     } else if (request == ptrace_setfpregs) {
-      COMMON_INTERCEPTOR_READ_RANGE(ctx, data, struct_user_fpregs_struct_sz);
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, data_arg,
+                                    struct_user_fpregs_struct_sz);
     } else if (request == ptrace_setfpxregs) {
-      COMMON_INTERCEPTOR_READ_RANGE(ctx, data, struct_user_fpxregs_struct_sz);
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, data_arg,
+                                    struct_user_fpxregs_struct_sz);
     } else if (request == ptrace_setvfpregs) {
-      COMMON_INTERCEPTOR_READ_RANGE(ctx, data, struct_user_vfpregs_struct_sz);
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, data_arg,
+                                    struct_user_vfpregs_struct_sz);
     } else if (request == ptrace_setsiginfo) {
-      COMMON_INTERCEPTOR_READ_RANGE(ctx, data, siginfo_t_sz);
+      COMMON_INTERCEPTOR_READ_RANGE(ctx, data_arg, siginfo_t_sz);
 
-    // Some kernel might zero the iovec::iov_base in case of invalid
-    // write access.  In this case copy the invalid address for further
-    // inspection.
+      // 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;
+      __sanitizer_iovec *iovec = (__sanitizer_iovec *)data_arg;
       COMMON_INTERCEPTOR_READ_RANGE(ctx, iovec, sizeof(*iovec));
       local_iovec = *iovec;
       if (request == ptrace_setregset)
@@ -3459,23 +3470,26 @@ INTERCEPTOR(uptr, ptrace, int request, int pid, void *addr, void *data) {
   // https://github.com/google/sanitizers/issues/321.
   uptr res = REAL(ptrace)(request, pid, addr, data);
 
-  if (!res && data) {
+  if (!res && data_arg) {
     // 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);
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data_arg, struct_user_regs_struct_sz);
     } else if (request == ptrace_getfpregs) {
-      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, struct_user_fpregs_struct_sz);
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data_arg,
+                                     struct_user_fpregs_struct_sz);
     } else if (request == ptrace_getfpxregs) {
-      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, struct_user_fpxregs_struct_sz);
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data_arg,
+                                     struct_user_fpxregs_struct_sz);
     } else if (request == ptrace_getvfpregs) {
-      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, struct_user_vfpregs_struct_sz);
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data_arg,
+                                     struct_user_vfpregs_struct_sz);
     } else if (request == ptrace_getsiginfo) {
-      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, siginfo_t_sz);
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data_arg, siginfo_t_sz);
     } else if (request == ptrace_geteventmsg) {
-      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, sizeof(unsigned long));
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data_arg, sizeof(unsigned long));
     } else if (request == ptrace_getregset) {
-      __sanitizer_iovec *iovec = (__sanitizer_iovec*)data;
+      __sanitizer_iovec *iovec = (__sanitizer_iovec *)data_arg;
       COMMON_INTERCEPTOR_WRITE_RANGE(ctx, iovec, sizeof(*iovec));
       COMMON_INTERCEPTOR_WRITE_RANGE(ctx, local_iovec.iov_base,
                                      local_iovec.iov_len);
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc
index 14615f9668dea6..c8be8a230bd964 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc
@@ -2530,18 +2530,26 @@ PRE_SYSCALL(ptrace)(long request, long pid, long addr, long data) {
 #  if !SANITIZER_ANDROID &&                                                   \
       (defined(__i386) || defined(__x86_64) || defined(__mips64) ||           \
        defined(__powerpc64__) || defined(__aarch64__) || defined(__s390__) || \
-       defined(__loongarch__) || SANITIZER_RISCV64)
-  if (data) {
+       defined(__loongarch__) || SANITIZER_RISCV64 || defined(__sparc__))
+  long data_arg = data;
+#    if defined(__sparc__)
+  // The meanings of addr and data are reversed for a few requests on
+  // Linux/sparc64.
+  if (request == ptrace_getregs || request == ptrace_getfpregs ||
+      request == ptrace_setregs || request == ptrace_setfpregs)
+    data_arg = addr;
+#    endif
+  if (data_arg) {
     if (request == ptrace_setregs) {
-      PRE_READ((void *)data, struct_user_regs_struct_sz);
+      PRE_READ((void *)data_arg, struct_user_regs_struct_sz);
     } else if (request == ptrace_setfpregs) {
-      PRE_READ((void *)data, struct_user_fpregs_struct_sz);
+      PRE_READ((void *)data_arg, struct_user_fpregs_struct_sz);
     } else if (request == ptrace_setfpxregs) {
-      PRE_READ((void *)data, struct_user_fpxregs_struct_sz);
+      PRE_READ((void *)data_arg, struct_user_fpxregs_struct_sz);
     } else if (request == ptrace_setsiginfo) {
-      PRE_READ((void *)data, siginfo_t_sz);
+      PRE_READ((void *)data_arg, siginfo_t_sz);
     } else if (request == ptrace_setregset) {
-      __sanitizer_iovec *iov = (__sanitizer_iovec *)data;
+      __sanitizer_iovec *iov = (__sanitizer_iovec *)data_arg;
       PRE_READ(iov->iov_base, iov->iov_len);
     }
   }
@@ -2552,25 +2560,33 @@ POST_SYSCALL(ptrace)(long res, long request, long pid, long addr, long data) {
 #  if !SANITIZER_ANDROID &&                                                   \
       (defined(__i386) || defined(__x86_64) || defined(__mips64) ||           \
        defined(__powerpc64__) || defined(__aarch64__) || defined(__s390__) || \
-       defined(__loongarch__) || SANITIZER_RISCV64)
-  if (res >= 0 && data) {
+       defined(__loongarch__) || SANITIZER_RISCV64 || defined(__sparc__))
+  long data_arg = data;
+#    if defined(__sparc__)
+  // The meanings of addr and data are reversed for a few requests on
+  // Linux/sparc64.
+  if (request == ptrace_getregs || request == ptrace_getfpregs ||
+      request == ptrace_setregs || request == ptrace_setfpregs)
+    data_arg = addr;
+#    endif
+  if (res >= 0 && data_arg) {
     // Note that this is different from the interceptor in
     // sanitizer_common_interceptors.inc.
     // PEEK* requests return resulting values through data pointer.
     if (request == ptrace_getregs) {
-      POST_WRITE((void *)data, struct_user_regs_struct_sz);
+      POST_WRITE((void *)data_arg, struct_user_regs_struct_sz);
     } else if (request == ptrace_getfpregs) {
-      POST_WRITE((void *)data, struct_user_fpregs_struct_sz);
+      POST_WRITE((void *)data_arg, struct_user_fpregs_struct_sz);
     } else if (request == ptrace_getfpxregs) {
-      POST_WRITE((void *)data, struct_user_fpxregs_struct_sz);
+      POST_WRITE((void *)data_arg, struct_user_fpxregs_struct_sz);
     } else if (request == ptrace_getsiginfo) {
-      POST_WRITE((void *)data, siginfo_t_sz);
+      POST_WRITE((void *)data_arg, siginfo_t_sz);
     } else if (request == ptrace_getregset) {
-      __sanitizer_iovec *iov = (__sanitizer_iovec *)data;
+      __sanitizer_iovec *iov = (__sanitizer_iovec *)data_arg;
       POST_WRITE(iov->iov_base, iov->iov_len);
     } else if (request == ptrace_peekdata || request == ptrace_peektext ||
                request == ptrace_peekuser) {
-      POST_WRITE((void *)data, sizeof(void *));
+      POST_WRITE((void *)data_arg, sizeof(void *));
     }
   }
 #  endif
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
index e71a6bcd6a8371..b1dc1ec204bc8c 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -279,8 +279,9 @@
 #if SI_LINUX_NOT_ANDROID &&                                                \
     (defined(__i386) || defined(__x86_64) || defined(__mips64) ||          \
      defined(__powerpc64__) || defined(__aarch64__) || defined(__arm__) || \
-     defined(__s390__) || defined(__loongarch__) || SANITIZER_RISCV64)
-#define SANITIZER_INTERCEPT_PTRACE 1
+     defined(__s390__) || defined(__loongarch__) || SANITIZER_RISCV64 ||   \
+     defined(__sparc__))
+#  define SANITIZER_INTERCEPT_PTRACE 1
 #else
 #define SANITIZER_INTERCEPT_PTRACE 0
 #endif
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
index 6d61d276d77e35..67250de1eed695 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
@@ -94,8 +94,9 @@
 #if SANITIZER_LINUX
 # include <utime.h>
 # include <sys/ptrace.h>
-#    if defined(__mips64) || defined(__aarch64__) || defined(__arm__) || \
-        defined(__hexagon__) || defined(__loongarch__) ||SANITIZER_RISCV64
+#    if defined(__mips64) || defined(__aarch64__) || defined(__arm__) ||       \
+        defined(__hexagon__) || defined(__loongarch__) || SANITIZER_RISCV64 || \
+        defined(__sparc__)
 #      include <asm/ptrace.h>
 #      ifdef __arm__
 typedef struct user_fpregs elf_fpregset_t;
@@ -358,11 +359,12 @@ unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr);
   const int wordexp_wrde_dooffs = WRDE_DOOFFS;
 #  endif  // !SANITIZER_ANDROID
 
-#if SANITIZER_LINUX && !SANITIZER_ANDROID &&                               \
-    (defined(__i386) || defined(__x86_64) || defined(__mips64) ||          \
-     defined(__powerpc64__) || defined(__aarch64__) || defined(__arm__) || \
-     defined(__s390__) || defined(__loongarch__)|| SANITIZER_RISCV64)
-#if defined(__mips64) || defined(__powerpc64__) || defined(__arm__)
+#  if SANITIZER_LINUX && !SANITIZER_ANDROID &&                               \
+      (defined(__i386) || defined(__x86_64) || defined(__mips64) ||          \
+       defined(__powerpc64__) || defined(__aarch64__) || defined(__arm__) || \
+       defined(__s390__) || defined(__loongarch__) || SANITIZER_RISCV64 ||   \
+       defined(__sparc__))
+#    if defined(__mips64) || defined(__powerpc64__) || defined(__arm__)
   unsigned struct_user_regs_struct_sz = sizeof(struct pt_regs);
   unsigned struct_user_fpregs_struct_sz = sizeof(elf_fpregset_t);
 #elif SANITIZER_RISCV64
@@ -377,19 +379,22 @@ unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr);
 #elif defined(__s390__)
   unsigned struct_user_regs_struct_sz = sizeof(struct _user_regs_struct);
   unsigned struct_user_fpregs_struct_sz = sizeof(struct _user_fpregs_struct);
-#else
+#    elif defined(__sparc__)
+  unsigned struct_user_regs_struct_sz = sizeof(struct sunos_regs);
+  unsigned struct_user_fpregs_struct_sz = sizeof(struct sunos_fp);
+#    else
   unsigned struct_user_regs_struct_sz = sizeof(struct user_regs_struct);
   unsigned struct_user_fpregs_struct_sz = sizeof(struct user_fpregs_struct);
-#endif // __mips64 || __powerpc64__ || __aarch64__ || __loongarch__
-#if defined(__x86_64) || defined(__mips64) || defined(__powerpc64__) || \
-    defined(__aarch64__) || defined(__arm__) || defined(__s390__) ||    \
-    defined(__loongarch__) || SANITIZER_RISCV64
+#    endif  // __mips64 || __powerpc64__ || __aarch64__ || __loongarch__
+#    if defined(__x86_64) || defined(__mips64) || defined(__powerpc64__) || \
+        defined(__aarch64__) || defined(__arm__) || defined(__s390__) ||    \
+        defined(__loongarch__) || SANITIZER_RISCV64 || defined(__sparc__)
   unsigned struct_user_fpxregs_struct_sz = 0;
 #else
   unsigned struct_user_fpxregs_struct_sz = sizeof(struct user_fpxregs_struct);
 #endif // __x86_64 || __mips64 || __powerpc64__ || __aarch64__ || __arm__
-// || __s390__ || __loongarch__
-#ifdef __arm__
+  // || __s390__ || __loongarch__ || SANITIZER_RISCV64 || __sparc__
+#    ifdef __arm__
   unsigned struct_user_vfpregs_struct_sz = ARM_VFPREGS_SIZE;
 #else
   unsigned struct_user_vfpregs_struct_sz = 0;
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
index 34bfef1f7ef456..370d73a03a73e0 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
@@ -855,10 +855,11 @@ typedef void __sanitizer_FILE;
 # define SANITIZER_HAS_STRUCT_FILE 0
 #endif
 
-#if SANITIZER_LINUX && !SANITIZER_ANDROID &&                               \
-    (defined(__i386) || defined(__x86_64) || defined(__mips64) ||          \
-     defined(__powerpc64__) || defined(__aarch64__) || defined(__arm__) || \
-     defined(__s390__) || defined(__loongarch__) || SANITIZER_RISCV64)
+#  if SANITIZER_LINUX && !SANITIZER_ANDROID &&                               \
+      (defined(__i386) || defined(__x86_64) || defined(__mips64) ||          \
+       defined(__powerpc64__) || defined(__aarch64__) || defined(__arm__) || \
+       defined(__s390__) || defined(__loongarch__) || SANITIZER_RISCV64 ||   \
+       defined(__sparc__))
 extern unsigned struct_user_regs_struct_sz;
 extern unsigned struct_user_fpregs_struct_sz;
 extern unsigned struct_user_fpxregs_struct_sz;
diff --git a/compiler-rt/test/asan/TestCases/Linux/ptrace.cpp b/compiler-rt/test/asan/TestCases/Linux/ptrace.cpp
index e01021ff344c3a..edff30e5a47538 100644
--- a/compiler-rt/test/asan/TestCases/Linux/ptrace.cpp
+++ b/compiler-rt/test/asan/TestCases/Linux/ptrace.cpp
@@ -81,6 +81,13 @@ typedef __riscv_q_ext_state fpregs_struct;
 #define PRINT_REG_PC(__regs) printf("%lx\n", (unsigned long)(__regs.pc))
 #define PRINT_REG_FP(__fpregs) printf("%lx\n", (unsigned long)(__fpregs.fcsr))
 #define ARCH_IOVEC_FOR_GETREGSET
+
+#elif defined(__sparc__)
+typedef sunos_regs regs_struct;
+typedef sunos_fp fpregs_struct;
+#  define PRINT_REG_PC(__regs) printf("%x\n", (unsigned)(__regs.pc))
+#  define PRINT_REG_FP(__fpregs) printf("%x\n", (unsigned)(__fpregs.fsr))
+#  define __PTRACE_FPREQUEST PTRACE_GETFPREGS
 #endif
 
 
@@ -110,7 +117,13 @@ int main(void) {
     regset_io.iov_len = sizeof(regs_struct);
 #else
 # define __PTRACE_REQUEST  PTRACE_GETREGS
-# define __PTRACE_ARGS     NULL, pregs
+#  ifdef __sparc__
+    // The meanings of addr and data are reversed for a few requests on
+    // Linux/sparc64.
+#    define __PTRACE_ARGS pregs, NULL
+#  else
+#    define __PTRACE_ARGS NULL, pregs
+#  endif
 #endif
     res = ptrace((enum __ptrace_request)__PTRACE_REQUEST, pid, __PTRACE_ARGS);
     // CHECK: AddressSanitizer: stack-buffer-overflow
@@ -127,7 +140,13 @@ int main(void) {
     res = ptrace((enum __ptrace_request)PTRACE_GETREGSET, pid, (void*)NT_FPREGSET,
                  (void*)&regset_io);
 #else
-# define __PTRACE_FPARGS     NULL, &fpregs
+    // The meanings of addr and data are reversed for a few requests on
+    // Linux/sparc64.
+#  ifdef __sparc__
+#    define __PTRACE_FPARGS &fpregs, NULL
+#  else
+#    define __PTRACE_FPARGS NULL, &fpregs
+#  endif
 #endif
     res = ptrace((enum __ptrace_request)__PTRACE_FPREQUEST, pid, __PTRACE_FPARGS);
     assert(!res);

Copy link
Collaborator

@vitalybuka vitalybuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but extracting helper would be nice

@rorth rorth merged commit 587eaef into llvm:main Sep 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants