Set a new TLS when using CLONE_VM.

sandbox/linux/services/credentials.cc

Index: sandbox/linux/services/credentials.cc
diff --git a/sandbox/linux/services/credentials.cc b/sandbox/linux/services/credentials.cc
index dc62f1b21392b05012a6dda3a24b50be97bacf91..d5f10233bc9649e22d69d4418de95a8234d23315 100644
--- a/sandbox/linux/services/credentials.cc
+++ b/sandbox/linux/services/credentials.cc
@@ -91,9 +91,24 @@ 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_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;