From c14312c3a2c3699ba2d574d24d17c5f8eeaa06e3 Mon Sep 17 00:00:00 2001 From: Wiktor Garbacz Date: Wed, 9 Aug 2023 05:59:49 -0700 Subject: [PATCH] Kill on each iteration of graceful exit loop I believe it's possible for the `main_pid` to disappear between `kill` and `sigtimedwait` by means of an `exec` from a multithreaded process (`PTRACE_EVENT_EXIT` happens after the `exec`ing thread changes its tid to main_pid) PiperOrigin-RevId: 555137959 Change-Id: Id22908fb31497c0906e4f4fda66400fbf9ac9efb --- sandboxed_api/sandbox2/monitor_ptrace.cc | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/sandboxed_api/sandbox2/monitor_ptrace.cc b/sandboxed_api/sandbox2/monitor_ptrace.cc index 803fa83..5589d46 100644 --- a/sandboxed_api/sandbox2/monitor_ptrace.cc +++ b/sandboxed_api/sandbox2/monitor_ptrace.cc @@ -407,10 +407,6 @@ void PtraceMonitor::Run() { const bool log_stack_traces = result_.final_status() != Result::OK && absl::GetFlag(FLAGS_sandbox2_log_all_stack_traces); - if (!log_stack_traces) { - // Try to make sure main pid is killed and reaped - kill(process_.main_pid, SIGKILL); - } constexpr auto kGracefullExitTimeout = absl::Milliseconds(200); auto deadline = absl::Now() + kGracefullExitTimeout; if (log_stack_traces) { @@ -432,9 +428,12 @@ void PtraceMonitor::Run() { } break; } - if (!log_stack_traces && ret == process_.main_pid && - (WIFSIGNALED(status) || WIFEXITED(status))) { - break; + if (!log_stack_traces) { + if (ret == process_.main_pid && + (WIFSIGNALED(status) || WIFEXITED(status))) { + break; + } + kill(process_.main_pid, SIGKILL); } if (ret == 0) { @@ -454,10 +453,6 @@ void PtraceMonitor::Run() { continue; } } - - if (!log_stack_traces) { - kill(process_.main_pid, SIGKILL); - } } } }