diff --git a/cmake/SapiDeps.cmake b/cmake/SapiDeps.cmake index 9688d8f..d864a32 100644 --- a/cmake/SapiDeps.cmake +++ b/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) diff --git a/cmake/libunwind.cmake b/cmake/libunwind.cmake index a67532d..d918fd8 100644 --- a/cmake/libunwind.cmake +++ b/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) -target_include_directories(unwind_ptrace_wrapped PUBLIC +add_library(unwind::unwind_ptrace ALIAS unwind_ptrace) +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 ) diff --git a/sandboxed_api/bazel/external/libunwind.BUILD b/sandboxed_api/bazel/external/libunwind.BUILD index 12db868..d6bbe80 100644 --- a/sandboxed_api/bazel/external/libunwind.BUILD +++ b/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", ], ) diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc index ca47b6f..0d9aed7 100644 --- a/sandboxed_api/sandbox2/policybuilder.cc +++ b/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(); diff --git a/sandboxed_api/sandbox2/policybuilder.h b/sandboxed_api/sandbox2/policybuilder.h index d363440..118d2e7 100644 --- a/sandboxed_api/sandbox2/policybuilder.h +++ b/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); diff --git a/sandboxed_api/sandbox2/stack_trace.cc b/sandboxed_api/sandbox2/stack_trace.cc index 57a9749..c07375f 100644 --- a/sandboxed_api/sandbox2/stack_trace.cc +++ b/sandboxed_api/sandbox2/stack_trace.cc @@ -120,6 +120,7 @@ absl::StatusOr> StackTracePeer::GetPolicy( .AllowSyscall(__NR_madvise) // Required for our ptrace replacement. + .TrapPtrace() .AddPolicyOnSyscall( __NR_process_vm_readv, { diff --git a/sandboxed_api/sandbox2/unwind/BUILD.bazel b/sandboxed_api/sandbox2/unwind/BUILD.bazel index 4bdb88c..60bb949 100644 --- a/sandboxed_api/sandbox2/unwind/BUILD.bazel +++ b/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 = ["@com_google_absl//absl/strings"], + deps = [ + "//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", ], ) diff --git a/sandboxed_api/sandbox2/unwind/CMakeLists.txt b/sandboxed_api/sandbox2/unwind/CMakeLists.txt index 5f49371..59bb7e8 100644 --- a/sandboxed_api/sandbox2/unwind/CMakeLists.txt +++ b/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 diff --git a/sandboxed_api/sandbox2/unwind/ptrace_hook.cc b/sandboxed_api/sandbox2/unwind/ptrace_hook.cc index 9c5130b..8d07d1b 100644 --- a/sandboxed_api/sandbox2/unwind/ptrace_hook.cc +++ b/sandboxed_api/sandbox2/unwind/ptrace_hook.cc @@ -17,6 +17,7 @@ #include // For NT_PRSTATUS #include #include +#include #include #include @@ -25,6 +26,8 @@ #include #include +#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(); -// 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. +// Hooks 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(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(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", - request); + fprintf(stderr, "ptrace_hook(): operation not permitted: %d\n", 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(args[0]), static_cast(args[1]), + reinterpret_cast(args[2]), reinterpret_cast(args[3])); + return true; + }); +} + } // namespace sandbox2