Do 1 level of recursion on libunwind crashes

PiperOrigin-RevId: 566617450
Change-Id: If5e3ce2e9763360c6cbd50145c432dfb62621136

cmake/abseil-cpp.cmake

@@ -19,6 +19,9 @@ FetchContent_Declare(absl
 set(ABSL_CXX_STANDARD ${SAPI_CXX_STANDARD} CACHE STRING "" FORCE)
 set(ABSL_PROPAGATE_CXX_STD ON CACHE BOOL "" FORCE)
 set(ABSL_RUN_TESTS OFF CACHE BOOL "" FORCE)
+set(ABSL_BUILD_TEST_HELPERS ON CACHE BOOL "" FORCE)
+set(ABSL_USE_EXTERNAL_GOOGLETEST ON)
+set(ABSL_FIND_GOOGLETEST OFF)
 set(ABSL_USE_GOOGLETEST_HEAD OFF CACHE BOOL "" FORCE)
 
 FetchContent_MakeAvailable(absl)

sandboxed_api/sandbox2/BUILD.bazel

@@ -1032,9 +1032,11 @@ cc_test(
         "//sandboxed_api:testing",
         "//sandboxed_api/util:fileops",
         "//sandboxed_api/util:status_matchers",
+        "@com_google_absl//absl/base:log_severity",
         "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/flags:reflection",
         "@com_google_absl//absl/log:check",
+        "@com_google_absl//absl/log:scoped_mock_log",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/time",
         "@com_google_googletest//:gtest_main",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -1118,6 +1118,8 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING)
   target_link_libraries(sandbox2_stack_trace_test PRIVATE
     absl::check
     absl::flags
+    absl::log_severity
+    absl::scoped_mock_log
     absl::status
     absl::strings
     absl::time

sandboxed_api/sandbox2/executor.h

@@ -97,6 +97,8 @@ class Executor final {
     return *this;
   }
 
+  int libunwind_recursion_depth() { return libunwind_recursion_depth_; }
+
  private:
   friend class MonitorBase;
   friend class PtraceMonitor;
@@ -104,8 +106,9 @@ class Executor final {
 
   // Internal constructor for executing libunwind on the given pid
   // enable_sandboxing_pre_execve=false as we are not going to execve.
-  explicit Executor(pid_t libunwind_sbox_for_pid)
+  explicit Executor(pid_t libunwind_sbox_for_pid, int libunwind_recursion_depth)
       : libunwind_sbox_for_pid_(libunwind_sbox_for_pid),
+        libunwind_recursion_depth_(libunwind_recursion_depth),
         enable_sandboxing_pre_execve_(false) {
     CHECK_GE(libunwind_sbox_for_pid_, 0);
     SetUpServerSideCommsFd();
@@ -131,6 +134,7 @@ class Executor final {
   // If this executor is running the libunwind sandbox for a process,
   // this variable will hold the PID of the process. Otherwise it is zero.
   pid_t libunwind_sbox_for_pid_ = 0;
+  int libunwind_recursion_depth_ = 0;
 
   // Should the sandboxing be enabled before execve() occurs, or the binary will
   // do it by itself, using the Client object's methods

sandboxed_api/sandbox2/monitor_base.cc

@@ -401,7 +401,7 @@ bool MonitorBase::StackTraceCollectionPossible() const {
   // recursion).
   if ((policy_->GetNamespace() ||
        absl::GetFlag(FLAGS_sandbox_libunwind_crash_handler) == false) &&
-      executor_->libunwind_sbox_for_pid_ == 0) {
+      executor_->libunwind_recursion_depth() <= 1) {
     return true;
   }
   LOG(ERROR) << "Cannot collect stack trace. Unwind pid "
@@ -443,7 +443,8 @@ bool MonitorBase::ShouldCollectStackTrace(Result::StatusEnum status) const {
 absl::StatusOr<std::vector<std::string>> MonitorBase::GetStackTrace(
     const Regs* regs) {
   return sandbox2::GetStackTrace(regs, policy_->GetNamespaceOrNull(),
-                                 uses_custom_forkserver_);
+                                 uses_custom_forkserver_,
+                                 executor_->libunwind_recursion_depth() + 1);
 }
 
 absl::StatusOr<std::vector<std::string>> MonitorBase::GetAndLogStackTrace(

sandboxed_api/sandbox2/stack_trace.cc

@@ -97,7 +97,8 @@ class StackTracePeer {
       const Namespace* ns, bool uses_custom_forkserver);
 
   static absl::StatusOr<std::vector<std::string>> LaunchLibunwindSandbox(
-      const Regs* regs, const Namespace* ns, bool uses_custom_forkserver);
+      const Regs* regs, const Namespace* ns, bool uses_custom_forkserver,
+      int recursion_depth);
 };
 
 absl::StatusOr<std::unique_ptr<Policy>> StackTracePeer::GetPolicy(
@@ -185,7 +186,8 @@ SandboxPeer::SpawnFn SandboxPeer::spawn_fn_ = nullptr;
 }  // namespace internal
 
 absl::StatusOr<std::vector<std::string>> StackTracePeer::LaunchLibunwindSandbox(
-    const Regs* regs, const Namespace* ns, bool uses_custom_forkserver) {
+    const Regs* regs, const Namespace* ns, bool uses_custom_forkserver,
+    int recursion_depth) {
   const pid_t pid = regs->pid();
 
   sapi::file_util::fileops::FDCloser memory_fd(
@@ -195,7 +197,7 @@ absl::StatusOr<std::vector<std::string>> StackTracePeer::LaunchLibunwindSandbox(
   }
   // Tell executor to use this special internal mode. Using `new` to access a
   // non-public constructor.
-  auto executor = absl::WrapUnique(new Executor(pid));
+  auto executor = absl::WrapUnique(new Executor(pid, recursion_depth));
 
   executor->limits()->set_rlimit_cpu(10).set_walltime_limit(absl::Seconds(5));
 
@@ -275,7 +277,8 @@ absl::StatusOr<std::vector<std::string>> StackTracePeer::LaunchLibunwindSandbox(
 
   absl::Cleanup kill_sandbox = [&sandbox]() {
     sandbox->Kill();
-    sandbox->AwaitResult().IgnoreResult();
+    sandbox2::Result result = sandbox->AwaitResult();
+    LOG(INFO) << "Libunwind execution status: " << result.ToString();
   };
 
   if (!comms->SendProtoBuf(msg)) {
@@ -313,7 +316,8 @@ absl::StatusOr<std::vector<std::string>> StackTracePeer::LaunchLibunwindSandbox(
 }
 
 absl::StatusOr<std::vector<std::string>> GetStackTrace(
-    const Regs* regs, const Namespace* ns, bool uses_custom_forkserver) {
+    const Regs* regs, const Namespace* ns, bool uses_custom_forkserver,
+    int recursion_depth) {
   if (absl::GetFlag(FLAGS_sandbox_disable_all_stack_traces)) {
     return absl::UnavailableError("Stacktraces disabled");
   }
@@ -333,8 +337,8 @@ absl::StatusOr<std::vector<std::string>> GetStackTrace(
     return UnsafeGetStackTrace(regs->pid());
   }
 
-  return StackTracePeer::LaunchLibunwindSandbox(regs, ns,
+  return StackTracePeer::LaunchLibunwindSandbox(
-                                                uses_custom_forkserver);
+      regs, ns, uses_custom_forkserver, recursion_depth);
 }
 
 std::vector<std::string> CompactStackTrace(

sandboxed_api/sandbox2/stack_trace.h

@@ -37,6 +37,7 @@
 namespace sandbox2 {
 
 class Sandbox2;
+class StackTraceTestPeer;
 
 namespace internal {
 
@@ -56,6 +57,7 @@ class SandboxPeer {
 
  private:
   friend class ::sandbox2::Sandbox2;
+  friend class ::sandbox2::StackTraceTestPeer;
   using SpawnFn = std::unique_ptr<SandboxPeer> (*)(std::unique_ptr<Executor>,
                                                    std::unique_ptr<Policy>);
   static SpawnFn spawn_fn_;
@@ -68,7 +70,8 @@ constexpr size_t kDefaultMaxFrames = 200;
 
 // Returns the stack-trace of the PID=pid, one line per frame.
 absl::StatusOr<std::vector<std::string>> GetStackTrace(
-    const Regs* regs, const Namespace* ns, bool uses_custom_forkserver);
+    const Regs* regs, const Namespace* ns, bool uses_custom_forkserver,
+    int recursion_depth);
 
 // Returns a stack trace that collapses duplicate stack frames and annotates
 // them with a repetition count.

sandboxed_api/sandbox2/stack_trace_test.cc

@@ -25,10 +25,12 @@
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "absl/base/log_severity.h"
 #include "absl/flags/declare.h"
 #include "absl/flags/flag.h"
 #include "absl/flags/reflection.h"
 #include "absl/log/check.h"
+#include "absl/log/scoped_mock_log.h"
 #include "absl/strings/str_cat.h"
 #include "absl/time/time.h"
 #include "sandboxed_api/sandbox2/executor.h"
@@ -44,12 +46,57 @@
 ABSL_DECLARE_FLAG(bool, sandbox_libunwind_crash_handler);
 
 namespace sandbox2 {
+
+class StackTraceTestPeer {
+ public:
+  static StackTraceTestPeer& GetInstance() {
+    static auto* peer = new StackTraceTestPeer();
+    return *peer;
+  }
+  std::unique_ptr<internal::SandboxPeer> SpawnFn(
+      std::unique_ptr<Executor> executor, std::unique_ptr<Policy> policy) {
+    if (crash_unwind_) {
+      policy = PolicyBuilder().BuildOrDie();
+      crash_unwind_ = false;
+    }
+    return old_spawn_fn_(std::move(executor), std::move(policy));
+  }
+  void ReplaceSpawnFn() {
+    old_spawn_fn_ = internal::SandboxPeer::spawn_fn_;
+    internal::SandboxPeer::spawn_fn_ = +[](std::unique_ptr<Executor> executor,
+                                           std::unique_ptr<Policy> policy) {
+      return GetInstance().SpawnFn(std::move(executor), std::move(policy));
+    };
+  }
+  void RestoreSpawnFn() { internal::SandboxPeer::spawn_fn_ = old_spawn_fn_; }
+  void CrashNextUnwind() { crash_unwind_ = true; }
+
+ private:
+  internal::SandboxPeer::SpawnFn old_spawn_fn_;
+  bool crash_unwind_ = false;
+};
+
+struct ScopedSpawnOverride {
+  ScopedSpawnOverride() { StackTraceTestPeer::GetInstance().ReplaceSpawnFn(); }
+  ~ScopedSpawnOverride() { StackTraceTestPeer::GetInstance().RestoreSpawnFn(); }
+  ScopedSpawnOverride(ScopedSpawnOverride&&) = delete;
+  ScopedSpawnOverride& operator=(ScopedSpawnOverride&&) = delete;
+  ScopedSpawnOverride(const ScopedSpawnOverride&) = delete;
+  ScopedSpawnOverride& operator=(const ScopedSpawnOverride&) = delete;
+
+  void CrashNextUnwind() {
+    StackTraceTestPeer::GetInstance().CrashNextUnwind();
+  }
+};
+
 namespace {
 
 namespace file_util = ::sapi::file_util;
 using ::sapi::CreateDefaultPermissiveTestPolicy;
 using ::sapi::GetTestSourcePath;
+using ::testing::_;
 using ::testing::Contains;
+using ::testing::ContainsRegex;
 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::IsEmpty;
@@ -213,6 +260,30 @@ TEST(StackTraceTest, CompactStackTrace) {
                           "(previous frame repeated 3 times)"));
 }
 
+TEST(StackTraceTest, RecursiveStackTrace) {
+  // Very first sandbox run will initialize spawn_fn_
+  SKIP_SANITIZERS;
+  ScopedSpawnOverride spawn_override;
+  SymbolizationWorksCommon({});
+  absl::ScopedMockLog log;
+  EXPECT_CALL(
+      log,
+      Log(absl::LogSeverity::kInfo, _,
+          ContainsRegex(
+              "Libunwind execution status: SYSCALL VIOLATION.*Stack: \\w+")));
+  spawn_override.CrashNextUnwind();
+  const std::string path = GetTestSourcePath("sandbox2/testcases/symbolize");
+  std::vector<std::string> args = {path, absl::StrCat(1), absl::StrCat(1)};
+  PolicyBuilder builder = CreateDefaultPermissiveTestPolicy(path);
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy, builder.TryBuild());
+
+  Sandbox2 s2(std::make_unique<Executor>(path, args), std::move(policy));
+  log.StartCapturingLogs();
+  ASSERT_TRUE(s2.RunAsync());
+  auto result = s2.AwaitResult();
+  EXPECT_THAT(result.final_status(), Eq(Result::SIGNALED));
+}
+
 INSTANTIATE_TEST_SUITE_P(
     Instantiation, StackTraceTest,
     ::testing::Values(