Set a new TLS when using CLONE_VM.

sandbox/linux/services/credentials.cc

 // Copyright (c) 2013 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/credentials.h"
 
 #include <errno.h>
 #include <signal.h>
 #include <stdint.h>
 #include <stdio.h>
@@ skipping 35 matching lines @@
   const bool uids_are_equal = (ruid == euid) && (ruid == suid);
   const bool gids_are_equal = (rgid == egid) && (rgid == sgid);
   if (!uids_are_equal || !gids_are_equal) return false;
   if (resuid) *resuid = euid;
   if (resgid) *resgid = egid;
   return true;
 }
 
 const int kExitSuccess = 0;
 
 int ChrootToSelfFdinfo(void*) {

[jln (very slow on Chromium), 2015/07/21 23:39:17]

Maybe add a comment here saying that this function needs to be very low level as
it's running from a vfork-ed child.

[rickyz (no longer on Chrome), 2015/07/22 04:00:32]

Done.

   RAW_CHECK(sys_chroot("/proc/self/fdinfo/") == 0);
 
   // CWD is essentially an implicit file descriptor, so be careful to not
   // leave it behind.
   RAW_CHECK(chdir("/") == 0);
   _exit(kExitSuccess);
 }
 
 // chroot() to an empty dir that is "safe". To be safe, it must not contain
 // any subdirectory (chroot-ing there would allow a chroot escape) and it must
@@ skipping 17 matching lines @@
   pid_t pid = -1;
   char stack_buf[PTHREAD_STACK_MIN];
 #if defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARM_FAMILY) || \
     defined(ARCH_CPU_MIPS64_FAMILY) || defined(ARCH_CPU_MIPS_FAMILY)
   // The stack grows downward.
   void* stack = stack_buf + sizeof(stack_buf);
 #else
 #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_MIPS64_FAMILY) || \
+    defined(ARCH_CPU_MIPS_FAMILY)

[jln (very slow on Chromium), 2015/07/22 00:04:20]

Let's just exclude MIPS here. We've not tested there and there is no need to add
this complexity on that architecture.

[rickyz (no longer on Chrome), 2015/07/22 04:00:32]

Done - also noticed that I forgot to add ARM in this list.

+  // 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;
+
+  // The stack grows down on these architectures. Place the TLS at the bottom of

[jln (very slow on Chromium), 2015/07/21 23:39:17]

"bottom of the stack" is very confusing here. I would say "top of the stack" and
maybe qualify with "(lowest address of the stack)".

I wonder if we shouldn't simply have another buffer though. And initialize it to
all-zero.

I'm worried we could get inconsistent behavior if something called from
ChrootToSelfFdinfo starts accessing the TCB / TLS. At the very least we should
fail consistently.

[rickyz (no longer on Chrome), 2015/07/22 04:00:32]

Yeah, I ended up switching this to a zero-initialized buffer - definitely better
than reading uninit memory. Unfortunately, there isn't much guidance given on
minimum TLS size. PTHREAD_STACK_MIN should always be more than sufficient for
glibc's TLS usage.

+  // the stack.
+  tls = stack_buf;
+#endif
+
+  pid = clone(ChrootToSelfFdinfo, stack, clone_flags, nullptr, nullptr, tls,
+              nullptr);
   PCHECK(pid != -1);
 
   int status = -1;
   PCHECK(HANDLE_EINTR(waitpid(pid, &status, 0)) == pid);
 
   return WIFEXITED(status) && WEXITSTATUS(status) == kExitSuccess;
 }
 
 // CHECK() that an attempt to move to a new user namespace raised an expected
 // errno.
@@ skipping 194 matching lines @@
   if (pid != 0) {
     return pid;
   }
 
   // Since we just forked, we are single threaded.
   PCHECK(DropAllCapabilitiesOnCurrentThread());
   return 0;
 }
 
 }  // namespace sandbox.