Linux sandbox: better APIs with /proc/ arguments

Unify sandbox:: APIs to always take /proc/ file descriptors
instead of /proc/self/ or /proc/self/task/.

Moreover, require |proc_fd| arguments to critical APIs rather
than rely on the caller to perform the right checks.

A descriptor to /proc is a better choice than a descriptor to
/proc/self/* because it keeps the same semantics after a fork().

BUG=312380, 457377
TBR=nasko

Review URL: https://codereview.chromium.org/938223004

Cr-Commit-Position: refs/heads/master@{#317757}

components/nacl/loader/nonsfi/nonsfi_sandbox.cc

@@ -300,11 +300,11 @@ ResultExpr NaClNonSfiBPFSandboxPolicy::InvalidSyscall() const {
   return CrashSIGSYS();
 }
 
-bool InitializeBPFSandbox(base::ScopedFD proc_task_fd) {
+bool InitializeBPFSandbox(base::ScopedFD proc_fd) {
   bool sandbox_is_initialized = content::InitializeSandbox(
       scoped_ptr<sandbox::bpf_dsl::Policy>(
           new nacl::nonsfi::NaClNonSfiBPFSandboxPolicy()),
-      proc_task_fd.Pass());
+      proc_fd.Pass());
   if (!sandbox_is_initialized)
     return false;
   RunSandboxSanityChecks();

components/nacl/loader/nonsfi/nonsfi_sandbox.h

@@ -30,7 +30,7 @@ class NaClNonSfiBPFSandboxPolicy : public sandbox::bpf_dsl::Policy {
 
 // Initializes seccomp-bpf sandbox for non-SFI NaCl. Returns false on
 // failure.
-bool InitializeBPFSandbox(base::ScopedFD proc_task_fd);
+bool InitializeBPFSandbox(base::ScopedFD proc_fd);
 
 }  // namespace nonsfi
 }  // namespace nacl

components/nacl/loader/sandbox_linux/nacl_bpf_sandbox_linux.cc

@@ -162,11 +162,11 @@ void RunSandboxSanityChecks() {
 
 #endif  // defined(USE_SECCOMP_BPF)
 
-bool InitializeBPFSandbox(base::ScopedFD proc_task_fd) {
+bool InitializeBPFSandbox(base::ScopedFD proc_fd) {
 #if defined(USE_SECCOMP_BPF)
   bool sandbox_is_initialized = content::InitializeSandbox(
       scoped_ptr<sandbox::bpf_dsl::Policy>(new NaClBPFSandboxPolicy),
-      proc_task_fd.Pass());
+      proc_fd.Pass());
   if (sandbox_is_initialized) {
     RunSandboxSanityChecks();
     return true;

components/nacl/loader/sandbox_linux/nacl_bpf_sandbox_linux.h

@@ -9,7 +9,7 @@
 
 namespace nacl {
 
-bool InitializeBPFSandbox(base::ScopedFD proc_task_fd);
+bool InitializeBPFSandbox(base::ScopedFD proc_fd);
 
 }  // namespace nacl
 

components/nacl/loader/sandbox_linux/nacl_sandbox_linux.cc

@@ -48,15 +48,6 @@ bool IsSandboxed() {
   return true;
 }
 
-// Open a new file descriptor to /proc/self/task/ by using
-// |proc_fd|.
-base::ScopedFD GetProcSelfTask(int proc_fd) {
-  base::ScopedFD proc_self_task(HANDLE_EINTR(
-      openat(proc_fd, "self/task/", O_RDONLY | O_DIRECTORY | O_CLOEXEC)));
-  PCHECK(proc_self_task.is_valid());
-  return proc_self_task.Pass();
-}
-
 bool MaybeSetProcessNonDumpable() {
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
@@ -122,8 +113,7 @@ NaClSandbox::~NaClSandbox() {
 
 bool NaClSandbox::IsSingleThreaded() {
   CHECK(proc_fd_.is_valid());
-  base::ScopedFD proc_self_task(GetProcSelfTask(proc_fd_.get()));
-  return sandbox::ThreadHelpers::IsSingleThreaded(proc_self_task.get());
+  return sandbox::ThreadHelpers::IsSingleThreaded(proc_fd_.get());
 }
 
 bool NaClSandbox::HasOpenDirectory() {
@@ -149,11 +139,10 @@ void NaClSandbox::InitializeLayerOneSandbox() {
     layer_one_enabled_ = true;
   } else if (sandbox::NamespaceSandbox::InNewUserNamespace()) {
     CHECK(sandbox::Credentials::MoveToNewUserNS());
-    // This relies on SealLayerOneSandbox() to be called later.
-    CHECK(!HasOpenDirectory());
-    CHECK(sandbox::Credentials::DropFileSystemAccess());
-    CHECK(IsSingleThreaded());
-    CHECK(sandbox::Credentials::DropAllCapabilities());
+    // This relies on SealLayerOneSandbox() to be called later since this
+    // class is keeping a file descriptor to /proc/.
+    CHECK(sandbox::Credentials::DropFileSystemAccess(proc_fd_.get()));
+    CHECK(sandbox::Credentials::DropAllCapabilities(proc_fd_.get()));
     CHECK(IsSandboxed());
     layer_one_enabled_ = true;
   }
@@ -189,19 +178,19 @@ void NaClSandbox::InitializeLayerTwoSandbox(bool uses_nonsfi_mode) {
 
   RestrictAddressSpaceUsage();
 
-  base::ScopedFD proc_self_task(GetProcSelfTask(proc_fd_.get()));
-
+  // Pass proc_fd_ ownership to the BPF sandbox, which guarantees it will
+  // be closed. There is no point in keeping it around since the BPF policy
+  // will prevent its usage.
   if (uses_nonsfi_mode) {
-    layer_two_enabled_ =
-        nacl::nonsfi::InitializeBPFSandbox(proc_self_task.Pass());
+    layer_two_enabled_ = nacl::nonsfi::InitializeBPFSandbox(proc_fd_.Pass());
     layer_two_is_nonsfi_ = true;
   } else {
-    layer_two_enabled_ = nacl::InitializeBPFSandbox(proc_self_task.Pass());
+    layer_two_enabled_ = nacl::InitializeBPFSandbox(proc_fd_.Pass());
   }
 }
 
 void NaClSandbox::SealLayerOneSandbox() {
-  if (!layer_two_enabled_) {
+  if (proc_fd_.is_valid() && !layer_two_enabled_) {
     // If nothing prevents us, check that there is no superfluous directory
     // open.
     CHECK(!HasOpenDirectory());

content/common/sandbox_linux/sandbox_init_linux.cc

@@ -12,9 +12,9 @@
 namespace content {
 
 bool InitializeSandbox(scoped_ptr<sandbox::bpf_dsl::Policy> policy,
-                       base::ScopedFD proc_task_fd) {
+                       base::ScopedFD proc_fd) {
   return SandboxSeccompBPF::StartSandboxWithExternalPolicy(policy.Pass(),
-                                                           proc_task_fd.Pass());
+                                                           proc_fd.Pass());
 }
 
 scoped_ptr<sandbox::bpf_dsl::Policy> GetBPFSandboxBaselinePolicy() {

content/common/sandbox_linux/sandbox_linux.cc

@@ -76,22 +76,23 @@ bool IsRunningTSAN() {
 #endif
 }
 
-// Try to open /proc/self/task/ with the help of |proc_fd|. |proc_fd| can be
-// -1. Will return -1 on error and set errno like open(2).
+// Get a file descriptor to /proc. Either duplicate |proc_fd| or try to open
+// it by using the filesystem directly.
 // TODO(jln): get rid of this ugly interface.
-int OpenProcTaskFd(int proc_fd) {
-  int proc_self_task = -1;
+base::ScopedFD OpenProc(int proc_fd) {
+  int ret_proc_fd = -1;
   if (proc_fd >= 0) {
     // If a handle to /proc is available, use it. This allows to bypass file
     // system restrictions.
-    proc_self_task = HANDLE_EINTR(
-        openat(proc_fd, "self/task/", O_RDONLY | O_DIRECTORY | O_CLOEXEC));
+    ret_proc_fd =
+        HANDLE_EINTR(openat(proc_fd, ".", O_RDONLY | O_DIRECTORY | O_CLOEXEC));
   } else {
     // Otherwise, make an attempt to access the file system directly.
-    proc_self_task = HANDLE_EINTR(openat(AT_FDCWD, "/proc/self/task/",
-                                         O_RDONLY | O_DIRECTORY | O_CLOEXEC));
+    ret_proc_fd = HANDLE_EINTR(
+        openat(AT_FDCWD, "/proc/", O_RDONLY | O_DIRECTORY | O_CLOEXEC));
   }
-  return proc_self_task;
+  DCHECK_LE(0, ret_proc_fd);
+  return base::ScopedFD(ret_proc_fd);
 }
 
 }  // namespace
@@ -183,11 +184,9 @@ void LinuxSandbox::EngageNamespaceSandbox() {
 
   CHECK(sandbox::Credentials::MoveToNewUserNS());
   // Note: this requires SealSandbox() to be called later in this process to be
-  // safe, as this class is keeping a file descriptor to /proc.
-  CHECK(!HasOpenDirectories());
-  CHECK(sandbox::Credentials::DropFileSystemAccess());
-  CHECK(IsSingleThreaded());
-  CHECK(sandbox::Credentials::DropAllCapabilities());
+  // safe, as this class is keeping a file descriptor to /proc/.
+  CHECK(sandbox::Credentials::DropFileSystemAccess(proc_fd_));
+  CHECK(sandbox::Credentials::DropAllCapabilities(proc_fd_));
 
   // This needs to happen after moving to a new user NS, since doing so involves
   // writing the UID/GID map.
@@ -257,14 +256,13 @@ int LinuxSandbox::GetStatus() {
 // PID namespaces and existing sandboxes, so "self" must really be used instead
 // of using the pid.
 bool LinuxSandbox::IsSingleThreaded() const {
-  base::ScopedFD proc_self_task(OpenProcTaskFd(proc_fd_));
+  base::ScopedFD proc_fd(OpenProc(proc_fd_));
 
-  CHECK(proc_self_task.is_valid())
-      << "Could not count threads, the sandbox was not "
+  CHECK(proc_fd.is_valid()) << "Could not count threads, the sandbox was not "
                             << "pre-initialized properly.";
 
   const bool is_single_threaded =
-      sandbox::ThreadHelpers::IsSingleThreaded(proc_self_task.get());
+      sandbox::ThreadHelpers::IsSingleThreaded(proc_fd.get());
 
   return is_single_threaded;
 }
@@ -283,9 +281,8 @@ bool LinuxSandbox::StartSeccompBPF(const std::string& process_type) {
   CHECK(!seccomp_bpf_started_);
   CHECK(pre_initialized_);
   if (seccomp_bpf_supported()) {
-    base::ScopedFD proc_self_task(OpenProcTaskFd(proc_fd_));
     seccomp_bpf_started_ =
-        SandboxSeccompBPF::StartSandbox(process_type, proc_self_task.Pass());
+        SandboxSeccompBPF::StartSandbox(process_type, OpenProc(proc_fd_));
   }
 
   if (seccomp_bpf_started_) {
@@ -452,10 +449,10 @@ void LinuxSandbox::CheckForBrokenPromises(const std::string& process_type) {
 
 void LinuxSandbox::StopThreadAndEnsureNotCounted(base::Thread* thread) const {
   DCHECK(thread);
-  base::ScopedFD proc_self_task(OpenProcTaskFd(proc_fd_));
-  PCHECK(proc_self_task.is_valid());
-  CHECK(sandbox::ThreadHelpers::StopThreadAndWatchProcFS(proc_self_task.get(),
-                                                         thread));
+  base::ScopedFD proc_fd(OpenProc(proc_fd_));
+  PCHECK(proc_fd.is_valid());
+  CHECK(
+      sandbox::ThreadHelpers::StopThreadAndWatchProcFS(proc_fd.get(), thread));
 }
 
 }  // namespace content

content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc

@@ -62,7 +62,7 @@ namespace content {
 namespace {
 
 void StartSandboxWithPolicy(sandbox::bpf_dsl::Policy* policy,
-                            base::ScopedFD proc_task_fd);
+                            base::ScopedFD proc_fd);
 
 inline bool IsChromeOS() {
 #if defined(OS_CHROMEOS)
@@ -148,7 +148,7 @@ void RunSandboxSanityChecks(const std::string& process_type) {
 
 // This function takes ownership of |policy|.
 void StartSandboxWithPolicy(sandbox::bpf_dsl::Policy* policy,
-                            base::ScopedFD proc_task_fd) {
+                            base::ScopedFD proc_fd) {
   // Starting the sandbox is a one-way operation. The kernel doesn't allow
   // us to unload a sandbox policy after it has been started. Nonetheless,
   // in order to make the use of the "Sandbox" object easier, we allow for
@@ -156,7 +156,7 @@ void StartSandboxWithPolicy(sandbox::bpf_dsl::Policy* policy,
   // doing so does not stop the sandbox.
   SandboxBPF sandbox(policy);
 
-  sandbox.SetProcTaskFd(proc_task_fd.Pass());
+  sandbox.SetProcFd(proc_fd.Pass());
   CHECK(sandbox.StartSandbox(SandboxBPF::SeccompLevel::SINGLE_THREADED));
 }
 
@@ -187,7 +187,7 @@ scoped_ptr<SandboxBPFBasePolicy> GetGpuProcessSandbox() {
 // Initialize the seccomp-bpf sandbox.
 bool StartBPFSandbox(const base::CommandLine& command_line,
                      const std::string& process_type,
-                     base::ScopedFD proc_task_fd) {
+                     base::ScopedFD proc_fd) {
   scoped_ptr<SandboxBPFBasePolicy> policy;
 
   if (process_type == switches::kGpuProcess) {
@@ -204,7 +204,7 @@ bool StartBPFSandbox(const base::CommandLine& command_line,
   }
 
   CHECK(policy->PreSandboxHook());
-  StartSandboxWithPolicy(policy.release(), proc_task_fd.Pass());
+  StartSandboxWithPolicy(policy.release(), proc_fd.Pass());
 
   RunSandboxSanityChecks(process_type);
   return true;
@@ -267,7 +267,7 @@ bool SandboxSeccompBPF::SupportsSandboxWithTsync() {
 }
 
 bool SandboxSeccompBPF::StartSandbox(const std::string& process_type,
-                                     base::ScopedFD proc_task_fd) {
+                                     base::ScopedFD proc_fd) {
 #if defined(USE_SECCOMP_BPF)
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
@@ -278,7 +278,7 @@ bool SandboxSeccompBPF::StartSandbox(const std::string& process_type,
     // If the kernel supports the sandbox, and if the command line says we
     // should enable it, enable it or die.
     bool started_sandbox =
-        StartBPFSandbox(command_line, process_type, proc_task_fd.Pass());
+        StartBPFSandbox(command_line, process_type, proc_fd.Pass());
     CHECK(started_sandbox);
     return true;
   }
@@ -288,11 +288,11 @@ bool SandboxSeccompBPF::StartSandbox(const std::string& process_type,
 
 bool SandboxSeccompBPF::StartSandboxWithExternalPolicy(
     scoped_ptr<sandbox::bpf_dsl::Policy> policy,
-    base::ScopedFD proc_task_fd) {
+    base::ScopedFD proc_fd) {
 #if defined(USE_SECCOMP_BPF)
   if (IsSeccompBPFDesired() && SupportsSandbox()) {
     CHECK(policy);
-    StartSandboxWithPolicy(policy.release(), proc_task_fd.Pass());
+    StartSandboxWithPolicy(policy.release(), proc_fd.Pass());
     return true;
   }
 #endif  // defined(USE_SECCOMP_BPF)

content/common/sandbox_linux/sandbox_seccomp_bpf_linux.h

@@ -34,13 +34,13 @@ class SandboxSeccompBPF {
   // Start the sandbox and apply the policy for process_type, depending on
   // command line switches.
   static bool StartSandbox(const std::string& process_type,
-                           base::ScopedFD proc_task_fd);
+                           base::ScopedFD proc_fd);
 
   // This is the API to enable a seccomp-bpf sandbox by using an
   // external policy.
   static bool StartSandboxWithExternalPolicy(
       scoped_ptr<sandbox::bpf_dsl::Policy> policy,
-      base::ScopedFD proc_task_fd);
+      base::ScopedFD proc_fd);
   // The "baseline" policy can be a useful base to build a sandbox policy.
   static scoped_ptr<sandbox::bpf_dsl::Policy> GetBaselinePolicy();
 

content/public/common/sandbox_init.h

@@ -88,11 +88,11 @@ class SandboxInitializerDelegate;
 
 // Initialize a seccomp-bpf sandbox. |policy| may not be NULL.
 // If an existing layer of sandboxing is present that would prevent access to
-// /proc, |proc_task_fd| must be a valid file descriptor to /proc/self/task.
+// /proc, |proc_fd| must be a valid file descriptor to /proc/.
 // Returns true if the sandbox has been properly engaged.
 CONTENT_EXPORT bool InitializeSandbox(
     scoped_ptr<sandbox::bpf_dsl::Policy> policy,
-    base::ScopedFD proc_task_fd);
+    base::ScopedFD proc_fd);
 
 // Return a "baseline" policy. This is used by a SandboxInitializerDelegate to
 // implement a policy that is derived from the baseline.

sandbox/linux/seccomp-bpf/sandbox_bpf.cc

@@ -46,8 +46,8 @@ namespace {
 
 bool IsRunningOnValgrind() { return RUNNING_ON_VALGRIND; }
 
-bool IsSingleThreaded(int proc_task_fd) {
-  return ThreadHelpers::IsSingleThreaded(proc_task_fd);
+bool IsSingleThreaded(int proc_fd) {
+  return ThreadHelpers::IsSingleThreaded(proc_fd);
 }
 
 // Check if the kernel supports seccomp-filter (a.k.a. seccomp mode 2) via
@@ -82,7 +82,7 @@ bool KernelSupportsSeccompTsync() {
 }  // namespace
 
 SandboxBPF::SandboxBPF(bpf_dsl::Policy* policy)
-    : proc_task_fd_(), sandbox_has_started_(false), policy_(policy) {
+    : proc_fd_(), sandbox_has_started_(false), policy_(policy) {
 }
 
 SandboxBPF::~SandboxBPF() {
@@ -118,8 +118,8 @@ bool SandboxBPF::StartSandbox(SeccompLevel seccomp_level) {
     return false;
   }
 
-  if (!proc_task_fd_.is_valid()) {
-    SetProcTaskFd(ProcUtil::OpenProcSelfTask());
+  if (!proc_fd_.is_valid()) {
+    SetProcFd(ProcUtil::OpenProc());
   }
 
   const bool supports_tsync = KernelSupportsSeccompTsync();
@@ -127,9 +127,9 @@ bool SandboxBPF::StartSandbox(SeccompLevel seccomp_level) {
   if (seccomp_level == SeccompLevel::SINGLE_THREADED) {
     // Wait for /proc/self/task/ to update if needed and assert the
     // process is single threaded.
-    ThreadHelpers::AssertSingleThreaded(proc_task_fd_.get());
+    ThreadHelpers::AssertSingleThreaded(proc_fd_.get());
   } else if (seccomp_level == SeccompLevel::MULTI_THREADED) {
-    if (IsSingleThreaded(proc_task_fd_.get())) {
+    if (IsSingleThreaded(proc_fd_.get())) {
       SANDBOX_DIE("Cannot start sandbox; "
                   "process may be single-threaded when reported as not");
       return false;
@@ -144,8 +144,8 @@ bool SandboxBPF::StartSandbox(SeccompLevel seccomp_level) {
   // We no longer need access to any files in /proc. We want to do this
   // before installing the filters, just in case that our policy denies
   // close().
-  if (proc_task_fd_.is_valid()) {
-    proc_task_fd_.reset();
+  if (proc_fd_.is_valid()) {
+    proc_fd_.reset();
   }
 
   // Install the filters.
@@ -155,8 +155,8 @@ bool SandboxBPF::StartSandbox(SeccompLevel seccomp_level) {
   return true;
 }
 
-void SandboxBPF::SetProcTaskFd(base::ScopedFD proc_task_fd) {
-  proc_task_fd_.swap(proc_task_fd);
+void SandboxBPF::SetProcFd(base::ScopedFD proc_fd) {
+  proc_fd_.swap(proc_fd);
 }
 
 // static

sandbox/linux/seccomp-bpf/sandbox_bpf.h

@@ -64,14 +64,14 @@ class SANDBOX_EXPORT SandboxBPF {
   // combined policy. So, it should only be used if there are no alternatives.
   bool StartSandbox(SeccompLevel level) WARN_UNUSED_RESULT;
 
-  // The sandbox needs to be able to access files in "/proc/self/task/". If
+  // The sandbox needs to be able to access files in "/proc/self/". If
   // this directory is not accessible when "StartSandbox()" gets called, the
   // caller must provide an already opened file descriptor by calling
-  // "SetProcTaskFd()".
+  // "SetProcFd()".
   // The sandbox becomes the new owner of this file descriptor and will
   // close it when "StartSandbox()" executes or when the sandbox object
   // disappears.
-  void SetProcTaskFd(base::ScopedFD proc_task_fd);
+  void SetProcFd(base::ScopedFD proc_fd);
 
   // Checks whether a particular system call number is valid on the current
   // architecture.
@@ -106,7 +106,7 @@ class SANDBOX_EXPORT SandboxBPF {
   // been configured with SetSandboxPolicy().
   void InstallFilter(bool must_sync_threads);
 
-  base::ScopedFD proc_task_fd_;
+  base::ScopedFD proc_fd_;
   bool sandbox_has_started_;
   scoped_ptr<bpf_dsl::Policy> policy_;
 

sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc

@@ -71,7 +71,7 @@ TEST(SandboxBPF, ProcTaskFdDescriptorGetsClosed) {
 
   {
     SandboxBPF sandbox(nullptr);
-    sandbox.SetProcTaskFd(write_end.Pass());
+    sandbox.SetProcFd(write_end.Pass());
   }
 
   ASSERT_EQ(0, fcntl(read_end.get(), F_SETFL, O_NONBLOCK));

sandbox/linux/services/credentials.cc

@@ -23,7 +23,9 @@
 #include "base/template_util.h"
 #include "base/third_party/valgrind/valgrind.h"
 #include "sandbox/linux/services/namespace_utils.h"
+#include "sandbox/linux/services/proc_util.h"
 #include "sandbox/linux/services/syscall_wrappers.h"
+#include "sandbox/linux/services/thread_helpers.h"
 
 namespace sandbox {
 
@@ -129,7 +131,10 @@ void CheckCloneNewUserErrno(int error) {
 
 }  // namespace.
 
-bool Credentials::DropAllCapabilities() {
+bool Credentials::DropAllCapabilities(int proc_fd) {
+  DCHECK_LE(0, proc_fd);
+  CHECK(ThreadHelpers::IsSingleThreaded(proc_fd));
+
   ScopedCap cap(cap_init());
   CHECK(cap);
   PCHECK(0 == cap_set_proc(cap.get()));
@@ -138,6 +143,11 @@ bool Credentials::DropAllCapabilities() {
   return true;
 }
 
+bool Credentials::DropAllCapabilities() {
+  base::ScopedFD proc_fd(ProcUtil::OpenProc());
+  return Credentials::DropAllCapabilities(proc_fd.get());
+}
+
 bool Credentials::HasAnyCapability() {
   ScopedCap current_cap(cap_get_proc());
   CHECK(current_cap);
@@ -220,9 +230,12 @@ bool Credentials::MoveToNewUserNS() {
   return true;
 }
 
-bool Credentials::DropFileSystemAccess() {
+bool Credentials::DropFileSystemAccess(int proc_fd) {
+  CHECK_LE(0, proc_fd);
+
   CHECK(ChrootToSafeEmptyDir());
   CHECK(!base::DirectoryExists(base::FilePath("/proc")));
+  CHECK(!ProcUtil::HasOpenDirectory(proc_fd));
   // We never let this function fail.
   return true;
 }

sandbox/linux/services/credentials.h

@@ -29,7 +29,12 @@ class SANDBOX_EXPORT Credentials {
   // the current process. For security reasons, since capabilities are
   // per-thread, the caller is responsible for ensuring it is single-threaded
   // when calling this API.
+  // |proc_fd| must be a file descriptor to /proc/ and remains owned by
+  // the caller.
+  static bool DropAllCapabilities(int proc_fd) WARN_UNUSED_RESULT;
+  // A similar API which assumes that it can open /proc/self/ by itself.
   static bool DropAllCapabilities() WARN_UNUSED_RESULT;
+
   // Return true iff there is any capability in any of the capabilities sets
   // of the current process.
   static bool HasAnyCapability();
@@ -58,13 +63,14 @@ class SANDBOX_EXPORT Credentials {
   // available.
   // The implementation currently uses chroot(2) and requires CAP_SYS_CHROOT.
   // CAP_SYS_CHROOT can be acquired by using the MoveToNewUserNS() API.
-  // Make sure to call DropAllCapabilities() after this call to prevent
-  // escapes.
-  // To be secure, the caller must ensure that any directory file descriptors
-  // are closed (for example, by checking the result of
-  // ProcUtil::HasOpenDirectory with a file descriptor for /proc, then closing
-  // that file descriptor). Otherwise it may be possible to escape the chroot.
-  static bool DropFileSystemAccess() WARN_UNUSED_RESULT;
+  // |proc_fd| must be a file descriptor to /proc/ and must be the only open
+  // directory file descriptor of the process.
+  //
+  // CRITICAL:
+  //   - the caller must close |proc_fd| eventually or access to the file
+  // system can be recovered.
+  //   - DropAllCapabilities() must be called to prevent escapes.
+  static bool DropFileSystemAccess(int proc_fd) WARN_UNUSED_RESULT;
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(Credentials);

sandbox/linux/services/credentials_unittest.cc

@@ -16,6 +16,7 @@
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "sandbox/linux/services/proc_util.h"
 #include "sandbox/linux/tests/unit_tests.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -135,7 +136,7 @@ SANDBOX_TEST(Credentials, DISABLE_ON_ASAN(DropFileSystemAccessIsSafe)) {
   CHECK(Credentials::DropAllCapabilities());
   // Probably missing kernel support.
   if (!Credentials::MoveToNewUserNS()) return;
-  CHECK(Credentials::DropFileSystemAccess());
+  CHECK(Credentials::DropFileSystemAccess(ProcUtil::OpenProc().get()));
   CHECK(!base::DirectoryExists(base::FilePath("/proc")));
   CHECK(WorkingDirectoryIsRoot());
   CHECK(base::IsDirectoryEmpty(base::FilePath("/")));
@@ -147,11 +148,12 @@ SANDBOX_TEST(Credentials, DISABLE_ON_ASAN(DropFileSystemAccessIsSafe)) {
 // Check that after dropping filesystem access and dropping privileges
 // it is not possible to regain capabilities.
 SANDBOX_TEST(Credentials, DISABLE_ON_ASAN(CannotRegainPrivileges)) {
-  CHECK(Credentials::DropAllCapabilities());
+  base::ScopedFD proc_fd(ProcUtil::OpenProc());
+  CHECK(Credentials::DropAllCapabilities(proc_fd.get()));
   // Probably missing kernel support.
   if (!Credentials::MoveToNewUserNS()) return;
-  CHECK(Credentials::DropFileSystemAccess());
-  CHECK(Credentials::DropAllCapabilities());
+  CHECK(Credentials::DropFileSystemAccess(proc_fd.get()));
+  CHECK(Credentials::DropAllCapabilities(proc_fd.get()));
 
   // The kernel should now prevent us from regaining capabilities because we
   // are in a chroot.

sandbox/linux/services/namespace_sandbox_unittest.cc

@@ -21,6 +21,7 @@
 #include "base/test/multiprocess_test.h"
 #include "sandbox/linux/services/credentials.h"
 #include "sandbox/linux/services/namespace_utils.h"
+#include "sandbox/linux/services/proc_util.h"
 #include "sandbox/linux/tests/unit_tests.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/multiprocess_func_list.h"
@@ -85,7 +86,7 @@ TEST_F(NamespaceSandboxTest, BasicUsage) {
 MULTIPROCESS_TEST_MAIN(ChrootMe) {
   CHECK(!RootDirectoryIsEmpty());
   CHECK(sandbox::Credentials::MoveToNewUserNS());
-  CHECK(sandbox::Credentials::DropFileSystemAccess());
+  CHECK(sandbox::Credentials::DropFileSystemAccess(ProcUtil::OpenProc().get()));
   CHECK(RootDirectoryIsEmpty());
   return 0;
 }

sandbox/linux/services/proc_util.cc

@@ -28,12 +28,20 @@ struct DIRCloser {
 
 typedef scoped_ptr<DIR, DIRCloser> ScopedDIR;
 
+base::ScopedFD OpenDirectory(const char* path) {
+  DCHECK(path);
+  base::ScopedFD directory_fd(
+      HANDLE_EINTR(open(path, O_RDONLY | O_DIRECTORY | O_CLOEXEC)));
+  PCHECK(directory_fd.is_valid());
+  return directory_fd.Pass();
+}
+
 }  // namespace
 
 int ProcUtil::CountOpenFds(int proc_fd) {
   DCHECK_LE(0, proc_fd);
   int proc_self_fd = HANDLE_EINTR(
-      openat(proc_fd, "self/fd", O_DIRECTORY | O_RDONLY | O_CLOEXEC));
+      openat(proc_fd, "self/fd/", O_DIRECTORY | O_RDONLY | O_CLOEXEC));
   PCHECK(0 <= proc_self_fd);
 
   // Ownership of proc_self_fd is transferred here, it must not be closed
@@ -61,24 +69,10 @@ int ProcUtil::CountOpenFds(int proc_fd) {
 }
 
 bool ProcUtil::HasOpenDirectory(int proc_fd) {
-  int proc_self_fd = -1;
-  if (proc_fd >= 0) {
-    proc_self_fd = openat(proc_fd, "self/fd", O_DIRECTORY | O_RDONLY);
-  } else {
-    proc_self_fd = openat(AT_FDCWD, "/proc/self/fd", O_DIRECTORY | O_RDONLY);
-    if (proc_self_fd < 0) {
-      // If this process has been chrooted (eg into /proc/self/fdinfo) then
-      // the new root dir will not have directory listing permissions for us
-      // (hence EACCES).  And if we do have this permission, then /proc won't
-      // exist anyway (hence ENOENT).
-      DPCHECK(errno == EACCES || errno == ENOENT)
-        << "Unexpected failure when trying to open /proc/self/fd: ("
-        << errno << ") " << strerror(errno);
-
-      // If not available, guess false.
-      return false;
-    }
-  }
+  DCHECK_LE(0, proc_fd);
+  int proc_self_fd =
+      openat(proc_fd, "self/fd/", O_DIRECTORY | O_RDONLY | O_CLOEXEC);
+
   PCHECK(0 <= proc_self_fd);
 
   // Ownership of proc_self_fd is transferred here, it must not be closed
@@ -111,13 +105,15 @@ bool ProcUtil::HasOpenDirectory(int proc_fd) {
   return false;
 }
 
+bool ProcUtil::HasOpenDirectory() {
+  base::ScopedFD proc_fd(
+      HANDLE_EINTR(open("/proc/", O_DIRECTORY | O_RDONLY | O_CLOEXEC)));
+  return HasOpenDirectory(proc_fd.get());
+}
+
 //static
-base::ScopedFD ProcUtil::OpenProcSelfTask() {
-  base::ScopedFD proc_self_task(
-   HANDLE_EINTR(
-      open("/proc/self/task/", O_RDONLY | O_DIRECTORY | O_CLOEXEC)));
-  PCHECK(proc_self_task.is_valid());
-  return proc_self_task.Pass();
+base::ScopedFD ProcUtil::OpenProc() {
+  return OpenDirectory("/proc/");
 }
 
 }  // namespace sandbox

sandbox/linux/services/proc_util.h

@@ -15,7 +15,7 @@ class SANDBOX_EXPORT ProcUtil {
  public:
   // Returns the number of file descriptors in the current process's FD
   // table, excluding |proc_fd|, which should be a file descriptor for
-  // /proc.
+  // /proc/.
   static int CountOpenFds(int proc_fd);
 
   // Checks whether the current process has any directory file descriptor open.
@@ -24,16 +24,14 @@ class SANDBOX_EXPORT ProcUtil {
   // DropFileSystemAccess().
   // Sometimes it's useful to call HasOpenDirectory() after file system access
   // has been dropped. In this case, |proc_fd| should be a file descriptor to
-  // /proc. The file descriptor in |proc_fd| will be ignored by
+  // /proc/. The file descriptor in |proc_fd| will be ignored by
   // HasOpenDirectory() and remains owned by the caller. It is very important
   // for the caller to close it.
-  // If /proc is available, |proc_fd| can be passed as -1.
-  // If |proc_fd| is -1 and /proc is not available, this function will return
-  // false.
-  static bool HasOpenDirectory(int proc_fd);
+  static bool HasOpenDirectory(int proc_fd) WARN_UNUSED_RESULT;
+  static bool HasOpenDirectory() WARN_UNUSED_RESULT;
 
-  // Open /proc/self/task/ or crash if it cannot.
-  static base::ScopedFD OpenProcSelfTask();
+  // Open /proc/ or crash if not possible.
+  static base::ScopedFD OpenProc();
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(ProcUtil);

sandbox/linux/services/proc_util_unittest.cc

@@ -14,7 +14,7 @@
 namespace sandbox {
 
 TEST(ProcUtil, CountOpenFds) {
-  base::ScopedFD proc_fd(open("/proc", O_RDONLY | O_DIRECTORY));
+  base::ScopedFD proc_fd(open("/proc/", O_RDONLY | O_DIRECTORY));
   ASSERT_TRUE(proc_fd.is_valid());
   int fd_count = ProcUtil::CountOpenFds(proc_fd.get());
   int fd = open("/dev/null", O_RDONLY);
@@ -26,35 +26,35 @@ TEST(ProcUtil, CountOpenFds) {
 
 TEST(ProcUtil, HasOpenDirectory) {
   // No open directory should exist at startup.
-  EXPECT_FALSE(ProcUtil::HasOpenDirectory(-1));
+  EXPECT_FALSE(ProcUtil::HasOpenDirectory());
   {
     // Have a "/proc" file descriptor around.
-    int proc_fd = open("/proc", O_RDONLY | O_DIRECTORY);
+    int proc_fd = open("/proc/", O_RDONLY | O_DIRECTORY);
     base::ScopedFD proc_fd_closer(proc_fd);
-    EXPECT_TRUE(ProcUtil::HasOpenDirectory(-1));
+    EXPECT_TRUE(ProcUtil::HasOpenDirectory());
   }
-  EXPECT_FALSE(ProcUtil::HasOpenDirectory(-1));
+  EXPECT_FALSE(ProcUtil::HasOpenDirectory());
 }
 
 TEST(ProcUtil, HasOpenDirectoryWithFD) {
-  int proc_fd = open("/proc", O_RDONLY | O_DIRECTORY);
+  int proc_fd = open("/proc/", O_RDONLY | O_DIRECTORY);
   base::ScopedFD proc_fd_closer(proc_fd);
   ASSERT_LE(0, proc_fd);
 
   // Don't pass |proc_fd|, an open directory (proc_fd) should
   // be detected.
-  EXPECT_TRUE(ProcUtil::HasOpenDirectory(-1));
+  EXPECT_TRUE(ProcUtil::HasOpenDirectory());
   // Pass |proc_fd| and no open directory should be detected.
   EXPECT_FALSE(ProcUtil::HasOpenDirectory(proc_fd));
 
   {
     // Have a directory file descriptor around.
-    int open_directory_fd = open("/proc/self", O_RDONLY | O_DIRECTORY);
+    int open_directory_fd = open("/proc/self/", O_RDONLY | O_DIRECTORY);
     base::ScopedFD open_directory_fd_closer(open_directory_fd);
     EXPECT_TRUE(ProcUtil::HasOpenDirectory(proc_fd));
   }
 
-  // The "/proc/self" file descriptor should now be closed, |proc_fd| is the
+  // The "/proc/" file descriptor should now be closed, |proc_fd| is the
   // only directory file descriptor open.
   EXPECT_FALSE(ProcUtil::HasOpenDirectory(proc_fd));
 }

sandbox/linux/services/thread_helpers.cc

@@ -31,10 +31,10 @@ namespace {
 const char kAssertSingleThreadedError[] =
     "Current process is not mono-threaded!";
 
-bool IsSingleThreadedImpl(int proc_self_task) {
-  CHECK_LE(0, proc_self_task);
+bool IsSingleThreadedImpl(int proc_fd) {
+  CHECK_LE(0, proc_fd);
   struct stat task_stat;
-  int fstat_ret = fstat(proc_self_task, &task_stat);
+  int fstat_ret = fstatat(proc_fd, "self/task/", &task_stat, 0);
   PCHECK(0 == fstat_ret);
 
   // At least "..", "." and the current thread should be present.
@@ -45,11 +45,11 @@ bool IsSingleThreadedImpl(int proc_self_task) {
   return task_stat.st_nlink == 3;
 }
 
-bool IsThreadPresentInProcFS(int proc_self_task,
+bool IsThreadPresentInProcFS(int proc_fd,
                              const std::string& thread_id_dir_str) {
   struct stat task_stat;
   const int fstat_ret =
-      fstatat(proc_self_task, thread_id_dir_str.c_str(), &task_stat, 0);
+      fstatat(proc_fd, thread_id_dir_str.c_str(), &task_stat, 0);
   if (fstat_ret < 0) {
     PCHECK(ENOENT == errno);
     return false;
@@ -98,44 +98,44 @@ void RunWhileTrue(const base::Callback<bool(void)>& cb) {
   NOTREACHED();
 }
 
-bool IsMultiThreaded(int proc_self_task) {
-  return !ThreadHelpers::IsSingleThreaded(proc_self_task);
+bool IsMultiThreaded(int proc_fd) {
+  return !ThreadHelpers::IsSingleThreaded(proc_fd);
 }
 
 }  // namespace
 
 // static
-bool ThreadHelpers::IsSingleThreaded(int proc_self_task) {
-  DCHECK_LE(0, proc_self_task);
-  return IsSingleThreadedImpl(proc_self_task);
+bool ThreadHelpers::IsSingleThreaded(int proc_fd) {
+  DCHECK_LE(0, proc_fd);
+  return IsSingleThreadedImpl(proc_fd);
 }
 
 // static
 bool ThreadHelpers::IsSingleThreaded() {
-  base::ScopedFD task_fd(ProcUtil::OpenProcSelfTask());
+  base::ScopedFD task_fd(ProcUtil::OpenProc());
   return IsSingleThreaded(task_fd.get());
 }
 
 // static
-void ThreadHelpers::AssertSingleThreaded(int proc_self_task) {
-  DCHECK_LE(0, proc_self_task);
-  const base::Callback<bool(void)> cb =
-      base::Bind(&IsMultiThreaded, proc_self_task);
+void ThreadHelpers::AssertSingleThreaded(int proc_fd) {
+  DCHECK_LE(0, proc_fd);
+  const base::Callback<bool(void)> cb = base::Bind(&IsMultiThreaded, proc_fd);
   RunWhileTrue(cb);
 }
 
 void ThreadHelpers::AssertSingleThreaded() {
-  base::ScopedFD task_fd(ProcUtil::OpenProcSelfTask());
+  base::ScopedFD task_fd(ProcUtil::OpenProc());
   AssertSingleThreaded(task_fd.get());
 }
 
 // static
-bool ThreadHelpers::StopThreadAndWatchProcFS(int proc_self_task,
+bool ThreadHelpers::StopThreadAndWatchProcFS(int proc_fd,
                                              base::Thread* thread) {
-  DCHECK_LE(0, proc_self_task);
+  DCHECK_LE(0, proc_fd);
   DCHECK(thread);
   const base::PlatformThreadId thread_id = thread->thread_id();
-  const std::string thread_id_dir_str = base::IntToString(thread_id) + "/";
+  const std::string thread_id_dir_str =
+      "self/task/" + base::IntToString(thread_id) + "/";
 
   // The kernel is at liberty to wake the thread id futex before updating
   // /proc. Following Stop(), the thread is joined, but entries in /proc may
@@ -143,7 +143,7 @@ bool ThreadHelpers::StopThreadAndWatchProcFS(int proc_self_task,
   thread->Stop();
 
   const base::Callback<bool(void)> cb =
-      base::Bind(&IsThreadPresentInProcFS, proc_self_task, thread_id_dir_str);
+      base::Bind(&IsThreadPresentInProcFS, proc_fd, thread_id_dir_str);
 
   RunWhileTrue(cb);
 

sandbox/linux/services/thread_helpers.h

@@ -14,24 +14,23 @@ namespace sandbox {
 
 class SANDBOX_EXPORT ThreadHelpers {
  public:
-  // Check whether the current process is single threaded. |proc_self_tasks|
-  // must be a file descriptor to /proc/self/task/ and remains owned by the
+  // Check whether the current process is single threaded. |proc_fd|
+  // must be a file descriptor to /proc/ and remains owned by the
   // caller.
-  static bool IsSingleThreaded(int proc_self_task);
+  static bool IsSingleThreaded(int proc_fd);
   static bool IsSingleThreaded();
 
   // Crash if the current process is not single threaded. This will wait
   // on /proc to be updated. In the case where this doesn't crash, this will
   // return promptly. In the case where this does crash, this will first wait
   // for a few ms in Debug mode, a few seconds in Release mode.
-  static void AssertSingleThreaded(int proc_self_task);
+  static void AssertSingleThreaded(int proc_fd);
   static void AssertSingleThreaded();
 
   // Stop |thread| and ensure that it does not have an entry in
   // /proc/self/task/ from the point of view of the current thread. This is
   // the way to stop threads before calling IsSingleThreaded().
-  static bool StopThreadAndWatchProcFS(int proc_self_task,
-                                       base::Thread* thread);
+  static bool StopThreadAndWatchProcFS(int proc_fd, base::Thread* thread);
 
   static const char* GetAssertSingleThreadedErrorMessageForTests();
 

sandbox/linux/services/thread_helpers_unittests.cc

@@ -38,71 +38,71 @@ int GetRaceTestIterations() {
   }
 }
 
-class ScopedProcSelfTask {
+class ScopedProc {
  public:
-  ScopedProcSelfTask() : fd_(-1) {
-    fd_ = open("/proc/self/task/", O_RDONLY | O_DIRECTORY);
+  ScopedProc() : fd_(-1) {
+    fd_ = open("/proc/", O_RDONLY | O_DIRECTORY);
     CHECK_LE(0, fd_);
   }
 
-  ~ScopedProcSelfTask() { PCHECK(0 == IGNORE_EINTR(close(fd_))); }
+  ~ScopedProc() { PCHECK(0 == IGNORE_EINTR(close(fd_))); }
 
   int fd() { return fd_; }
 
  private:
   int fd_;
-  DISALLOW_COPY_AND_ASSIGN(ScopedProcSelfTask);
+  DISALLOW_COPY_AND_ASSIGN(ScopedProc);
 };
 
 TEST(ThreadHelpers, IsSingleThreadedBasic) {
-  ScopedProcSelfTask task;
-  ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(task.fd()));
+  ScopedProc proc_fd;
+  ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(proc_fd.fd()));
   ASSERT_TRUE(ThreadHelpers::IsSingleThreaded());
 
   base::Thread thread("sandbox_tests");
   ASSERT_TRUE(thread.Start());
-  ASSERT_FALSE(ThreadHelpers::IsSingleThreaded(task.fd()));
+  ASSERT_FALSE(ThreadHelpers::IsSingleThreaded(proc_fd.fd()));
   ASSERT_FALSE(ThreadHelpers::IsSingleThreaded());
   // Explicitly stop the thread here to not pollute the next test.
-  ASSERT_TRUE(ThreadHelpers::StopThreadAndWatchProcFS(task.fd(), &thread));
+  ASSERT_TRUE(ThreadHelpers::StopThreadAndWatchProcFS(proc_fd.fd(), &thread));
 }
 
 SANDBOX_TEST(ThreadHelpers, AssertSingleThreaded) {
-  ScopedProcSelfTask task;
-  SANDBOX_ASSERT(ThreadHelpers::IsSingleThreaded(task.fd()));
+  ScopedProc proc_fd;
+  SANDBOX_ASSERT(ThreadHelpers::IsSingleThreaded(proc_fd.fd()));
   SANDBOX_ASSERT(ThreadHelpers::IsSingleThreaded());
 
-  ThreadHelpers::AssertSingleThreaded(task.fd());
+  ThreadHelpers::AssertSingleThreaded(proc_fd.fd());
   ThreadHelpers::AssertSingleThreaded();
 }
 
 TEST(ThreadHelpers, IsSingleThreadedIterated) {
-  ScopedProcSelfTask task;
-  ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(task.fd()));
+  ScopedProc proc_fd;
+  ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(proc_fd.fd()));
 
   // Iterate to check for race conditions.
   for (int i = 0; i < GetRaceTestIterations(); ++i) {
     base::Thread thread("sandbox_tests");
     ASSERT_TRUE(thread.Start());
-    ASSERT_FALSE(ThreadHelpers::IsSingleThreaded(task.fd()));
+    ASSERT_FALSE(ThreadHelpers::IsSingleThreaded(proc_fd.fd()));
     // Explicitly stop the thread here to not pollute the next test.
-    ASSERT_TRUE(ThreadHelpers::StopThreadAndWatchProcFS(task.fd(), &thread));
+    ASSERT_TRUE(ThreadHelpers::StopThreadAndWatchProcFS(proc_fd.fd(), &thread));
   }
 }
 
 TEST(ThreadHelpers, IsSingleThreadedStartAndStop) {
-  ScopedProcSelfTask task;
-  ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(task.fd()));
+  ScopedProc proc_fd;
+  ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(proc_fd.fd()));
 
   base::Thread thread("sandbox_tests");
   // This is testing for a race condition, so iterate.
   // Manually, this has been tested with more that 1M iterations.
   for (int i = 0; i < GetRaceTestIterations(); ++i) {
     ASSERT_TRUE(thread.Start());
-    ASSERT_FALSE(ThreadHelpers::IsSingleThreaded(task.fd()));
+    ASSERT_FALSE(ThreadHelpers::IsSingleThreaded(proc_fd.fd()));
 
-    ASSERT_TRUE(ThreadHelpers::StopThreadAndWatchProcFS(task.fd(), &thread));
-    ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(task.fd()));
+    ASSERT_TRUE(ThreadHelpers::StopThreadAndWatchProcFS(proc_fd.fd(), &thread));
+    ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(proc_fd.fd()));
     ASSERT_EQ(1, base::GetNumberOfThreads(base::GetCurrentProcessHandle()));
   }
 }