UnotifyMonitor: Refactor killing the sandboxee/init

This is to avoid log spam - init can already be dead and waited when cleanup runs.
Additionally remove risk of killing wrong process when the pid is recycled - keeping reference to the proc dir keeps the pid around.

PiperOrigin-RevId: 903812678
Change-Id: Ia75c290c99dfcbb017ea369ba175b01dc9edfef5

Author: happyCoder92

sandboxed_api/sandbox2/monitor_unotify.cc

@@ -126,6 +126,17 @@ absl::Status WaitForTaskToStop(pid_t pid) {
                              : absl::InternalError("task did not stop");
 }
 
+void KillProcess(pid_t pid) {
+  VLOG(1) << "Sending SIGKILL to the PID: " << pid;
+  if (kill(pid, SIGKILL) != 0) {
+    if (errno != ESRCH) {
+      PLOG(ERROR) << "Could not send SIGKILL to PID " << pid;
+    } else {
+      VLOG(1) << "Process " << pid << " already dead";
+    }
+  }
+}
+
 }  // namespace
 
 UnotifyMonitor::UnotifyMonitor(Executor* executor, Policy* policy,
@@ -261,7 +272,15 @@ void UnotifyMonitor::Run() {
     OnDone();
   };
   absl::Cleanup setup_notify = [this] { setup_notification_.Notify(); };
-  absl::Cleanup process_cleanup = [this] { KillInit(); };
+  absl::Cleanup process_cleanup =
+      [this, init_pidfd = FDCloser(open(
+                 absl::StrCat("/proc/", process_.init_pid).c_str(), O_PATH))] {
+        KillInit();
+      };
+  // Keep reference to the main pid to avoid killing a random process recycling
+  // the pid.
+  FDCloser main_pidfd(
+      open(absl::StrCat("/proc/", process_.main_pid).c_str(), O_PATH));
   if (!InitSetupUnotify()) {
     SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_NOTIFY);
     return;
@@ -426,21 +445,9 @@ void UnotifyMonitor::NotifyMonitor() {
          sizeof(value));
 }
 
-bool UnotifyMonitor::KillSandboxee() {
-  VLOG(1) << "Sending SIGKILL to the PID: " << process_.main_pid;
-  if (kill(process_.main_pid, SIGKILL) != 0) {
-    PLOG(ERROR) << "Could not send SIGKILL to PID " << process_.main_pid;
-    return false;
-  }
-  return true;
-}
+void UnotifyMonitor::KillSandboxee() { KillProcess(process_.main_pid); }
 
-void UnotifyMonitor::KillInit() {
-  VLOG(1) << "Sending SIGKILL to the PID: " << process_.init_pid;
-  if (kill(process_.init_pid, SIGKILL) != 0) {
-    PLOG(ERROR) << "Could not send SIGKILL to PID " << process_.init_pid;
-  }
-}
+void UnotifyMonitor::KillInit() { KillProcess(process_.init_pid); }
 
 void UnotifyMonitor::Join() {
   absl::MutexLock lock(notify_mutex_);

sandboxed_api/sandbox2/monitor_unotify.h

@@ -77,8 +77,7 @@ class UnotifyMonitor : public MonitorBase {
   bool InitSetupUnotify();
   bool InitSetupNotifyEventFd();
   // Kills the main traced PID with SIGKILL.
-  // Returns false if an error occurred and process could not be killed.
-  bool KillSandboxee();
+  void KillSandboxee();
   void KillInit();
 
   void AllowSyscallViaUnotify(seccomp_notif req);