Automated rollback of commit f6fd27618b446c68520b5e611fab155bb621f83b.

PiperOrigin-RevId: 529395980
Change-Id: I6a5d451ed84f8d4a522777815c6cc2d7d7a8923c

sandboxed_api/sandbox2/BUILD.bazel

@@ -268,6 +268,7 @@ cc_binary(
         ":util",
         "//sandboxed_api/util:raw_logging",
         "//sandboxed_api/util:strerror",
+        "@com_google_absl//absl/flags:parse",
         "@com_google_absl//absl/log:globals",
     ],
 )
@@ -628,6 +629,7 @@ cc_library(
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/container:flat_hash_set",
+        "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
@@ -647,6 +649,7 @@ cc_library(
         ":forkserver_cc_proto",
         "//sandboxed_api/util:fileops",
         "@com_google_absl//absl/base:core_headers",
+        "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/synchronization",
@@ -876,6 +879,7 @@ cc_test(
         ":sandbox2",
         "//sandboxed_api:testing",
         "//sandboxed_api/util:raw_logging",
+        "@com_google_absl//absl/log",
         "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/strings",
         "@com_google_googletest//:gtest_main",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -215,6 +215,8 @@ set_target_properties(sandbox2_forkserver_bin PROPERTIES
 add_executable(sandbox2::forkserver_bin ALIAS sandbox2_forkserver_bin)
 target_link_libraries(sandbox2_forkserver_bin PRIVATE
   absl::log_globals
+  absl::flags
+  absl::flags_parse
   sandbox2::client
   sandbox2::comms
   sandbox2::forkserver
@@ -590,6 +592,7 @@ target_link_libraries(sandbox2_fork_client
   PRIVATE sandbox2::comms
           sandbox2::forkserver_proto
   PUBLIC absl::core_headers
+         absl::flags
          absl::synchronization
          sapi::base
          sapi::fileops
@@ -924,6 +927,7 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING)
   )
   target_link_libraries(sandbox2_forkserver_test PRIVATE
     absl::check
+    absl::log
     absl::strings
     sandbox2::forkserver
     sandbox2::forkserver_proto

sandboxed_api/sandbox2/fork_client.cc

@@ -14,11 +14,19 @@
 
 #include "sandboxed_api/sandbox2/fork_client.h"
 
+#include "absl/flags/flag.h"
 #include "absl/log/check.h"
 #include "absl/log/log.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/forkserver.pb.h"
 
+// Make the forkserver use a signal handler for SIGCHLD instead of setting
+// `SA_NOCLDWAIT`. This is needed in certain cases where the process need
+// to be explicitly reaped by waitpid (see getrusage).
+ABSL_FLAG(
+    bool, sandbox2_forkserver_use_waitpid, false,
+    "Use waitpid to reap child processes instead of relying on SA_NOCLDWAIT");
+
 namespace sandbox2 {
 
 using ::sapi::file_util::fileops::FDCloser;

sandboxed_api/sandbox2/forkserver.cc

@@ -39,6 +39,8 @@
 
 #include "absl/container/flat_hash_map.h"
 #include "absl/container/flat_hash_set.h"
+#include "absl/flags/declare.h"
+#include "absl/flags/flag.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/match.h"
@@ -61,6 +63,8 @@
 #include "sandboxed_api/util/raw_logging.h"
 #include "sandboxed_api/util/strerror.h"
 
+ABSL_DECLARE_FLAG(bool, sandbox2_forkserver_use_waitpid);
+
 namespace sandbox2 {
 
 namespace file_util = ::sapi::file_util;
@@ -69,6 +73,18 @@ namespace {
 
 using ::sapi::StrError;
 
+void SigChldHandler(int signal) {
+  if (signal != SIGCHLD) {
+    return;
+  }
+
+  pid_t pid;
+  int wstatus;
+  do {
+    pid = waitpid(-1, &wstatus, WNOHANG);
+  } while (pid > 0);
+}
+
 // "Moves" FDs in move_fds from current to target FD number while keeping FDs
 // in keep_fds open - potentially moving them to another FD number as well in
 // case of colisions.
@@ -547,11 +563,20 @@ bool ForkServer::Initialize() {
   // Don't convert terminated child processes into zombies. It's up to the
   // sandbox (Monitor) to track them and receive/report their final status.
   struct sigaction sa;
-  sa.sa_handler = SIG_DFL;
-  sa.sa_flags = SA_NOCLDWAIT;
+  if (absl::GetFlag(FLAGS_sandbox2_forkserver_use_waitpid)) {
+    sa.sa_handler = SigChldHandler;
+    sa.sa_flags = 0;
+  } else {
+    sa.sa_handler = SIG_DFL;
+    sa.sa_flags = SA_NOCLDWAIT;
+  }
   sigemptyset(&sa.sa_mask);
   if (sigaction(SIGCHLD, &sa, nullptr) == -1) {
-    SAPI_RAW_PLOG(ERROR, "sigaction(SIGCHLD, flags=SA_NOCLDWAIT)");
+    if (absl::GetFlag(FLAGS_sandbox2_forkserver_use_waitpid)) {
+      SAPI_RAW_PLOG(ERROR, "sigaction(SIGCHLD, sa_handler=SigChldHandler)");
+    } else {
+      SAPI_RAW_PLOG(ERROR, "sigaction(SIGCHLD, flags=SA_NOCLDWAIT)");
+    }
     return false;
   }
   return true;

sandboxed_api/sandbox2/forkserver_bin.cc

@@ -19,16 +19,20 @@
 #include <csignal>
 #include <cstdlib>
 
+#include "absl/flags/parse.h"
 #include "absl/log/globals.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/forkserver.h"
 #include "sandboxed_api/sandbox2/sanitizer.h"
 #include "sandboxed_api/util/raw_logging.h"
 
-int main() {
+int main(int argc, char* argv[]) {
   // Make sure the logs go stderr.
   absl::SetStderrThreshold(absl::LogSeverityAtLeast::kInfo);
 
+  // Parse command line arguments.
+  absl::ParseCommandLine(argc, argv);
+
   // Close all non-essential FDs to keep newly opened FD numbers consistent.
   absl::Status status = sandbox2::sanitizer::CloseAllFDsExcept(
       {0, 1, 2, sandbox2::Comms::kSandbox2ClientCommsFD});

sandboxed_api/sandbox2/global_forkclient.cc

@@ -34,6 +34,7 @@
 #include <vector>
 
 #include "absl/cleanup/cleanup.h"
+#include "absl/flags/declare.h"
 #include "absl/flags/flag.h"
 #include "absl/log/log.h"
 #include "absl/status/status.h"
@@ -53,6 +54,8 @@
 #include "sandboxed_api/util/fileops.h"
 #include "sandboxed_api/util/raw_logging.h"
 
+ABSL_DECLARE_FLAG(bool, sandbox2_forkserver_use_waitpid);
+
 namespace sandbox2 {
 
 namespace file_util = ::sapi::file_util;
@@ -124,6 +127,7 @@ GlobalForkserverStartModeSet GetForkserverStartMode() {
 struct ForkserverArgs {
   int exec_fd;
   int comms_fd;
+  bool use_waitpid;
 };
 
 int LaunchForkserver(void* vargs) {
@@ -142,7 +146,11 @@ int LaunchForkserver(void* vargs) {
                   "duping comms fd failed");
 
   char proc_name[] = "S2-FORK-SERV";
-  char* const argv[] = {proc_name, nullptr};
+  char use_waitpid[] = "--sandbox2_forkserver_use_waitpid";
+  char* argv[] = {proc_name, nullptr, nullptr};
+  if (args->use_waitpid) {
+    argv[1] = use_waitpid;
+  }
   util::Execveat(args->exec_fd, "", argv, environ, AT_EMPTY_PATH);
   SAPI_RAW_PLOG(FATAL, "Could not launch forkserver binary");
 }
@@ -196,6 +204,7 @@ absl::StatusOr<std::unique_ptr<GlobalForkClient>> StartGlobalForkServer() {
   ForkserverArgs args = {
       .exec_fd = exec_fd,
       .comms_fd = sv[0],
+      .use_waitpid = absl::GetFlag(FLAGS_sandbox2_forkserver_use_waitpid),
   };
   pid_t pid = clone(LaunchForkserver, &stack[stack_size], clone_flags, &args,
                     nullptr, nullptr, nullptr);