Set a new TLS when using CLONE_VM.

On some libcs, clone writes to the new child's TLS before returning, so
we set a new TLS to avoid corrupting the parent process's TLS.

On glibc, this caused an assertion failure inside of
pthread_getattr_np(pthread_self(), &attr).

This is a reland of https://codereview.chromium.org/1248673004, which
was reverted due to ASAN failures.

BUG=512623

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

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

2 files changed, 52 insertions, 3 deletions

diff --git a/sandbox/linux/services/credentials.cc b/sandbox/linux/services/credentials.cc
index dc62f1b..dd26472 100644
--- a/sandbox/linux/services/credentials.cc
+++ b/sandbox/linux/services/credentials.cc
@@ -53,7 +53,16 @@ bool GetRESIds(uid_t* resuid, gid_t* resgid) {
 
 const int kExitSuccess = 0;
 
+#if defined(__clang__)
+// Disable sanitizers that rely on TLS and may write to non-stack memory.
+__attribute__((no_sanitize_address))
+__attribute__((no_sanitize_thread))
+__attribute__((no_sanitize_memory))
+#endif
 int ChrootToSelfFdinfo(void*) {
+  // This function can be run from a vforked child, so it should not write to
+  // any memory other than the stack or errno. Reads from TLS may be different
+  // from in the parent process.
   RAW_CHECK(sys_chroot("/proc/self/fdinfo/") == 0);
 
   // CWD is essentially an implicit file descriptor, so be careful to not
@@ -91,9 +100,22 @@ bool ChrootToSafeEmptyDir() {
 #error "Unsupported architecture"
 #endif
 
-  pid = clone(ChrootToSelfFdinfo, stack,
-              CLONE_VM | CLONE_VFORK | CLONE_FS | LINUX_SIGCHLD, nullptr,
-              nullptr, nullptr, nullptr);
+  int clone_flags = CLONE_FS | LINUX_SIGCHLD;
+  void* tls = nullptr;
+#if defined(ARCH_CPU_X86_64) || defined(ARCH_CPU_ARM_FAMILY)
+  // Use CLONE_VM | CLONE_VFORK as an optimization to avoid copying page tables.
+  // Since clone writes to the new child's TLS before returning, we must set a
+  // new TLS to avoid corrupting the current process's TLS. On ARCH_CPU_X86,
+  // glibc performs syscalls by calling a function pointer in TLS, so we do not
+  // attempt this optimization.
+  clone_flags |= CLONE_VM | CLONE_VFORK | CLONE_SETTLS;
+
+  char tls_buf[PTHREAD_STACK_MIN] = {0};
+  tls = tls_buf;
+#endif
+
+  pid = clone(ChrootToSelfFdinfo, stack, clone_flags, nullptr, nullptr, tls,
+              nullptr);
   PCHECK(pid != -1);
 
   int status = -1;
diff --git a/sandbox/linux/services/credentials_unittest.cc b/sandbox/linux/services/credentials_unittest.cc
index 6b93c86..39fd6a7 100644
--- a/sandbox/linux/services/credentials_unittest.cc
+++ b/sandbox/linux/services/credentials_unittest.cc
@@ -6,6 +6,8 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <pthread.h>
+#include <signal.h>
 #include <stdio.h>
 #include <sys/capability.h>
 #include <sys/stat.h>
@@ -237,6 +239,31 @@ SANDBOX_TEST(Credentials, SetCapabilitiesMatchesLibCap2) {
   CHECK_EQ(0, cap_compare(expected_cap.get(), actual_cap.get()));
 }
 
+volatile sig_atomic_t signal_handler_called;
+void SignalHandler(int sig) {
+  signal_handler_called = 1;
+}
+
+// Disabled on ASAN because of crbug.com/451603.
+SANDBOX_TEST(Credentials, DISABLE_ON_ASAN(DropFileSystemAccessPreservesTLS)) {
+  // Probably missing kernel support.
+  if (!Credentials::MoveToNewUserNS()) return;
+  CHECK(Credentials::DropFileSystemAccess(ProcUtil::OpenProc().get()));
+
+  // In glibc, pthread_getattr_np makes an assertion about the cached PID/TID in
+  // TLS.
+  pthread_attr_t attr;
+  EXPECT_EQ(0, pthread_getattr_np(pthread_self(), &attr));
+
+  // raise also uses the cached TID in glibc.
+  struct sigaction action = {};
+  action.sa_handler = &SignalHandler;
+  PCHECK(sigaction(SIGUSR1, &action, nullptr) == 0);
+
+  PCHECK(raise(SIGUSR1) == 0);
+  CHECK_EQ(1, signal_handler_called);
+}
+
 }  // namespace.
 
 }  // namespace sandbox.