Skip to content

Commit

Permalink
Automated rollback of commit f06ee44.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 705414332
Change-Id: I420eccf475bde17f28d1f54d9106407a7f5f0c24
  • Loading branch information
@happyCoder92 @copybara-github
happyCoder92 authored and copybara-github committed Dec 12, 2024
1 parent 19e0490 commit 47dcb51
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 44 deletions.
6 changes: 3 additions & 3 deletions sandboxed_api/sandbox2/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,8 @@ add_library(sandbox2_monitor_ptrace ${SAPI_LIB_TYPE}
)
add_library(sandbox2::monitor_ptrace ALIAS sandbox2_monitor_ptrace)
target_link_libraries(sandbox2_monitor_ptrace
PRIVATE absl::cleanup
PRIVATE absl::core_headers
absl::cleanup
absl::flat_hash_set
absl::flags
absl::log
Expand All @@ -485,15 +486,14 @@ target_link_libraries(sandbox2_monitor_ptrace
sapi::status
sandbox2::client
sandbox2::comms
sandbox2::pid_waiter
sandbox2::result
sandbox2::sanitizer
sandbox2::util
PUBLIC absl::check
absl::core_headers
sandbox2::executor
sandbox2::monitor_base
sandbox2::notify
sandbox2::pid_waiter
sandbox2::policy
sandbox2::regs
sandbox2::syscall
Expand Down
74 changes: 44 additions & 30 deletions sandboxed_api/sandbox2/monitor_ptrace.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,13 +200,14 @@ bool PtraceMonitor::InterruptSandboxee() {
#define __WPTRACEEVENT(x) ((x & 0xff0000) >> 16)

void PtraceMonitor::NotifyMonitor() {
absl::MutexLock lock(&notify_mutex_);
pid_waiter_.SetDeadline(absl::InfinitePast());
notified_ = true;
absl::ReaderMutexLock lock(&notify_mutex_);
if (thread_.IsJoinable()) {
pthread_kill(thread_.handle(), SIGCHLD);
}
}

void PtraceMonitor::Join() {
absl::MutexLock lock(&thread_mutex_);
absl::MutexLock lock(&notify_mutex_);
if (thread_.IsJoinable()) {
thread_.Join();
CHECK(IsDone()) << "Monitor did not terminate";
Expand All @@ -216,10 +217,7 @@ void PtraceMonitor::Join() {
}

void PtraceMonitor::RunInternal() {
{
absl::MutexLock lock(&thread_mutex_);
thread_ = sapi::Thread(this, &PtraceMonitor::Run, "sandbox2-Monitor");
}
thread_ = sapi::Thread(this, &PtraceMonitor::Run, "sandbox2-Monitor");

// Wait for the Monitor to set-up the sandboxee correctly (or fail while
// doing that). From here on, it is safe to use the IPC object for
Expand All @@ -234,6 +232,12 @@ void PtraceMonitor::Run() {
};

absl::Cleanup setup_notify = [this] { setup_notification_.Notify(); };
// It'd be costly to initialize the sigset_t for each sigtimedwait()
// invocation, so do it once per Monitor.
if (!InitSetupSignals()) {
SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_SIGNALS);
return;
}
// This call should be the last in the init sequence, because it can cause the
// sandboxee to enter ptrace-stopped state, in which it will not be able to
// send any messages over the Comms channel.
Expand All @@ -247,15 +251,11 @@ void PtraceMonitor::Run() {
std::move(setup_notify).Invoke();

bool sandboxee_exited = false;
pid_waiter_.SetPriorityPid(process_.main_pid);
PidWaiter pid_waiter(process_.main_pid);
int status;
// All possible still running children of main process, will be killed due to
// PTRACE_O_EXITKILL ptrace() flag.
while (result().final_status() == Result::UNSET) {
{
absl::MutexLock lock(&notify_mutex_);
notified_ = false;
}
if (absl::Now() >= hard_deadline_) {
LOG(WARNING) << "Hard deadline exceeded (timed_out=" << timed_out_
<< ", external_kill=" << external_kill_
Expand Down Expand Up @@ -295,18 +295,13 @@ void PtraceMonitor::Run() {
break;
}
}
absl::Time effective_deadline = hard_deadline_;
if (deadline != 0 && hard_deadline_ == absl::InfiniteFuture()) {
effective_deadline = absl::FromUnixMillis(deadline);
}
{
absl::MutexLock lock(&notify_mutex_);
if (!notified_) {
pid_waiter_.SetDeadline(effective_deadline);
}
}
pid_t ret = pid_waiter_.Wait(&status);

pid_t ret = pid_waiter.Wait(&status);
if (ret == 0) {
constexpr timespec ts = {kWakeUpPeriodSec, kWakeUpPeriodNSec};
int signo = sigtimedwait(&sset_, nullptr, &ts);
LOG_IF(ERROR, signo != -1 && signo != SIGCHLD)
<< "Unknown signal received: " << signo;
continue;
}

Expand All @@ -315,7 +310,7 @@ void PtraceMonitor::Run() {
LOG(ERROR) << "PANIC(). The main process has not exited yet, "
<< "yet we haven't seen its exit event";
SetExitStatusCode(Result::INTERNAL_ERROR, Result::FAILED_CHILD);
} else if (errno != EINTR) {
} else {
PLOG(ERROR) << "waitpid() failed";
}
continue;
Expand Down Expand Up @@ -379,17 +374,14 @@ void PtraceMonitor::Run() {
absl::GetFlag(FLAGS_sandbox2_stack_traces_collection_timeout);
}
for (;;) {
auto left = deadline - absl::Now();
if (absl::Now() >= deadline) {
LOG(WARNING)
<< "Waiting for sandboxee exit timed out. Sandboxee result: "
<< result_.ToString();
break;
}
{
absl::MutexLock lock(&notify_mutex_);
pid_waiter_.SetDeadline(deadline);
}
pid_t ret = pid_waiter_.Wait(&status);
pid_t ret = pid_waiter.Wait(&status);
if (ret == -1) {
if (!log_stack_traces || ret != ECHILD) {
PLOG(ERROR) << "waitpid() failed";
Expand All @@ -405,6 +397,8 @@ void PtraceMonitor::Run() {
}

if (ret == 0) {
auto ts = absl::ToTimespec(left);
sigtimedwait(&sset_, nullptr, &ts);
continue;
}

Expand Down Expand Up @@ -440,6 +434,26 @@ void PtraceMonitor::LogStackTraceOfPid(pid_t pid) {
}
}

bool PtraceMonitor::InitSetupSignals() {
if (sigemptyset(&sset_) == -1) {
PLOG(ERROR) << "sigemptyset()";
return false;
}

// sigtimedwait will react (wake-up) to arrival of this signal.
if (sigaddset(&sset_, SIGCHLD) == -1) {
PLOG(ERROR) << "sigaddset(SIGCHLD)";
return false;
}

if (pthread_sigmask(SIG_BLOCK, &sset_, nullptr) == -1) {
PLOG(ERROR) << "pthread_sigmask(SIG_BLOCK, SIGCHLD)";
return false;
}

return true;
}

absl::Status TryAttach(const absl::flat_hash_set<int>& tasks,
absl::Time deadline,
absl::flat_hash_set<int>& tasks_attached) {
Expand Down
20 changes: 9 additions & 11 deletions sandboxed_api/sandbox2/monitor_ptrace.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <cstdint>
#include <memory>

#include "absl/base/thread_annotations.h"
#include "absl/container/flat_hash_map.h"
#include "absl/log/log.h"
#include "absl/synchronization/mutex.h"
Expand All @@ -35,7 +34,6 @@
#include "sandboxed_api/sandbox2/policy.h"
#include "sandboxed_api/sandbox2/regs.h"
#include "sandboxed_api/sandbox2/syscall.h"
#include "sandboxed_api/sandbox2/util/pid_waiter.h"
#include "sandboxed_api/util/thread.h"

namespace sandbox2 {
Expand Down Expand Up @@ -64,11 +62,14 @@ class PtraceMonitor : public MonitorBase {
absl::Time deadline = absl::Now() + limit;
deadline_millis_.store(absl::ToUnixMillis(deadline),
std::memory_order_relaxed);
NotifyMonitor();
}
}

private:
// Timeout used with sigtimedwait (0.5s).
static constexpr int kWakeUpPeriodSec = 0L;
static constexpr int kWakeUpPeriodNSec = (500L * 1000L * 1000L);

// Waits for events from monitored clients and signals from the main process.
void RunInternal() override;
void Join() override;
Expand Down Expand Up @@ -126,6 +127,9 @@ class PtraceMonitor : public MonitorBase {
// Returns false if an error occurred and process could not be interrupted.
bool InterruptSandboxee();

// Sets up required signal masks/handlers; prepare mask for sigtimedwait().
bool InitSetupSignals();

// ptrace(PTRACE_SEIZE) to the Client.
// Returns success/failure status.
bool InitPtraceAttach();
Expand Down Expand Up @@ -155,18 +159,12 @@ class PtraceMonitor : public MonitorBase {
sigset_t sset_;
// Deadline after which sandboxee get terminated via PTRACE_O_EXITKILL.
absl::Time hard_deadline_ = absl::InfiniteFuture();
// PidWaiter for waiting for sandboxee events.
PidWaiter pid_waiter_;

// Synchronizes joining the monitor thread.
absl::Mutex thread_mutex_;
// Monitor thread object.
sapi::Thread ABSL_GUARDED_BY(thread_mutex_) thread_;
sapi::Thread thread_;

// Synchronizes deadline setting and notifying the monitor.
// Synchronizes monitor thread deletion and notifying the monitor.
absl::Mutex notify_mutex_;
// True iff monitor thread is notified
bool notified_ ABSL_GUARDED_BY(notify_mutex_) = false;
};

} // namespace sandbox2
Expand Down

0 comments on commit 47dcb51

Please sign in to comment.