Simplify ptrace emulation and code style fixes

PiperOrigin-RevId: 369862187
Change-Id: Ia0759c320cde1c9e3798f0df5c2a0d50ca20fd71

sandboxed_api/sandbox2/unwind/BUILD.bazel

@@ -27,6 +27,7 @@ cc_library(
     hdrs = ["ptrace_hook.h"],
     copts = sapi_platform_copts(),
     visibility = ["@org_gnu_libunwind//:__subpackages__"],
+    deps = ["@com_google_absl//absl/strings"],
 )
 
 cc_library(

sandboxed_api/sandbox2/unwind/CMakeLists.txt

@@ -18,8 +18,9 @@ add_library(sandbox2_ptrace_hook STATIC
   ptrace_hook.h
 )
 add_library(sandbox2::ptrace_hook ALIAS sandbox2_ptrace_hook)
-target_link_libraries(sandbox2_ptrace_hook PRIVATE
-  sapi::base
+target_link_libraries(sandbox2_ptrace_hook
+  PRIVATE sapi::base
+  PUBLIC absl::strings
 )
 
 # sandboxed_api/sandbox2/unwind:unwind

sandboxed_api/sandbox2/unwind/ptrace_hook.cc

@@ -21,80 +21,75 @@
 #include <cstdint>
 #include <cstdio>
 #include <cstring>
+#include <vector>
 
-// Maximum register struct size: 128 u64.
-constexpr size_t kRegisterBufferSize = 128 * 8;
-// Contains the register values in a ptrace specified format.
-// This format is pretty opaque which is why we just forward
-// the raw bytes (up to a certain limit).
-static unsigned char register_values[kRegisterBufferSize];
-static size_t n_register_values_bytes_used = 0;
+namespace sandbox2 {
+namespace {
 
-// It should not be necessary to put this in a thread local storage as we
-// do not support setting up the forkserver when there is more than one thread.
-// However there might be some edge-cases, so we do this just in case.
-thread_local bool emulate_ptrace = false;
+// Register size is `long` for the supported architectures according to the
+// kernel.
+using RegType = long;  // NOLINT
+constexpr size_t kRegSize = sizeof(RegType);
 
-void ArmPtraceEmulation() { emulate_ptrace = true; }
+// Contains the register values in a ptrace specified format. This format is
+// pretty opaque which is why we just forward the raw bytes (up to a certain
+// limit).
+auto* g_registers = new std::vector<RegType>();
 
-void InstallUserRegs(const char *ptr, size_t size) {
-  if (sizeof(register_values) < size) {
-    fprintf(stderr, "install_user_regs: Got more bytes than supported (%lu)\n",
-            size);
-  } else {
-    memcpy(&register_values, ptr, size);
-    n_register_values_bytes_used = size;
-  }
+// Whether ptrace() emulation is in effect. This can only be enabled (per
+// thread), never disabled.
+thread_local bool g_emulate_ptrace = false;
+
+}  // namespace
+
+void EnablePtraceEmulationWithUserRegs(absl::string_view regs) {
+  g_registers->resize((regs.size() + 1) / kRegSize);
+  memcpy(&g_registers->front(), regs.data(), regs.size());
+  g_emulate_ptrace = true;
 }
 
 // Replaces the libc version of ptrace.
 // This wrapper makes use of process_vm_readv to read process memory instead of
 // issuing ptrace syscalls. Accesses to registers will be emulated, for this the
-// register values should be set via install_user_regs().
-// The emulation can be switched on using arm_ptrace_emulation().
+// register values should be set via EnablePtraceEmulationWithUserRegs().
 extern "C" long int ptrace_wrapped(  // NOLINT
     enum __ptrace_request request, pid_t pid, void* addr, void* data) {
-  // Register size is `long` for the supported architectures according to the
-  // kernel.
-  using reg_type = long;  // NOLINT
-  constexpr size_t reg_size = sizeof(reg_type);
-  if (!emulate_ptrace) {
+  if (!g_emulate_ptrace) {
     return ptrace(request, pid, addr, data);
   }
+
   switch (request) {
     case PTRACE_PEEKDATA: {
       long int read_data;  // NOLINT
+      struct iovec local = {
+          .iov_base = &read_data,
+          .iov_len = sizeof(long int),  // NOLINT
+      };
+      struct iovec remote = {
+          .iov_base = addr,
+          .iov_len = sizeof(long int),  // NOLINT
+      };
 
-      struct iovec local, remote;
-      local.iov_len = sizeof(long int);  // NOLINT
-      local.iov_base = &read_data;
-
-      remote.iov_len = sizeof(long int);  // NOLINT
-      remote.iov_base = addr;
-
-      if (process_vm_readv(pid, &local, 1, &remote, 1, 0) > 0) {
+      if (process_vm_readv(pid, &local, 1, &remote, 1, 0) <= 0) {
+        return -1;
+      }
       return read_data;
-      } else {
-        return -1;
     }
-    } break;
-    case PTRACE_PEEKUSER: {
-      uintptr_t next_offset = reinterpret_cast<uintptr_t>(addr) + reg_size;
+    case PTRACE_PEEKUSER:
       // Make sure read is in-bounds and aligned.
-      if (next_offset <= n_register_values_bytes_used &&
-          reinterpret_cast<uintptr_t>(addr) % reg_size == 0) {
-        return reinterpret_cast<reg_type *>(
-            &register_values)[reinterpret_cast<uintptr_t>(addr) / reg_size];
-      } else {
+      if (uintptr_t offset = reinterpret_cast<uintptr_t>(addr);
+          offset + kRegSize > g_registers->size() * kRegSize ||
+          offset % kRegSize != 0) {
         return -1;
+      } else {
+        return (*g_registers)[offset / kRegSize];
       }
-    } break;
-    default: {
-      fprintf(stderr, "ptrace-wrapper: forbidden operation invoked: %d\n",
+    default:
+      fprintf(stderr, "ptrace_wrapped(): operation not permitted: %d\n",
               request);
       _exit(1);
   }
-  }
-
   return 0;
 }
+
+}  // namespace sandbox2

sandboxed_api/sandbox2/unwind/ptrace_hook.h

@@ -15,12 +15,13 @@
 #ifndef SANDBOXED_API_SANDBOX2_UNWIND_PTRACE_HOOK_H_
 #define SANDBOXED_API_SANDBOX2_UNWIND_PTRACE_HOOK_H_
 
-#include <cstddef>
+#include "absl/strings/string_view.h"
+
+namespace sandbox2 {
 
 // Sets the register values that the ptrace emulation will return.
-void InstallUserRegs(const char* ptr, size_t size);
+void EnablePtraceEmulationWithUserRegs(absl::string_view regs);
 
-// Enables the ptrace emulation.
-void ArmPtraceEmulation();
+}  // namespace sandbox2
 
 #endif  // SANDBOXED_API_SANDBOX2_UNWIND_PTRACE_HOOK_H_

sandboxed_api/sandbox2/unwind/unwind.cc

@@ -123,9 +123,7 @@ bool RunLibUnwindAndSymbolizer(Comms* comms) {
     return false;
   }
 
-  const std::string& data = setup.regs();
-  InstallUserRegs(data.c_str(), data.length());
-  ArmPtraceEmulation();
+  EnablePtraceEmulationWithUserRegs(setup.regs());
 
   std::vector<uintptr_t> ips;
   std::vector<std::string> stack_trace =