Allow Linux Seccomp-BPF sandbox to start with multiple threads

Adds an |allow_threads_before_starting_sandbox| option to the
Linux sandbox, which enables TSYNC if there are already
multiple threads in the process.

Also adds |check_for_open_directories| option to the Linux
sandbox, which can be set to false to skip the check for
open directories, which is only relevant if we are engaging
the semantic layer of the sandbox, which we don't for the
GPU sandbox. And if we start the GPU sandbox with multiple
threads, and the above option will allow, then those other
threads may have opened directories and we don't want that
to be a failure.

Based on jorgelo@'s CL:
chromium-review.googlesource.com/c/chromium/src/+/1496305

Bug: 924759, 996455
Change-Id: Icee663f87d396f97ad7bb257c2709c15b06fab33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829866
Commit-Queue: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Julien Isorce <julien.isorce@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704348}

services/service_manager/sandbox/linux/sandbox_linux.cc

@@ -293,12 +293,18 @@ bool SandboxLinux::StartSeccompBPF(SandboxType sandbox_type,
   if (hook)
     CHECK(std::move(hook).Run(options));
 
+  // If we allow threads *and* have multiple threads, try to use TSYNC.
+  sandbox::SandboxBPF::SeccompLevel seccomp_level =
+      options.allow_threads_during_sandbox_init && !IsSingleThreaded()
+          ? sandbox::SandboxBPF::SeccompLevel::MULTI_THREADED
+          : sandbox::SandboxBPF::SeccompLevel::SINGLE_THREADED;
+
   // If the kernel supports the sandbox, and if the command line says we
   // should enable it, enable it or die.
   std::unique_ptr<BPFBasePolicy> policy =
       SandboxSeccompBPF::PolicyForSandboxType(sandbox_type, options);
-  SandboxSeccompBPF::StartSandboxWithExternalPolicy(std::move(policy),
-                                                    OpenProc(proc_fd_));
+  SandboxSeccompBPF::StartSandboxWithExternalPolicy(
+      std::move(policy), OpenProc(proc_fd_), seccomp_level);
   SandboxSeccompBPF::RunSandboxSanityChecks(sandbox_type, options);
   seccomp_bpf_started_ = true;
   LogSandboxStarted("seccomp-bpf");
@@ -329,9 +335,13 @@ bool SandboxLinux::InitializeSandbox(SandboxType sandbox_type,
       base::BindOnce(&SandboxLinux::CheckForBrokenPromises,
                      base::Unretained(this), sandbox_type));
 
-  // No matter what, it's always an error to call InitializeSandbox() after
-  // threads have been created.
-  if (!IsSingleThreaded()) {
+  const bool has_threads = !IsSingleThreaded();
+
+  // For now, restrict the |options.allow_threads_during_sandbox_init| option to
+  // the GPU process
+  DCHECK(process_type == switches::kGpuProcess ||
+         !options.allow_threads_during_sandbox_init);
+  if (has_threads && !options.allow_threads_during_sandbox_init) {
     std::string error_message =
         "InitializeSandbox() called with multiple threads in process " +
         process_type + ".";
@@ -340,13 +350,6 @@ bool SandboxLinux::InitializeSandbox(SandboxType sandbox_type,
     if (IsRunningTSAN())
       return false;
 
-#if defined(OS_CHROMEOS)
-    if (base::SysInfo::IsRunningOnChromeOS() &&
-        process_type == switches::kGpuProcess) {
-      error_message += " This error can be safely ignored in VMTests.";
-    }
-#endif
-
     // The GPU process is allowed to call InitializeSandbox() with threads.
     bool sandbox_failure_fatal = process_type != switches::kGpuProcess;
     // This can be disabled with the '--gpu-sandbox-failures-fatal' flag.
@@ -371,21 +374,31 @@ bool SandboxLinux::InitializeSandbox(SandboxType sandbox_type,
     }
   }
 
-  // Only one thread is running, pre-initialize if not already done.
+  // At this point we are either single threaded, or we won't be engaging the
+  // semantic layer of the sandbox and we won't care if there are file
+  // descriptors left open.
+
+  // Pre-initialize if not already done.
   if (!pre_initialized_)
     PreinitializeSandbox();
 
-  // Turn on the namespace sandbox if the zygote hasn't done so already.
+  // Turn on the namespace sandbox if our caller wants it (and the zygote hasn't
+  // done so already).
   if (options.engage_namespace_sandbox)
     EngageNamespaceSandbox(false /* from_zygote */);
 
-  CHECK(!HasOpenDirectories())
+  // Check for open directories, which can break the semantic sandbox layer. In
+  // some cases the caller doesn't want to enable the semantic sandbox layer,
+  // and this CHECK should be skipped. In this case, the caller should unset
+  // |options.check_for_open_directories|.
+  CHECK(!options.check_for_open_directories || !HasOpenDirectories())
       << "InitializeSandbox() called after unexpected directories have been "
       << "opened. This breaks the security of the setuid sandbox.";
 
   sandbox::InitLibcLocaltimeFunctions();
 
   // Attempt to limit the future size of the address space of the process.
+  // Fine to call with multiple threads as we don't use RLIMIT_STACK.
   int error = 0;
   const bool limited_as = LimitAddressSpace(&error);
   if (error) {
@@ -448,6 +461,7 @@ bool SandboxLinux::LimitAddressSpace(int* error) {
   // other memory-hungry attack modes.
 
   rlim_t process_data_size_limit = GetProcessDataSizeLimit(sandbox_type);
+  // Fine to call with multiple threads as we don't use RLIMIT_STACK.
   *error = sandbox::ResourceLimits::Lower(RLIMIT_DATA, process_data_size_limit);
 
   // Cache the resource limit before turning on the sandbox.
@@ -510,6 +524,11 @@ void SandboxLinux::StopThreadAndEnsureNotCounted(base::Thread* thread) const {
 
 bool SandboxLinux::EngageNamespaceSandboxInternal(bool from_zygote) {
   CHECK(pre_initialized_);
+  CHECK(IsSingleThreaded())
+      << "The process cannot have multiple threads when engaging the namespace "
+         "sandbox, because the thread engaging the sandbox cannot ensure that "
+         "other threads close all their open directories.";
+
   if (from_zygote) {
     // Check being in a new PID namespace created by the namespace sandbox and
     // being the init process.

services/service_manager/sandbox/linux/sandbox_linux.h

@@ -99,6 +99,16 @@ class SERVICE_MANAGER_SANDBOX_EXPORT SandboxLinux {
     // be done so again. Set to true to indicate that there isn't a zygote
     // for this process and the step is to be performed here explicitly.
     bool engage_namespace_sandbox = false;
+
+    // Allow starting the sandbox with multiple threads already running. This
+    // will enable TSYNC for seccomp-BPF, which syncs the seccomp-BPF policy
+    // across all running threads.
+    bool allow_threads_during_sandbox_init = false;
+
+    // Enables the CHECK for open directories. The open directory check is only
+    // useful for the chroot jail (from the semantic layer of the sandbox), and
+    // can safely be disabled if we are only enabling the seccomp-BPF layer.
+    bool check_for_open_directories = true;
   };
 
   // Callers can provide this hook to run code right before the policy
@@ -176,9 +186,11 @@ class SERVICE_MANAGER_SANDBOX_EXPORT SandboxLinux {
   // be used directly.
   sandbox::SetuidSandboxClient* setuid_sandbox_client() const;
 
-  // Check the policy and eventually start the seccomp-bpf sandbox. This should
-  // never be called with threads started. If we detect that threads have
-  // started we will crash.
+  // Check the policy and eventually start the seccomp-bpf sandbox. Fine to be
+  // called with threads, as long as
+  // |options.allow_threads_during_sandbox_init| is true and the kernel
+  // supports seccomp's TSYNC feature. If TSYNC is not available we treat
+  // multiple threads as a fatal error.
   bool StartSeccompBPF(service_manager::SandboxType sandbox_type,
                        PreSandboxHook hook,
                        const Options& options);

services/service_manager/sandbox/linux/sandbox_seccomp_bpf_linux.cc

@@ -225,7 +225,8 @@ void SandboxSeccompBPF::RunSandboxSanityChecks(
 
 bool SandboxSeccompBPF::StartSandboxWithExternalPolicy(
     std::unique_ptr<sandbox::bpf_dsl::Policy> policy,
-    base::ScopedFD proc_fd) {
+    base::ScopedFD proc_fd,
+    sandbox::SandboxBPF::SeccompLevel seccomp_level) {
 #if BUILDFLAG(USE_SECCOMP_BPF)
   if (IsSeccompBPFDesired() && SupportsSandbox()) {
     CHECK(policy);
@@ -236,7 +237,7 @@ bool SandboxSeccompBPF::StartSandboxWithExternalPolicy(
     // doing so does not stop the sandbox.
     SandboxBPF sandbox(std::move(policy));
     sandbox.SetProcFd(std::move(proc_fd));
-    CHECK(sandbox.StartSandbox(SandboxBPF::SeccompLevel::SINGLE_THREADED));
+    CHECK(sandbox.StartSandbox(seccomp_level));
     return true;
   }
 #endif  // BUILDFLAG(USE_SECCOMP_BPF)

services/service_manager/sandbox/linux/sandbox_seccomp_bpf_linux.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "sandbox/linux/bpf_dsl/policy.h"
+#include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
 #include "services/service_manager/sandbox/export.h"
 #include "services/service_manager/sandbox/linux/bpf_base_policy_linux.h"
 #include "services/service_manager/sandbox/sandbox_type.h"
@@ -60,7 +61,9 @@ class SERVICE_MANAGER_SANDBOX_EXPORT SandboxSeccompBPF {
   // external policy.
   static bool StartSandboxWithExternalPolicy(
       std::unique_ptr<sandbox::bpf_dsl::Policy> policy,
-      base::ScopedFD proc_fd);
+      base::ScopedFD proc_fd,
+      sandbox::SandboxBPF::SeccompLevel seccomp_level =
+          sandbox::SandboxBPF::SeccompLevel::SINGLE_THREADED);
 
   // The "baseline" policy can be a useful base to build a sandbox policy.
   static std::unique_ptr<sandbox::bpf_dsl::Policy> GetBaselinePolicy();