Handle S2 unwinding by trapping ptrace

PiperOrigin-RevId: 491893277
Change-Id: I427a2e485173c73fffead43e29511460c58c4f04

cmake/SapiDeps.cmake

@@ -77,7 +77,7 @@ endif()
 if(SAPI_DOWNLOAD_LIBUNWIND)
   include(cmake/libunwind.cmake)
 endif()
-sapi_check_target(unwind_ptrace_wrapped)
+sapi_check_target(unwind_ptrace)
 
 if(SAPI_DOWNLOAD_PROTOBUF)
   include(cmake/protobuf.cmake)

cmake/libunwind.cmake

@@ -110,7 +110,7 @@ elseif(CMAKE_SYSTEM_PROCESSOR STREQUAL "arm")
   )
 endif()
 
-add_library(unwind_ptrace_wrapped STATIC
+add_library(unwind_ptrace STATIC
   # internal_headers
   ${libunwind_SOURCE_DIR}/include/compiler.h
   ${libunwind_SOURCE_DIR}/include/config.h
@@ -185,35 +185,22 @@ add_library(unwind_ptrace_wrapped STATIC
   # source_ptrace
   ${_unwind_ptrace_srcs}
 )
-add_library(unwind::unwind_ptrace_wrapped ALIAS unwind_ptrace_wrapped)
+add_library(unwind::unwind_ptrace ALIAS unwind_ptrace)
-target_include_directories(unwind_ptrace_wrapped PUBLIC
+target_include_directories(unwind_ptrace PUBLIC
   ${libunwind_SOURCE_DIR}/include
   ${libunwind_SOURCE_DIR}/include/tdep
   ${libunwind_SOURCE_DIR}/include/tdep-${_unwind_cpu}
   ${libunwind_SOURCE_DIR}/src
 )
-target_compile_options(unwind_ptrace_wrapped PRIVATE
+target_compile_options(unwind_ptrace PRIVATE
   -fno-common
   -Wno-cpp
 )
-target_compile_definitions(unwind_ptrace_wrapped
+target_compile_definitions(unwind_ptrace
   PRIVATE -DHAVE_CONFIG_H
           -D_GNU_SOURCE
           -DNO_FRAME_POINTER
-  PUBLIC -D_UPT_accessors=_UPT_accessors_wrapped
-         -D_UPT_create=_UPT_create_wrapped
-         -D_UPT_destroy=_UPT_destroy_wrapped
-
-         -D_U${_unwind_cpu}_create_addr_space=_U${_unwind_cpu}_create_addr_space_wrapped
-         -D_U${_unwind_cpu}_destroy_addr_space=_U${_unwind_cpu}_destroy_addr_space_wrapped
-         -D_U${_unwind_cpu}_get_proc_name=_U${_unwind_cpu}_get_proc_name_wrapped
-         -D_U${_unwind_cpu}_get_reg=_U${_unwind_cpu}_get_reg_wrapped
-         -D_U${_unwind_cpu}_init_remote=_U${_unwind_cpu}_init_remote_wrapped
-         -D_U${_unwind_cpu}_step=_U${_unwind_cpu}_step_wrapped
-
-         -Dptrace=ptrace_wrapped
 )
-target_link_libraries(unwind_ptrace_wrapped PRIVATE
+target_link_libraries(unwind_ptrace PRIVATE
   sapi::base
-  sandbox2::ptrace_hook
 )

sandboxed_api/bazel/external/libunwind.BUILD

@@ -106,7 +106,7 @@ filegroup(
 )
 
 cc_library(
-    name = "unwind-ptrace-wrapped",
+    name = "unwind-ptrace",
     srcs = [
         "src/mi/Gdyn-remote.c",
         "src/ptrace/_UPT_access_fpreg.c",
@@ -138,20 +138,9 @@ cc_library(
         "-DNO_FRAME_POINTER",
         "-fno-common",
         "-Wno-cpp",  # Warning in src/ptrace/_UPT_get_dyn_info_list_addr.c
-        "-D_UPT_accessors=_UPT_accessors_wrapped",
-        "-D_UPT_create=_UPT_create_wrapped",
-        "-D_UPT_destroy=_UPT_destroy_wrapped",
-        "-D_Ux86_64_create_addr_space=_Ux86_64_create_addr_space_wrapped",
-        "-D_Ux86_64_destroy_addr_space=_Ux86_64_destroy_addr_space_wrapped",
-        "-D_Ux86_64_get_proc_name=_Ux86_64_get_proc_name_wrapped",
-        "-D_Ux86_64_get_reg=_Ux86_64_get_reg_wrapped",
-        "-D_Ux86_64_init_remote=_Ux86_64_init_remote_wrapped",
-        "-D_Ux86_64_step=_Ux86_64_step_wrapped",
-        "-Dptrace=ptrace_wrapped",
     ],
     visibility = ["//visibility:public"],
     deps = [
         ":included_sources",
-        "@com_google_sandboxed_api//sandboxed_api/sandbox2/unwind:ptrace_hook",
     ],
 )

sandboxed_api/sandbox2/policybuilder.cc

@@ -1246,6 +1246,12 @@ PolicyBuilder& PolicyBuilder::AddNetworkProxyHandlerPolicy() {
   return *this;
 }
 
+PolicyBuilder& PolicyBuilder::TrapPtrace() {
+  AddPolicyOnSyscall(__NR_ptrace, {TRAP(0)});
+  user_policy_handles_ptrace_ = true;
+  return *this;
+}
+
 PolicyBuilder& PolicyBuilder::SetRootWritable() {
   EnableNamespaces();  // NOLINT(clang-diagnostic-deprecated-declarations)
   mounts_.SetRootWritable();

sandboxed_api/sandbox2/policybuilder.h

@@ -605,6 +605,9 @@ class PolicyBuilder final {
   // Allows a limited version of madvise
   PolicyBuilder& AllowLimitedMadvise();
 
+  // Traps instead of denying ptrace.
+  PolicyBuilder& TrapPtrace();
+
   // Appends code to block a specific syscall and setting errno at the end of
   // the policy - decision taken by user policy take precedence.
   PolicyBuilder& OverridableBlockSyscallWithErrno(uint32_t num, int error);

sandboxed_api/sandbox2/stack_trace.cc

@@ -120,6 +120,7 @@ absl::StatusOr<std::unique_ptr<Policy>> StackTracePeer::GetPolicy(
       .AllowSyscall(__NR_madvise)
 
       // Required for our ptrace replacement.
+      .TrapPtrace()
       .AddPolicyOnSyscall(
           __NR_process_vm_readv,
           {

sandboxed_api/sandbox2/unwind/BUILD.bazel

@@ -28,8 +28,10 @@ cc_library(
     srcs = ["ptrace_hook.cc"],
     hdrs = ["ptrace_hook.h"],
     copts = sapi_platform_copts(),
-    visibility = ["@org_gnu_libunwind//:__subpackages__"],
+    deps = [
-    deps = ["@com_google_absl//absl/strings"],
+        "//sandboxed_api/sandbox2/util:syscall_trap",
+        "@com_google_absl//absl/strings",
+    ],
 )
 
 cc_library(
@@ -39,19 +41,6 @@ cc_library(
     copts = sapi_platform_copts([
         # TODO(cblichmann): Remove this, fix bazel/external/libunwind.BUILD
         "-Iexternal/org_gnu_libunwind/include",
-    ] + [
-        "-D{symbol}={symbol}_wrapped".format(symbol = symbol)
-        for symbol in [
-            "_UPT_accessors",
-            "_UPT_create",
-            "_UPT_destroy",
-            "_Ux86_64_create_addr_space",
-            "_Ux86_64_destroy_addr_space",
-            "_Ux86_64_get_proc_name",
-            "_Ux86_64_get_reg",
-            "_Ux86_64_init_remote",
-            "_Ux86_64_step",
-        ]
     ]),
     visibility = ["//visibility:public"],
     deps = [
@@ -68,7 +57,7 @@ cc_library(
         "@com_google_absl//absl/cleanup",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
-        "@org_gnu_libunwind//:unwind-ptrace-wrapped",
+        "@org_gnu_libunwind//:unwind-ptrace",
     ],
 )
 

sandboxed_api/sandbox2/unwind/CMakeLists.txt

@@ -20,6 +20,7 @@ add_library(sandbox2_ptrace_hook STATIC
 add_library(sandbox2::ptrace_hook ALIAS sandbox2_ptrace_hook)
 target_link_libraries(sandbox2_ptrace_hook
   PRIVATE sapi::base
+          sandbox2::syscall_trap
   PUBLIC absl::strings
 )
 
@@ -44,7 +45,7 @@ target_link_libraries(sandbox2_unwind PRIVATE
   sapi::file_helpers
   sapi::raw_logging
   sapi::status
-  unwind::unwind_ptrace_wrapped
+  unwind::unwind_ptrace
 )
 
 # sandboxed_api/sandbox2/unwind:unwind_proto

sandboxed_api/sandbox2/unwind/ptrace_hook.cc

@@ -17,6 +17,7 @@
 #include <elf.h>  // For NT_PRSTATUS
 #include <sys/ptrace.h>
 #include <sys/uio.h>
+#include <syscall.h>
 #include <unistd.h>
 
 #include <cstdint>
@@ -25,6 +26,8 @@
 #include <cstring>
 #include <vector>
 
+#include "sandboxed_api/sandbox2/util/syscall_trap.h"
+
 // Android doesn't use an enum for __ptrace_request, use int instead.
 #if defined(__ANDROID__)
 using PtraceRequest = int;
@@ -45,38 +48,23 @@ constexpr size_t kRegSize = sizeof(RegType);
 // limit).
 auto* g_registers = new std::vector<RegType>();
 
-// Whether ptrace() emulation is in effect. This can only be enabled (per
+// Hooks ptrace.
-// 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 EnablePtraceEmulationWithUserRegs().
-extern "C" long int ptrace_wrapped(  // NOLINT
+long int ptrace_hook(  // NOLINT
     PtraceRequest request, pid_t pid, void* addr, void* data) {
-  if (!g_emulate_ptrace) {
-    return ptrace(request, pid, addr, data);
-  }
-
   switch (request) {
     case PTRACE_PEEKDATA: {
-      long int read_data;  // NOLINT
+      RegType read_data;
       iovec local = {.iov_base = &read_data, .iov_len = sizeof(read_data)};
       iovec remote = {.iov_base = addr, .iov_len = sizeof(read_data)};
 
       if (process_vm_readv(pid, &local, 1, &remote, 1, 0) <= 0) {
         return -1;
       }
-      return read_data;
+      *reinterpret_cast<RegType*>(data) = read_data;
+      break;
     }
     case PTRACE_PEEKUSER: {
       // Make sure read is in-bounds and aligned.
@@ -85,7 +73,8 @@ extern "C" long int ptrace_wrapped(  // NOLINT
           offset % kRegSize != 0) {
         return -1;
       }
-      return (*g_registers)[offset / kRegSize];
+      *reinterpret_cast<RegType*>(data) = (*g_registers)[offset / kRegSize];
+      break;
     }
     case PTRACE_GETREGSET: {
       // Only return general-purpose registers.
@@ -100,11 +89,26 @@ extern "C" long int ptrace_wrapped(  // NOLINT
       break;
     }
     default:
-      fprintf(stderr, "ptrace_wrapped(): operation not permitted: %d\n",
+      fprintf(stderr, "ptrace_hook(): operation not permitted: %d\n", request);
-              request);
       abort();
   }
   return 0;
 }
 
+}  // namespace
+
+void EnablePtraceEmulationWithUserRegs(absl::string_view regs) {
+  g_registers->resize((regs.size() + 1) / kRegSize);
+  memcpy(&g_registers->front(), regs.data(), regs.size());
+  SyscallTrap::Install([](int nr, SyscallTrap::Args args, uintptr_t* rv) {
+    if (nr != __NR_ptrace) {
+      return false;
+    }
+    *rv = ptrace_hook(
+        static_cast<PtraceRequest>(args[0]), static_cast<pid_t>(args[1]),
+        reinterpret_cast<void*>(args[2]), reinterpret_cast<void*>(args[3]));
+    return true;
+  });
+}
+
 }  // namespace sandbox2