From ab7943abdc7e6543db21c3ee794b47e46b400b48 Mon Sep 17 00:00:00 2001 From: Christian Blichmann Date: Thu, 22 Apr 2021 06:56:22 -0700 Subject: [PATCH] Simplify ptrace emulation and code style fixes PiperOrigin-RevId: 369862187 Change-Id: Ia0759c320cde1c9e3798f0df5c2a0d50ca20fd71 --- sandboxed_api/sandbox2/unwind/BUILD.bazel | 1 + sandboxed_api/sandbox2/unwind/CMakeLists.txt | 5 +- sandboxed_api/sandbox2/unwind/ptrace_hook.cc | 97 ++++++++++---------- sandboxed_api/sandbox2/unwind/ptrace_hook.h | 9 +- sandboxed_api/sandbox2/unwind/unwind.cc | 4 +- 5 files changed, 56 insertions(+), 60 deletions(-) diff --git a/sandboxed_api/sandbox2/unwind/BUILD.bazel b/sandboxed_api/sandbox2/unwind/BUILD.bazel index 9ac2e1e..a6c1707 100644 --- a/sandboxed_api/sandbox2/unwind/BUILD.bazel +++ b/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( diff --git a/sandboxed_api/sandbox2/unwind/CMakeLists.txt b/sandboxed_api/sandbox2/unwind/CMakeLists.txt index 58ac1fc..9107ee0 100644 --- a/sandboxed_api/sandbox2/unwind/CMakeLists.txt +++ b/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 diff --git a/sandboxed_api/sandbox2/unwind/ptrace_hook.cc b/sandboxed_api/sandbox2/unwind/ptrace_hook.cc index 4a79d22..e9bc80b 100644 --- a/sandboxed_api/sandbox2/unwind/ptrace_hook.cc +++ b/sandboxed_api/sandbox2/unwind/ptrace_hook.cc @@ -21,80 +21,75 @@ #include #include #include +#include -// 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(); -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(®ister_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) { + enum __ptrace_request 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 + 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) { - return read_data; - } else { + if (process_vm_readv(pid, &local, 1, &remote, 1, 0) <= 0) { return -1; } - } break; - case PTRACE_PEEKUSER: { - uintptr_t next_offset = reinterpret_cast(addr) + reg_size; + return read_data; + } + case PTRACE_PEEKUSER: // Make sure read is in-bounds and aligned. - if (next_offset <= n_register_values_bytes_used && - reinterpret_cast(addr) % reg_size == 0) { - return reinterpret_cast( - ®ister_values)[reinterpret_cast(addr) / reg_size]; - } else { + if (uintptr_t offset = reinterpret_cast(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 diff --git a/sandboxed_api/sandbox2/unwind/ptrace_hook.h b/sandboxed_api/sandbox2/unwind/ptrace_hook.h index 1ba1a6d..773d7ed 100644 --- a/sandboxed_api/sandbox2/unwind/ptrace_hook.h +++ b/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 +#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_ diff --git a/sandboxed_api/sandbox2/unwind/unwind.cc b/sandboxed_api/sandbox2/unwind/unwind.cc index a8b0a00..f5ae330 100644 --- a/sandboxed_api/sandbox2/unwind/unwind.cc +++ b/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 ips; std::vector stack_trace =