Start global fork-server on demand

Allow disabling global fork-server with a flag.

PiperOrigin-RevId: 340860588
Change-Id: I184603dc3a81eb90f715053e14fb3b8d66a6f104

sandboxed_api/sandbox2/BUILD.bazel

@@ -220,16 +220,34 @@ cc_library(
     srcs = ["global_forkclient.cc"],
     hdrs = ["global_forkclient.h"],
     copts = sapi_platform_copts(),
+    visibility = ["//visibility:public"],
     deps = [
         ":comms",
         ":fork_client",
         ":forkserver_bin_embed",
-        ":sanitizer",
+        ":forkserver_cc_proto",
+        ":util",
         "//sandboxed_api:embed_file",
+        "//sandboxed_api/sandbox2/util:fileops",
         "//sandboxed_api/sandbox2/util:strerror",
+        "//sandboxed_api/util:flags",
         "//sandboxed_api/util:raw_logging",
-        "@com_google_absl//absl/base:core_headers",
+        "@com_google_absl//absl/memory",
         "@com_google_absl//absl/strings",
+        "@com_google_glog//:glog",
+    ],
+)
+
+# Use only if Sandbox2 global forkserver has to be started very early on.
+# By default the forkserver is started on demand.
+cc_library(
+    name = "start_global_forkserver_lib_constructor",
+    srcs = ["global_forkclient_lib_ctor.cc"],
+    copts = sapi_platform_copts(),
+    visibility = ["//visibility:public"],
+    deps = [
+        ":global_forkserver",
+        "@com_google_absl//absl/base:core_headers",
     ],
 )
 

sandboxed_api/sandbox2/CMakeLists.txt

@@ -228,18 +228,36 @@ add_library(sandbox2_global_forkserver STATIC
   global_forkclient.h
 )
 add_library(sandbox2::global_forkserver ALIAS sandbox2_global_forkserver)
-target_link_libraries(sandbox2_global_forkserver PRIVATE
+target_link_libraries(sandbox2_global_forkserver
+  PRIVATE absl::memory
+          absl::strings
+          glog::glog
+          sandbox2::client
+          sandbox2::fileops
+          sandbox2::forkserver_bin_embed
+          sandbox2::strerror
+          sandbox2::util
+          sapi::base
+          sapi::embed_file
+          sapi::flags
+          sapi::raw_logging
+  PUBLIC sandbox2::comms
+         sandbox2::fork_client
+         sandbox2::forkserver_proto
+)
+
+# sandboxed_api/sandbox2:start_global_forkserver_lib_constructor
+# Use only if Sandbox2 global forkserver has to be started very early on.
+# By default the forkserver is started on demand.
+add_library(sandbox2_start_global_forkserver_lib_constructor STATIC
+  global_forkclient_lib_ctor.cc
+)
+add_library(sandbox2::start_global_forkserver_lib_constructor ALIAS
+  sandbox2_start_global_forkserver_lib_constructor)
+target_link_libraries(sandbox2_start_global_forkserver_lib_constructor PRIVATE
   absl::core_headers
-  absl::strings
-  sandbox2::client
-  sandbox2::comms
-  sandbox2::fork_client
-  sandbox2::forkserver_bin_embed
-  sandbox2::sanitizer
-  sandbox2::strerror
   sapi::base
-  sapi::embed_file
-  sapi::raw_logging
+  sandbox2::global_forkserver
 )
 
 # sandboxed_api/sandbox2:executor

sandboxed_api/sandbox2/executor.cc

@@ -46,14 +46,13 @@ Executor::Executor(int exec_fd, const std::string& path,
       exec_fd_(exec_fd),
       path_(path),
       argv_(argv),
-      envp_(envp) {
-  if (fork_client != nullptr) {
+      envp_(envp),
+      fork_client_(fork_client) {
+  if (fork_client_ != nullptr) {
     CHECK(exec_fd == -1 && path.empty());
-    fork_client_ = fork_client;
   } else {
     CHECK((exec_fd == -1 && (!path.empty() || libunwind_sbox_for_pid > 0)) ||
           (exec_fd >= 0 && path.empty()));
-    fork_client_ = GetGlobalForkClient();
   }
   SetUpServerSideCommsFd();
   SetDefaultCwd();
@@ -78,10 +77,6 @@ pid_t Executor::StartSubProcess(int32_t clone_flags, const Namespace* ns,
     LOG(ERROR) << "This executor has already been started";
     return -1;
   }
-  if (fork_client_ == nullptr) {
-    LOG(ERROR) << "The ForkClient object is not instantiated";
-    return -1;
-  }
 
   if (!path_.empty()) {
     exec_fd_ = open(path_.c_str(), O_PATH);
@@ -155,8 +150,14 @@ pid_t Executor::StartSubProcess(int32_t clone_flags, const Namespace* ns,
 
   pid_t init_pid = -1;
 
-  pid_t sandboxee_pid = fork_client_->SendRequest(
-      request, exec_fd_, client_comms_fd_, ns_fd, &init_pid);
+  pid_t sandboxee_pid;
+  if (fork_client_) {
+    sandboxee_pid = fork_client_->SendRequest(
+        request, exec_fd_, client_comms_fd_, ns_fd, &init_pid);
+  } else {
+    sandboxee_pid = GlobalForkClient::SendRequest(
+        request, exec_fd_, client_comms_fd_, ns_fd, &init_pid);
+  }
 
   if (init_pid < 0) {
     LOG(ERROR) << "Could not obtain init PID";
@@ -191,10 +192,11 @@ std::unique_ptr<ForkClient> Executor::StartForkServer() {
   // This flag is set explicitly to 'true' during object instantiation, and
   // custom fork-servers should never be sandboxed.
   set_enable_sandbox_before_exec(false);
-  if (StartSubProcess(0) == -1) {
+  pid_t pid = StartSubProcess(0);
+  if (pid == -1) {
     return nullptr;
   }
-  return absl::make_unique<ForkClient>(ipc_.comms());
+  return absl::make_unique<ForkClient>(pid, ipc_.comms());
 }
 
 void Executor::SetUpServerSideCommsFd() {

sandboxed_api/sandbox2/fork_client.h

@@ -30,8 +30,7 @@ class ForkRequest;
 
 class ForkClient {
  public:
-  explicit ForkClient(Comms* comms) : comms_(comms) {}
-
+  ForkClient(pid_t pid, Comms* comms) : pid_(pid), comms_(comms) {}
   ForkClient(const ForkClient&) = delete;
   ForkClient& operator=(const ForkClient&) = delete;
 
@@ -39,7 +38,11 @@ class ForkClient {
   pid_t SendRequest(const ForkRequest& request, int exec_fd, int comms_fd,
                     int user_ns_fd = -1, pid_t* init_pid = nullptr);
 
+  pid_t pid() { return pid_; }
+
  private:
+  // Pid of the ForkServer.
+  pid_t pid_;
   // Comms channel connecting with the ForkServer. Not owned by the object.
   Comms* comms_ ABSL_GUARDED_BY(comms_mutex_);
   // Mutex locking transactions (requests) over the Comms channel.

sandboxed_api/sandbox2/forkserver.cc

@@ -453,12 +453,6 @@ pid_t ForkServer::ServeRequest() {
 }
 
 bool ForkServer::Initialize() {
-  // If the parent goes down, so should we.
-  if (prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0) != 0) {
-    SAPI_RAW_PLOG(ERROR, "prctl(PR_SET_PDEATHSIG, SIGKILL)");
-    return false;
-  }
-
   // All processes spawned by the fork'd/execute'd process will see this process
   // as /sbin/init. Therefore it will receive (and ignore) their final status
   // (see the next comment as well). PR_SET_CHILD_SUBREAPER is available since

sandboxed_api/sandbox2/forkserver_test.cc

@@ -59,7 +59,7 @@ pid_t TestSingleRequest(Mode mode, int exec_fd, int userns_fd) {
   fork_req.add_envs("FOO=1");
 
   pid_t pid =
-      GetGlobalForkClient()->SendRequest(fork_req, exec_fd, sv[0], userns_fd);
+      GlobalForkClient::SendRequest(fork_req, exec_fd, sv[0], userns_fd);
   if (pid != -1) {
     VLOG(1) << "TestSingleRequest: Waiting for pid=" << pid;
     waitpid(pid, nullptr, 0);

sandboxed_api/sandbox2/global_forkclient.cc

@@ -25,92 +25,99 @@
 #include <csignal>
 #include <cstdlib>
 
-#include "absl/base/attributes.h"
-#include "absl/strings/str_cat.h"
+#include <glog/logging.h>
+#include "sandboxed_api/util/flag.h"
+#include "absl/memory/memory.h"
 #include "sandboxed_api/embed_file.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/forkserver_bin_embed.h"
-#include "sandboxed_api/sandbox2/sanitizer.h"
+#include "sandboxed_api/sandbox2/util.h"
+#include "sandboxed_api/sandbox2/util/fileops.h"
 #include "sandboxed_api/sandbox2/util/strerror.h"
 #include "sandboxed_api/util/raw_logging.h"
 
+ABSL_FLAG(bool, sandbox2_start_forkserver, true,
+          "Start Sandbox2 Forkserver process");
+
 namespace sandbox2 {
 
-// Global ForkClient object linking with the global ForkServer.
-static ForkClient* global_fork_client = nullptr;
-static pid_t global_fork_server_pid = -1;
+namespace {
 
-ForkClient* GetGlobalForkClient() {
-  SAPI_RAW_CHECK(global_fork_client != nullptr,
-                 "global fork client not initialized");
-  return global_fork_client;
-}
-
-pid_t GetGlobalForkServerPid() { return global_fork_server_pid; }
-
-static void StartGlobalForkServer() {
-  SAPI_RAW_CHECK(global_fork_client == nullptr,
-                 "global fork server already initialized");
+std::unique_ptr<GlobalForkClient> StartGlobalForkServer() {
   if (getenv(kForkServerDisableEnv)) {
     SAPI_RAW_VLOG(1,
                   "Start of the Global Fork-Server prevented by the '%s' "
                   "environment variable present",
                   kForkServerDisableEnv);
-    return;
+    return {};
   }
 
-  sanitizer::WaitForTsan();
-
-  // We should be really single-threaded now, as it's the point of the whole
-  // exercise.
-  int num_threads = sanitizer::GetNumberOfThreads(getpid());
-  if (num_threads != 1) {
-    SAPI_RAW_LOG(ERROR,
-                 "BADNESS MAY HAPPEN. ForkServer::Init() created in a "
-                 "multi-threaded context, %d threads present",
-                 num_threads);
+  if (!absl::GetFlag(FLAGS_sandbox2_start_forkserver)) {
+    SAPI_RAW_VLOG(
+        1, "Start of the Global Fork-Server prevented by commandline flag");
+    return {};
   }
 
+  file_util::fileops::FDCloser exec_fd(
+      sapi::EmbedFile::GetEmbedFileSingleton()->GetFdForFileToc(
+          forkserver_bin_embed_create()));
+  SAPI_RAW_CHECK(exec_fd.get() >= 0, "Getting FD for init binary failed");
+
+  std::string proc_name = "S2-FORK-SERV";
+
   int sv[2];
   SAPI_RAW_CHECK(socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, sv) != -1,
                  "creating socket pair");
 
   // Fork the fork-server, and clean-up the resources (close remote sockets).
-  pid_t pid;
-  {
-    pid = fork();
-  }
+  pid_t pid = util::ForkWithFlags(SIGCHLD);
   SAPI_RAW_PCHECK(pid != -1, "during fork");
 
-  // Parent.
-  if (pid > 0) {
-    close(sv[0]);
-    global_fork_client = new ForkClient{new Comms{sv[1]}};
-    global_fork_server_pid = pid;
-    return;
+  // Child.
+  if (pid == 0) {
+    // Move the comms FD to the proper, expected FD number.
+    // The new FD will not be CLOEXEC, which is what we want.
+    dup2(sv[0], Comms::kSandbox2ClientCommsFD);
+
+    char* const args[] = {proc_name.data(), nullptr};
+    char* const envp[] = {nullptr};
+    syscall(__NR_execveat, exec_fd.get(), "", args, envp, AT_EMPTY_PATH);
+    SAPI_RAW_PLOG(FATAL, "Could not launch forkserver binary");
+    abort();
   }
 
-  // Move the comms FD to the proper, expected FD number.
-  // The new FD will not be CLOEXEC, which is what we want.
-  dup2(sv[0], Comms::kSandbox2ClientCommsFD);
-
-  int exec_fd = sapi::EmbedFile::GetEmbedFileSingleton()->GetFdForFileToc(
-      forkserver_bin_embed_create());
-  SAPI_RAW_CHECK(exec_fd >= 0, "Getting FD for init binary failed");
-
-  char* const args[] = {strdup("S2-FORK-SERV"), nullptr};
-  char* const envp[] = {nullptr};
-  syscall(__NR_execveat, exec_fd, "", args, envp, AT_EMPTY_PATH);
-  SAPI_RAW_PCHECK(false, "Could not launch forkserver binary");
+  close(sv[0]);
+  return absl::make_unique<GlobalForkClient>(sv[1], pid);
 }
 
+GlobalForkClient* GetGlobalForkClient() {
+  static GlobalForkClient* global_fork_client =
+      StartGlobalForkServer().release();
+  return global_fork_client;
+}
+
+}  // namespace
+
+void GlobalForkClient::EnsureStarted() { GetGlobalForkClient(); }
+
+pid_t GlobalForkClient::SendRequest(const ForkRequest& request, int exec_fd,
+                                    int comms_fd, int user_ns_fd,
+                                    pid_t* init_pid) {
+  GlobalForkClient* global_fork_client = GetGlobalForkClient();
+  SAPI_RAW_CHECK(global_fork_client != nullptr,
+                 "global fork client not initialized");
+  pid_t pid = global_fork_client->fork_client_.SendRequest(
+      request, exec_fd, comms_fd, user_ns_fd, init_pid);
+  if (global_fork_client->comms_.IsTerminated()) {
+    LOG(ERROR) << "Global forkserver connection terminated";
+  }
+  return pid;
+}
+
+pid_t GlobalForkClient::GetPid() {
+  GlobalForkClient* global_fork_client = GetGlobalForkClient();
+  SAPI_RAW_CHECK(global_fork_client != nullptr,
+                 "global fork client not initialized");
+  return global_fork_client->fork_client_.pid();
+}
 }  // namespace sandbox2
-
-// Run the ForkServer from the constructor, when no other threads are present.
-// Because it's possible to start thread-inducing initializers before
-// RunInitializers() (base/googleinit.h) it's not enough to just register
-// a 0000_<name> initializer instead.
-ABSL_ATTRIBUTE_UNUSED
-__attribute__((constructor)) static void StartSandbox2Forkserver() {
-  sandbox2::StartGlobalForkServer();
-}

sandboxed_api/sandbox2/global_forkclient.h

@@ -20,14 +20,28 @@
 
 #include <sys/types.h>
 
+#include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/fork_client.h"
+#include "sandboxed_api/sandbox2/forkserver.pb.h"
 
 namespace sandbox2 {
 
-ForkClient* GetGlobalForkClient();
+class GlobalForkClient {
+ public:
+  GlobalForkClient(int fd, pid_t pid)
+      : comms_(fd), fork_client_(pid, &comms_) {}
 
-// Returns the fork server PID. Used in tests.
-pid_t GetGlobalForkServerPid();
+  static pid_t SendRequest(const ForkRequest& request, int exec_fd,
+                           int comms_fd, int user_ns_fd = -1,
+                           pid_t* init_pid = nullptr);
+  static pid_t GetPid();
+
+  static void EnsureStarted();
+
+ private:
+  Comms comms_;
+  ForkClient fork_client_;
+};
 
 }  // namespace sandbox2
 

sandboxed_api/sandbox2/global_forkclient_lib_ctor.cc

@@ -0,0 +1,25 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "absl/base/attributes.h"
+#include "sandboxed_api/sandbox2/global_forkclient.h"
+
+// Run the ForkServer from the constructor, when no other threads are present.
+// Because it's possible to start thread-inducing initializers before
+// RunInitializers() (base/googleinit.h) it's not enough to just register
+// a 0000_<name> initializer instead.
+ABSL_ATTRIBUTE_UNUSED
+__attribute__((constructor)) static void StartSandbox2Forkserver() {
+  sandbox2::GlobalForkClient::EnsureStarted();
+}

sandboxed_api/sandbox2/stack_trace_test.cc

@@ -158,7 +158,7 @@ static size_t FileCountInDirectory(const std::string& path) {
 TEST(StackTraceTest, ForkEnterNsLibunwindDoesNotLeakFDs) {
   SKIP_SANITIZERS_AND_COVERAGE;
   // Get list of open FDs in the global forkserver.
-  pid_t forkserver_pid = GetGlobalForkServerPid();
+  pid_t forkserver_pid = GlobalForkClient::GetPid();
   std::string forkserver_fd_path =
       absl::StrCat("/proc/", forkserver_pid, "/fd");
   size_t filecount_before = FileCountInDirectory(forkserver_fd_path);