The current implementation of Sandbox::Terminate results in timeout's being reported to coroner in cases where a Restart or Terminate with graceful exit is requested.

This change requests an exit from the sandboxee and then awaits the result either with a timeout of 1 second (the grace period) or else with infinite duration - which would then report the timeout again.

PiperOrigin-RevId: 589128986
Change-Id: Icc948b37f13f46af907fd1eab649cabb5ed50b25
This commit is contained in:
Oliver Kunz 2023-12-08 07:47:19 -08:00 committed by Copybara-Service
parent 19d8f4729a
commit 39e49549e6
2 changed files with 22 additions and 20 deletions

View File

@ -140,16 +140,6 @@ absl::Status RPCChannel::Exit() {
// Try the RPC exit sequence. But, the only thing that matters as a success // Try the RPC exit sequence. But, the only thing that matters as a success
// indicator is whether the Comms channel had been closed // indicator is whether the Comms channel had been closed
comms_->SendTLV(comms::kMsgExit, 0, nullptr); comms_->SendTLV(comms::kMsgExit, 0, nullptr);
bool unused;
comms_->RecvBool(&unused);
if (!comms_->IsTerminated()) {
LOG(ERROR) << "Comms channel not terminated in Exit()";
// TODO(hamacher): Better error code
return absl::FailedPreconditionError(
"Comms channel not terminated in Exit()");
}
return absl::OkStatus(); return absl::OkStatus();
} }

View File

@ -40,6 +40,7 @@
#include "sandboxed_api/sandbox2/executor.h" #include "sandboxed_api/sandbox2/executor.h"
#include "sandboxed_api/sandbox2/policy.h" #include "sandboxed_api/sandbox2/policy.h"
#include "sandboxed_api/sandbox2/policybuilder.h" #include "sandboxed_api/sandbox2/policybuilder.h"
#include "sandboxed_api/sandbox2/result.h"
#include "sandboxed_api/sandbox2/sandbox2.h" #include "sandboxed_api/sandbox2/sandbox2.h"
#include "sandboxed_api/sandbox2/util/bpf_helper.h" #include "sandboxed_api/sandbox2/util/bpf_helper.h"
#include "sandboxed_api/util/fileops.h" #include "sandboxed_api/util/fileops.h"
@ -105,20 +106,31 @@ void Sandbox::Terminate(bool attempt_graceful_exit) {
return; return;
} }
absl::StatusOr<sandbox2::Result> result;
if (attempt_graceful_exit) { if (attempt_graceful_exit) {
// Gracefully ask it to exit (with 1 second limit) first, then kill it. if (absl::Status requested_exit = rpc_channel_->Exit();
Exit(); !requested_exit.ok()) {
} else { LOG(WARNING)
// Kill it straight away << "rpc_channel->Exit() failed, calling AwaitResultWithTimeout(1) "
s2_->Kill(); << requested_exit;
}
result = s2_->AwaitResultWithTimeout(absl::Seconds(1));
if (!result.ok()) {
LOG(WARNING) << "s2_->AwaitResultWithTimeout failed, status: "
<< result.status() << " Killing PID: " << pid();
}
} }
const auto& result = AwaitResult(); if (!attempt_graceful_exit || !result.ok()) {
if (result.final_status() == sandbox2::Result::OK && s2_->Kill();
result.reason_code() == 0) { result = s2_->AwaitResult();
VLOG(2) << "Sandbox2 finished with: " << result.ToString(); }
if (result->final_status() == sandbox2::Result::OK &&
result->reason_code() == 0) {
VLOG(2) << "Sandbox2 finished with: " << result->ToString();
} else { } else {
LOG(WARNING) << "Sandbox2 finished with: " << result.ToString(); LOG(WARNING) << "Sandbox2 finished with: " << result->ToString();
} }
} }