Allow collecting stacktraces on normal process exit

This mainly a debugging facility.
It makes diagnosing problems where sandboxed process just randomly exits whereas unsandboxed one runs to completion due to differences in the setup/environment much easier.

PiperOrigin-RevId: 391005548
Change-Id: Ia19fe6632748da93c1f4291bb55e895f50a4e2b0

sandboxed_api/sandbox2/monitor.cc

@@ -360,6 +360,8 @@ bool Monitor::ShouldCollectStackTrace() {
       return policy_->collect_stacktrace_on_signal_;
     case Result::VIOLATION:
       return policy_->collect_stacktrace_on_violation_;
+    case Result::OK:
+      return policy_->collect_stacktrace_on_exit_;
     default:
       return false;
   }
@@ -861,7 +863,9 @@ void Monitor::EventPtraceExec(pid_t pid, int event_msg) {
 
 void Monitor::EventPtraceExit(pid_t pid, int event_msg) {
   // A regular exit, let it continue (fast-path).
-  if (WIFEXITED(event_msg)) {
+  if (ABSL_PREDICT_TRUE(
+          WIFEXITED(event_msg) &&
+          (!policy_->collect_stacktrace_on_exit_ || pid != pid_))) {
     ContinueProcess(pid, 0);
     return;
   }
@@ -883,10 +887,11 @@ void Monitor::EventPtraceExit(pid_t pid, int event_msg) {
     return;
   }
 
-  // This can be reached in three cases:
+  // This can be reached in four cases:
   // 1) Process was killed from the sandbox.
   // 2) Process was killed because it hit a timeout.
   // 3) Regular signal/other exit cause.
+  // 4) Normal exit for which we want to obtain stack trace.
   if (pid == pid_) {
     VLOG(1) << "PID: " << pid << " main special exit";
     if (network_violation_) {
@@ -896,6 +901,8 @@ void Monitor::EventPtraceExit(pid_t pid, int event_msg) {
       SetExitStatusCode(Result::EXTERNAL_KILL, 0);
     } else if (timed_out_) {
       SetExitStatusCode(Result::TIMEOUT, 0);
+    } else if (WIFEXITED(event_msg)) {
+      SetExitStatusCode(Result::OK, WEXITSTATUS(event_msg));
     } else {
       SetExitStatusCode(Result::SIGNALED, WTERMSIG(event_msg));
     }

sandboxed_api/sandbox2/policy.h

@@ -96,6 +96,7 @@ class Policy final {
   bool collect_stacktrace_on_signal_ = true;
   bool collect_stacktrace_on_timeout_ = true;
   bool collect_stacktrace_on_kill_ = true;
+  bool collect_stacktrace_on_exit_ = false;
 
   // The capabilities to keep in the sandboxee.
   std::vector<int> capabilities_;

sandboxed_api/sandbox2/policybuilder.cc

@@ -807,6 +807,7 @@ absl::StatusOr<std::unique_ptr<Policy>> PolicyBuilder::TryBuild() {
   output->collect_stacktrace_on_violation_ = collect_stacktrace_on_violation_;
   output->collect_stacktrace_on_timeout_ = collect_stacktrace_on_timeout_;
   output->collect_stacktrace_on_kill_ = collect_stacktrace_on_kill_;
+  output->collect_stacktrace_on_exit_ = collect_stacktrace_on_exit_;
   output->user_policy_ = std::move(user_policy_);
   output->user_policy_handles_bpf_ = user_policy_handles_bpf_;
 
@@ -959,6 +960,11 @@ PolicyBuilder& PolicyBuilder::CollectStacktracesOnKill(bool enable) {
   return *this;
 }
 
+PolicyBuilder& PolicyBuilder::CollectStacktracesOnExit(bool enable) {
+  collect_stacktrace_on_exit_ = enable;
+  return *this;
+}
+
 PolicyBuilder& PolicyBuilder::AddNetworkProxyPolicy() {
   if (allowed_hosts_) {
     SetError(absl::FailedPreconditionError(

sandboxed_api/sandbox2/policybuilder.h

@@ -540,6 +540,9 @@ class PolicyBuilder final {
   // monitor / the user.
   PolicyBuilder& CollectStacktracesOnKill(bool enable);
 
+  // Enables/disables stack trace collection on normal process exit.
+  PolicyBuilder& CollectStacktracesOnExit(bool enable);
+
   // Appends an unconditional ALLOW action for all syscalls.
   // Do not use in environment with untrusted code and/or data, ask
   // sandbox-team@ first if unsure.
@@ -590,6 +593,7 @@ class PolicyBuilder final {
   bool collect_stacktrace_on_signal_ = true;
   bool collect_stacktrace_on_timeout_ = true;
   bool collect_stacktrace_on_kill_ = false;
+  bool collect_stacktrace_on_exit_ = false;
 
   // Seccomp fields
   std::vector<sock_filter> user_policy_;

sandboxed_api/sandbox2/sandbox2_test.cc

@@ -118,6 +118,26 @@ TEST(ExecutorTest, ExecutorFdConstructor) {
   ASSERT_EQ(result.final_status(), Result::OK);
 }
 
+TEST(StackTraceTest, StackTraceOnExitWorks) {
+  SKIP_SANITIZERS_AND_COVERAGE;
+
+  const std::string path = GetTestSourcePath("sandbox2/testcases/minimal");
+  std::vector<std::string> args = {path};
+  auto executor = absl::make_unique<Executor>(path, args);
+
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
+                            PolicyBuilder()
+                                // Don't restrict the syscalls at all.
+                                .DangerDefaultAllowAll()
+                                .CollectStacktracesOnExit(true)
+                                .TryBuild());
+  Sandbox2 sandbox(std::move(executor), std::move(policy));
+  auto result = sandbox.Run();
+
+  ASSERT_EQ(result.final_status(), Result::OK);
+  ASSERT_THAT(result.stack_trace(), Not(IsEmpty()));
+}
+
 // Tests that we return the correct state when the sandboxee was killed by an
 // external signal. Also make sure that we do not have the stack trace.
 TEST(RunAsyncTest, SandboxeeExternalKill) {