Merge 249937 "Linux Sandbox: Stop GPU watchdog in accountable way."

> Linux Sandbox: Stop GPU watchdog in accountable way.
> 
> The Linux sandbox can sometimes detect a spurious running thread if it
> has just been stopped.
> 
> We add a new LinuxSandbox::StopThread() method to safely stop threads
> and make sure they won't be counted as still running.
> 
> BUG=328620
> NOTRY=true
> 
> Review URL: https://codereview.chromium.org/147203005

TBR=jln@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/branches/1750/src@251107 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 300 insertions, 30 deletions

diff --git a/content/common/sandbox_linux/sandbox_linux.cc b/content/common/sandbox_linux/sandbox_linux.cc
index 6b14a1c..a30e41c 100644
--- a/content/common/sandbox_linux/sandbox_linux.cc
+++ b/content/common/sandbox_linux/sandbox_linux.cc
@@ -8,6 +8,7 @@
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/types.h>
+#include <unistd.h>
 
 #include <limits>
 
@@ -15,6 +16,7 @@
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/strings/string_number_conversions.h"
@@ -25,10 +27,23 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/sandbox_linux.h"
 #include "sandbox/linux/services/credentials.h"
+#include "sandbox/linux/services/thread_helpers.h"
 #include "sandbox/linux/suid/client/setuid_sandbox_client.h"
 
 namespace {
 
+struct FDCloser {
+  inline void operator()(int* fd) const {
+    DCHECK(fd);
+    PCHECK(0 == IGNORE_EINTR(close(*fd)));
+    *fd = -1;
+  }
+};
+
+// Don't use base::ScopedFD since it doesn't CHECK that the file descriptor was
+// closed.
+typedef scoped_ptr<int, FDCloser> SafeScopedFD;
+
 void LogSandboxStarted(const std::string& sandbox_name) {
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   const std::string process_type =
@@ -64,6 +79,21 @@ 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).
+int OpenProcTaskFd(int proc_fd) {
+  int proc_self_task = -1;
+  if (proc_fd >= 0) {
+    // If a handle to /proc is available, use it. This allows to bypass file
+    // system restrictions.
+    proc_self_task = openat(proc_fd, "self/task/", O_RDONLY | O_DIRECTORY);
+  } else {
+    // Otherwise, make an attempt to access the file system directly.
+    proc_self_task = open("/proc/self/task/", O_RDONLY | O_DIRECTORY);
+  }
+  return proc_self_task;
+}
+
 }  // namespace
 
 namespace content {
@@ -125,6 +155,11 @@ bool LinuxSandbox::InitializeSandbox() {
   return linux_sandbox->InitializeSandboxImpl();
 }
 
+void LinuxSandbox::StopThread(base::Thread* thread) {
+  LinuxSandbox* linux_sandbox = LinuxSandbox::GetInstance();
+  linux_sandbox->StopThreadImpl(thread);
+}
+
 int LinuxSandbox::GetStatus() {
   CHECK(pre_initialized_);
   if (kSandboxLinuxInvalid == sandbox_status_flags_) {
@@ -153,35 +188,29 @@ int LinuxSandbox::GetStatus() {
 // PID namespaces and existing sandboxes, so "self" must really be used instead
 // of using the pid.
 bool LinuxSandbox::IsSingleThreaded() const {
-  struct stat task_stat;
-  int fstat_ret;
-  if (proc_fd_ >= 0) {
-    // If a handle to /proc is available, use it. This allows to bypass file
-    // system restrictions.
-    fstat_ret = fstatat(proc_fd_, "self/task/", &task_stat, 0);
-  } else {
-    // Otherwise, make an attempt to access the file system directly.
-    fstat_ret = fstatat(AT_FDCWD, "/proc/self/task/", &task_stat, 0);
-  }
-  // In Debug mode, it's mandatory to be able to count threads to catch bugs.
+  bool is_single_threaded = false;
+  int proc_self_task = OpenProcTaskFd(proc_fd_);
+
+// In Debug mode, it's mandatory to be able to count threads to catch bugs.
 #if !defined(NDEBUG)
-  // Using DCHECK here would be incorrect. DCHECK can be enabled in non
-  // official release mode.
-  CHECK_EQ(0, fstat_ret) << "Could not count threads, the sandbox was not "
-                         << "pre-initialized properly.";
+  // Using CHECK here since we want to check all the cases where
+  // !defined(NDEBUG)
+  // gets built.
+  CHECK_LE(0, proc_self_task) << "Could not count threads, the sandbox was not "
+                              << "pre-initialized properly.";
 #endif  // !defined(NDEBUG)
-  if (fstat_ret) {
+
+  if (proc_self_task < 0) {
     // Pretend to be monothreaded if it can't be determined (for instance the
     // setuid sandbox is already engaged but no proc_fd_ is available).
-    return true;
+    is_single_threaded = true;
+  } else {
+    SafeScopedFD task_closer(&proc_self_task);
+    is_single_threaded =
+        sandbox::ThreadHelpers::IsSingleThreaded(proc_self_task);
   }
 
-  // At least "..", "." and the current thread should be present.
-  CHECK_LE(3UL, task_stat.st_nlink);
-  // Counting threads via /proc/self/task could be racy. For the purpose of
-  // determining if the current proces is monothreaded it works: if at any
-  // time it becomes monothreaded, it'll stay so.
-  return task_stat.st_nlink == 3;
+  return is_single_threaded;
 }
 
 bool LinuxSandbox::seccomp_bpf_started() const {
@@ -257,6 +286,10 @@ bool LinuxSandbox::InitializeSandboxImpl() {
   return seccomp_bpf_started;
 }
 
+void LinuxSandbox::StopThreadImpl(base::Thread* thread) {
+  DCHECK(thread);
+  StopThreadAndEnsureNotCounted(thread);
+}
 
 bool LinuxSandbox::seccomp_bpf_supported() const {
   CHECK(pre_initialized_);
@@ -303,7 +336,7 @@ bool LinuxSandbox::LimitAddressSpace(const std::string& process_type) {
 #endif  // !defined(ADDRESS_SANITIZER)
 }
 
-bool LinuxSandbox::HasOpenDirectories() {
+bool LinuxSandbox::HasOpenDirectories() const {
   return sandbox::Credentials().HasOpenDirectory(proc_fd_);
 }
 
@@ -330,4 +363,13 @@ void LinuxSandbox::CheckForBrokenPromises(const std::string& process_type) {
   }
 }
 
+void LinuxSandbox::StopThreadAndEnsureNotCounted(base::Thread* thread) const {
+  DCHECK(thread);
+  int proc_self_task = OpenProcTaskFd(proc_fd_);
+  PCHECK(proc_self_task >= 0);
+  SafeScopedFD task_closer(&proc_self_task);
+  CHECK(
+      sandbox::ThreadHelpers::StopThreadAndWatchProcFS(proc_self_task, thread));
+}
+
 }  // namespace content
diff --git a/content/common/sandbox_linux/sandbox_linux.h b/content/common/sandbox_linux/sandbox_linux.h
index da1b963..8b5876a 100644
--- a/content/common/sandbox_linux/sandbox_linux.h
+++ b/content/common/sandbox_linux/sandbox_linux.h
@@ -12,6 +12,9 @@
 #include "content/public/common/sandbox_linux.h"
 
 template <typename T> struct DefaultSingletonTraits;
+namespace base {
+class Thread;
+}
 namespace sandbox { class SetuidSandboxClient; }
 
 namespace content {
@@ -45,8 +48,12 @@ class LinuxSandbox {
   // seccomp-bpf and address space limitations (the setuid sandbox works
   // differently and is set-up in the Zygote). This will instantiate the
   // LinuxSandbox singleton if it doesn't already exist.
+  // This function should only be called without any thread running.
   static bool InitializeSandbox();
 
+  // Stop |thread| in a way that can be trusted by the sandbox.
+  static void StopThread(base::Thread* thread);
+
   // Returns the status of the renderer, worker and ppapi sandbox. Can only
   // be queried after going through PreinitializeSandbox(). This is a bitmask
   // and uses the constants defined in "enum LinuxSandboxStatus". Since the
@@ -78,21 +85,25 @@ class LinuxSandbox {
  private:
   friend struct DefaultSingletonTraits<LinuxSandbox>;
 
-  // InitializeSandbox() is static and gets an instance of the Singleton. This
-  // is the non-static implementation.
+  // Some methods are static and get an instance of the Singleton. These
+  // are the non-static implementations.
   bool InitializeSandboxImpl();
+  void StopThreadImpl(base::Thread* thread);
   // We must have been pre_initialized_ before using this.
   bool seccomp_bpf_supported() const;
   // Returns true if it can be determined that the current process has open
   // directories that are not managed by the LinuxSandbox class. This would
   // be a vulnerability as it would allow to bypass the setuid sandbox.
-  bool HasOpenDirectories();
+  bool HasOpenDirectories() const;
   // The last part of the initialization is to make sure any temporary "hole"
   // in the sandbox is closed. For now, this consists of closing proc_fd_.
   void SealSandbox();
   // GetStatus() makes promises as to how the sandbox will behave. This
   // checks that no promises have been broken.
   void CheckForBrokenPromises(const std::string& process_type);
+  // Stop |thread| and make sure it does not appear in /proc/self/tasks/
+  // anymore.
+  void StopThreadAndEnsureNotCounted(base::Thread* thread) const;
 
   // A file descriptor to /proc. It's dangerous to have it around as it could
   // allow for sandbox bypasses. It needs to be closed before we consider
diff --git a/content/gpu/gpu_main.cc b/content/gpu/gpu_main.cc
index f7650e7..80715ec 100644
--- a/content/gpu/gpu_main.cc
+++ b/content/gpu/gpu_main.cc
@@ -403,13 +403,17 @@ bool StartSandboxLinux(const gpu::GPUInfo& gpu_info,
 
   WarmUpSandboxNvidia(gpu_info, should_initialize_gl_context);
 
-  if (watchdog_thread)
-    watchdog_thread->Stop();
+  if (watchdog_thread) {
+    // LinuxSandbox needs to be able to ensure that the thread
+    // has really been stopped.
+    LinuxSandbox::StopThread(watchdog_thread);
+  }
   // LinuxSandbox::InitializeSandbox() must always be called
   // with only one thread.
   res = LinuxSandbox::InitializeSandbox();
-  if (watchdog_thread)
+  if (watchdog_thread) {
     watchdog_thread->Start();
+  }
 
   return res;
 }
diff --git a/sandbox/linux/sandbox_linux.gypi b/sandbox/linux/sandbox_linux.gypi
index c20ab04..0e211f6 100644
--- a/sandbox/linux/sandbox_linux.gypi
+++ b/sandbox/linux/sandbox_linux.gypi
@@ -193,6 +193,8 @@
         'services/broker_process.h',
         'services/init_process_reaper.cc',
         'services/init_process_reaper.h',
+        'services/thread_helpers.cc',
+        'services/thread_helpers.h',
       ],
       'dependencies': [
         '../base/base.gyp:base',
diff --git a/sandbox/linux/sandbox_linux_test_sources.gypi b/sandbox/linux/sandbox_linux_test_sources.gypi
index 21c4214..a6a916f 100644
--- a/sandbox/linux/sandbox_linux_test_sources.gypi
+++ b/sandbox/linux/sandbox_linux_test_sources.gypi
@@ -18,6 +18,7 @@
     'tests/unit_tests.cc',
     'tests/unit_tests.h',
     'services/broker_process_unittest.cc',
+    'services/thread_helpers_unittests.cc',
   ],
   'conditions': [
     [ 'compile_suid_client==1', {
diff --git a/sandbox/linux/services/thread_helpers.cc b/sandbox/linux/services/thread_helpers.cc
new file mode 100644
index 0000000..e0794f8
--- /dev/null
+++ b/sandbox/linux/services/thread_helpers.cc
@@ -0,0 +1,84 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sandbox/linux/services/thread_helpers.h"
+
+#include <errno.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/threading/platform_thread.h"
+#include "base/threading/thread.h"
+
+namespace sandbox {
+
+bool ThreadHelpers::IsSingleThreaded(int proc_self_task) {
+  CHECK_LE(0, proc_self_task);
+  struct stat task_stat;
+  int fstat_ret = fstat(proc_self_task, &task_stat);
+  PCHECK(0 == fstat_ret);
+
+  // At least "..", "." and the current thread should be present.
+  CHECK_LE(3UL, task_stat.st_nlink);
+  // Counting threads via /proc/self/task could be racy. For the purpose of
+  // determining if the current proces is monothreaded it works: if at any
+  // time it becomes monothreaded, it'll stay so.
+  return task_stat.st_nlink == 3;
+}
+
+bool ThreadHelpers::StopThreadAndWatchProcFS(int proc_self_task,
+                                             base::Thread* thread) {
+  DCHECK_LE(0, proc_self_task);
+  DCHECK(thread);
+  const base::PlatformThreadId thread_id = thread->thread_id();
+  const std::string thread_id_dir_str = 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
+  // not have been updated.
+  thread->Stop();
+
+  unsigned int iterations = 0;
+  bool thread_present_in_procfs = true;
+  // Poll /proc with an exponential back-off, sleeping 2^iterations nanoseconds
+  // in nanosleep(2).
+  // Note: the clock may not allow for nanosecond granularity, in this case the
+  // first iterations would sleep a tiny bit more instead, which would not
+  // change the calculations significantly.
+  while (thread_present_in_procfs) {
+    struct stat task_stat;
+    const int fstat_ret =
+        fstatat(proc_self_task, thread_id_dir_str.c_str(), &task_stat, 0);
+    if (fstat_ret < 0) {
+      PCHECK(ENOENT == errno);
+      // The thread disappeared from /proc, we're done.
+      thread_present_in_procfs = false;
+      break;
+    }
+    // Increase the waiting time exponentially.
+    struct timespec ts = {0, 1L << iterations /* nanoseconds */};
+    PCHECK(0 == HANDLE_EINTR(nanosleep(&ts, &ts)));
+    ++iterations;
+
+    // Crash after 30 iterations, which means having spent roughly 2s in
+    // nanosleep(2) cumulatively.
+    CHECK_GT(30U, iterations);
+    // In practice, this never goes through more than a couple iterations. In
+    // debug mode, crash after 64ms (+ eventually 25 times the granularity of
+    // the clock) in nanosleep(2).
+    DCHECK_GT(25U, iterations);
+  }
+
+  return true;
+}
+
+}  // namespace sandbox
diff --git a/sandbox/linux/services/thread_helpers.h b/sandbox/linux/services/thread_helpers.h
new file mode 100644
index 0000000..651e5d9
--- /dev/null
+++ b/sandbox/linux/services/thread_helpers.h
@@ -0,0 +1,33 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SANDBOX_LINUX_SERVICES_THREAD_HELPERS_H_
+#define SANDBOX_LINUX_SERVICES_THREAD_HELPERS_H_
+
+#include "base/basictypes.h"
+
+namespace base { class Thread; }
+
+namespace sandbox {
+
+class ThreadHelpers {
+ public:
+  // Check whether the current process is single threaded. |proc_self_tasks|
+  // should be a file descriptor to /proc/self/task/ and remains owned by the
+  // caller.
+  static bool IsSingleThreaded(int proc_self_task);
+
+  // 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_tasks,
+                                       base::Thread* thread);
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(ThreadHelpers);
+};
+
+}  // namespace content
+
+#endif  // SANDBOX_LINUX_SERVICES_THREAD_HELPERS_H_
diff --git a/sandbox/linux/services/thread_helpers_unittests.cc b/sandbox/linux/services/thread_helpers_unittests.cc
new file mode 100644
index 0000000..725a62e
--- /dev/null
+++ b/sandbox/linux/services/thread_helpers_unittests.cc
@@ -0,0 +1,93 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sandbox/linux/services/thread_helpers.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/process/process_metrics.h"
+#include "base/threading/platform_thread.h"
+#include "base/threading/thread.h"
+#include "sandbox/linux/tests/unit_tests.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using base::PlatformThread;
+
+namespace sandbox {
+
+namespace {
+
+int GetRaceTestIterations() {
+  if (IsRunningOnValgrind()) {
+    return 2;
+  } else {
+    return 1000;
+  }
+}
+
+class ScopedProcSelfTask {
+ public:
+  ScopedProcSelfTask() : fd_(-1) {
+    fd_ = open("/proc/self/task/", O_RDONLY | O_DIRECTORY);
+    CHECK_LE(0, fd_);
+  }
+
+  ~ScopedProcSelfTask() { PCHECK(0 == IGNORE_EINTR(close(fd_))); }
+
+  int fd() { return fd_; }
+
+ private:
+  int fd_;
+  DISALLOW_COPY_AND_ASSIGN(ScopedProcSelfTask);
+};
+
+TEST(ThreadHelpers, IsSingleThreadedBasic) {
+  ScopedProcSelfTask task;
+  ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(task.fd()));
+
+  base::Thread thread("sandbox_tests");
+  ASSERT_TRUE(thread.Start());
+  ASSERT_FALSE(ThreadHelpers::IsSingleThreaded(task.fd()));
+}
+
+TEST(ThreadHelpers, IsSingleThreadedIterated) {
+  ScopedProcSelfTask task;
+  ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(task.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()));
+  }
+}
+
+TEST(ThreadHelpers, IsSingleThreadedStartAndStop) {
+  ScopedProcSelfTask task;
+  ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(task.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_TRUE(ThreadHelpers::StopThreadAndWatchProcFS(task.fd(), &thread));
+    ASSERT_TRUE(ThreadHelpers::IsSingleThreaded(task.fd()));
+    ASSERT_EQ(1, base::GetNumberOfThreads(base::GetCurrentProcessHandle()));
+  }
+}
+
+}  // namespace
+
+}  // namespace sandbox