Prevent FPU state corruption by directly creating the signal frame on the

stack of the newly created thread, instead of creating it on the caller's
stack and copying it over. This eliminates the need to do complicated touch-ups
of the signal stack's data structure, which turned out to be incorrect for
the FPU state. Thanks to Mark Seaborn for pointing out this simplification of
the code.

TEST=Chrome no longer crashes in tcmalloc
BUG=none
Review URL: http://codereview.chromium.org/2051005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46928 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 56 insertions, 69 deletions

diff --git a/sandbox/linux/seccomp/clone.cc b/sandbox/linux/seccomp/clone.cc
index 957ae2c..0d35181 100644
--- a/sandbox/linux/seccomp/clone.cc
+++ b/sandbox/linux/seccomp/clone.cc
@@ -63,18 +63,29 @@ long Sandbox::sandbox_clone(int flags, char* stack, int* pid, int* ctid,
     // developers treat the initial part of the stack frame as a stable part
     // of the ABI. So, we can rely on fixed, well-defined offsets for accessing
     // register values and for accessing the signal mask.
-    #if defined(__x86_64__) || defined(__i386__)
     #if defined(__x86_64__)
     // Red zone compensation. The instrumented system call will remove 128
     // bytes from the thread's stack prior to returning to the original
     // call site.
     stack                   -= 128;
     request.clone_req.stack  = stack;
-    #endif
-    asm("int $0"
-        : "=m"(request.clone_req.stack)
-        : "a"(__NR_clone + 0xF000), "d"(&request.clone_req.stack)
-        : "memory");
+    void *dummy;
+    asm volatile("mov %%rsp, %%rcx\n"
+                 "mov %3, %%rsp\n"
+                 "int $0\n"
+                 "mov %%rcx, %%rsp\n"
+                 : "=a"(request.clone_req.stack), "=&c"(dummy)
+                 : "a"(__NR_clone + 0xF000), "m"(request.clone_req.stack)
+                 : "memory");
+    #elif defined(__i386__)
+    void *dummy;
+    asm volatile("mov %%esp, %%ecx\n"
+                 "mov %3, %%esp\n"
+                 "int $0\n"
+                 "mov %%ecx, %%esp\n"
+                 : "=a"(request.clone_req.stack), "=&c"(dummy)
+                 : "a"(__NR_clone + 0xF000), "m"(request.clone_req.stack)
+                 : "memory");
     #else
     #error Unsupported target platform
     #endif
diff --git a/sandbox/linux/seccomp/sandbox.cc b/sandbox/linux/seccomp/sandbox.cc
index 93ce12e..32480b3 100644
--- a/sandbox/linux/seccomp/sandbox.cc
+++ b/sandbox/linux/seccomp/sandbox.cc
@@ -252,16 +252,8 @@ void (*Sandbox::segv())(int signo, SysCalls::siginfo *context, void *unused) {
       // Copy signal frame onto new stack. See clone.cc for details
    "14:cmp  $56+0xF000, %%rax\n"   // NR_clone + 0xF000
       "jnz  15f\n"
-      "mov  0xA8(%%rsp), %%rcx\n"  // %rsp at time of segmentation fault
-      "sub  %%rsp, %%rcx\n"        // %rcx = size of stack frame
-      "sub  $8, %%rcx\n"           // skip return address
-      "mov  %%rcx, %%rax\n"        // return size of signal stack frame
-      "mov  0(%%rdx), %%rdi\n"     // stack for newly clone()'d thread
-      "sub  %%rcx, %%rdi\n"        // copy onto new stack
-      "mov  %%rdi, 0(%%rdx)\n"     // allocate space on new stack
-      "lea  8(%%rsp), %%rsi\n"     // copy from current stack
-      "cld\n"
-      "rep  movsb\n"
+      "lea  8(%%rsp), %%rax\n"     // retain stack frame upon returning
+      "mov  %%rax, 0xA8(%%rsp)\n"  // %rsp at time of segmentation fault
       "jmp  7b\n"
 
       // Forward system call to syscallWrapper()
@@ -441,32 +433,9 @@ void (*Sandbox::segv())(int signo, SysCalls::siginfo *context, void *unused) {
       // See clone.cc for details
    "18:cmp  $120+0xF000, %%eax\n"  // NR_clone + 0xF000
       "jnz  19f\n"
-      "mov  0xC0(%%esp), %%ecx\n"  // %esp at time of segmentation fault
-      "sub  %%esp, %%ecx\n"        // %ecx = size of RT stack frame
-      "mov  %%ecx, %%eax\n"
-      "add  $0x1C8, %%eax\n"       // adjust for size of legacy stack frame
-      "sub  $0x100, %%ecx\n"
-      "mov  0(%%edx), %%edi\n"     // stack for newly clone()'d thread
-      "sub  %%ecx, %%edi\n"        // copy onto new stack
-      "lea  0x100(%%esp), %%esi\n"
-      "cld\n"
-      "rep movsb\n"                // copy parts of RT stack(sigmask, FP state)
-      "mov  0xF0(%%esp), %%ebx\n"  // adjust pointer to fpstate
-      "sub  %%esi, %%ebx\n"
-      "add  %%edi, %%ebx\n"
-      "sub  %%eax, %%edi\n"
-      "mov  %%edi, 0(%%edx)\n"     // allocate space on new stack
-      "lea  0xA4(%%esp), %%esi\n"  // copy sigcontext from current stack
-      "mov  $0x16, %%ecx\n"
-      "rep  movsl\n"
-      "mov  %%ebx, -0xC(%%edi)\n"  // set pointer to fpstate
-      "mov  0xFC(%%esp), %%ebx\n"  // copy first half of signal mask
-      "mov  %%ebx, -0x8(%%edi)\n"
-      "mov  %%eax, -0x2C(%%edi)\n" // return size of stack frame in %%eax
-      "addl $2, -0x20(%%edi)\n"    // adjust %eip
-      "mov  0(%%edx), %%esp\n"
-      "mov  $119, %%eax\n"         // NR_sigreturn
-      "int  $0x80\n"
+      "lea  -0x1C8(%%esp), %%eax\n"// retain stack frame upon returning
+      "mov  %%eax, 0xC0(%%esp)\n"  // %esp at time of segmentation fault
+      "jmp  3b\n"
 
       // Forward system call to syscallWrapper()
    "19:call playground$syscallWrapper\n"
diff --git a/sandbox/linux/seccomp/tests/test_syscalls.cc b/sandbox/linux/seccomp/tests/test_syscalls.cc
index bb6e6f7..f68374a 100644
--- a/sandbox/linux/seccomp/tests/test_syscalls.cc
+++ b/sandbox/linux/seccomp/tests/test_syscalls.cc
@@ -161,6 +161,23 @@ TEST(test_clone_disallowed_flags) {
   assert(errno == EPERM);
 }
 
+void *fp_thread(void *x) {
+  int val;
+  asm("movss %%xmm0, %0" : "=m"(val));
+  printf("val=%i\n", val);
+  return NULL;
+}
+
+TEST(test_fp_regs) {
+  StartSeccompSandbox();
+  int val = 1234;
+  asm("movss %0, %%xmm0" : "=m"(val));
+  pthread_t tid;
+  pthread_create(&tid, NULL, fp_thread, NULL);
+  pthread_join(tid, NULL);
+  printf("thread done OK\n");
+}
+
 long long read_tsc() {
   long long rc;
   asm volatile(
diff --git a/sandbox/linux/seccomp/trusted_thread.cc b/sandbox/linux/seccomp/trusted_thread.cc
index 240e65f..5819b0a 100644
--- a/sandbox/linux/seccomp/trusted_thread.cc
+++ b/sandbox/linux/seccomp/trusted_thread.cc
@@ -45,14 +45,12 @@ void Sandbox::createTrustedThread(int processFdPub, int cloneFdPub,
       // newly created thread's stacks prior to cloning. See clone.cc for
       // details.
       "mov  $56+0xF000, %%eax\n"   // __NR_clone + 0xF000
-      "sub  $8, %%rsp\n"
-      "mov  %%rsp, %%rdx\n"        // push a signal stack frame (see clone.cc)
-      "mov  %%rsp, 0(%%rsp)\n"
-      "int  $0\n"
-      "mov  0(%%rsp), %%r9\n"
-      "add  $8, 0xA0(%%r9)\n"      // pop stack upon call to sigreturn()
+      "mov  %%rsp, %%rcx\n"
+      "int  $0\n"                  // push a signal stack frame (see clone.cc)
+      "mov  %%rcx, 0xA0(%%rsp)\n"  // pop stack upon call to sigreturn()
+      "mov  %%rsp, %%r9\n"
       "mov  $2, %%rdi\n"           // how     = SIG_SETMASK
-      "movq $-1, 0(%%rsp)\n"
+      "pushq $-1\n"
       "mov  %%rsp, %%rsi\n"        // set     = full mask
       "xor  %%rdx, %%rdx\n"        // old_set = NULL
       "mov  $8, %%r10\n"           // mask all 64 signals
@@ -699,7 +697,6 @@ void Sandbox::createTrustedThread(int processFdPub, int cloneFdPub,
       "pop  %%rax\n"
 
       // Return to caller. We are in the new thread, now.
-      "xor  %%rax, %%rax\n"
       "test %%r15, %%r15\n"
       "jnz  34f\n"                 // Returning to createTrustedThread()
 
@@ -745,11 +742,6 @@ void Sandbox::createTrustedThread(int processFdPub, int cloneFdPub,
   asm volatile(
       "push %%ebx\n"
       "push %%ebp\n"
-      "movd %0, %%mm6\n"           // %mm6 = args
-      "lea  999f, %%ebx\n"         // continue in same thread
-      "movd %%ebx, %%mm3\n"
-      "xor  %%edi, %%edi\n"        // initial sequence number
-      "movd %%edi, %%mm2\n"
 
       // Signal handlers are process-wide. This means that for security
       // reasons, we cannot allow that the trusted thread ever executes any
@@ -773,16 +765,15 @@ void Sandbox::createTrustedThread(int processFdPub, int cloneFdPub,
       // through the same steps of creating a signal stack frame on the
       // newly created thread's stacks prior to cloning. See clone.cc for
       // details.
+      "mov  %0, %%edi\n"           // create signal stack before accessing MMX
       "mov  $120+0xF000, %%eax\n"  // __NR_clone + 0xF000
-      "sub  $8, %%esp\n"
-      "mov  %%esp, %%edx\n"        // push a signal stack frame (see clone.cc)
-      "mov  %%esp, 0(%%esp)\n"
-      "int  $0\n"
-      "mov  0(%%esp), %%ebp\n"
-      "add  $8, 0x1C(%%ebp)\n"     // pop stack upon call to sigreturn()
+      "mov  %%esp, %%ebp\n"
+      "int  $0\n"                  // push a signal stack frame (see clone.cc)
+      "mov  %%ebp, 0x1C(%%esp)\n"  // pop stack upon call to sigreturn()
+      "mov  %%esp, %%ebp\n"
       "mov  $2, %%ebx\n"           // how     = SIG_SETMASK
-      "movl $-1, 0(%%esp)\n"
-      "movl $-1, 4(%%esp)\n"
+      "pushl $-1\n"
+      "pushl $-1\n"
       "mov  %%esp, %%ecx\n"        // set     = full mask
       "xor  %%edx, %%edx\n"        // old_set = NULL
       "mov  $8, %%esi\n"           // mask all 64 signals
@@ -791,6 +782,11 @@ void Sandbox::createTrustedThread(int processFdPub, int cloneFdPub,
       "mov  $126, %%eax\n"         // NR_sigprocmask
       "int  $0x80\n"
       "xor  %%esp, %%esp\n"        // invalidate the stack in all trusted code
+      "movd %%edi, %%mm6\n"        // %mm6 = args
+      "lea  999f, %%edi\n"         // continue in same thread
+      "movd %%edi, %%mm3\n"
+      "xor  %%edi, %%edi\n"        // initial sequence number
+      "movd %%edi, %%mm2\n"
       "jmp  20f\n"                 // create trusted thread
 
       // TODO(markus): Coalesce the read() operations by reading into a bigger
@@ -1452,13 +1448,7 @@ void Sandbox::createTrustedThread(int processFdPub, int cloneFdPub,
       "pop  %%eax\n"
 
       // Return to caller. We are in the new thread, now.
-      "xor  %%eax, %%eax\n"
       "movd %%mm3, %%ebx\n"
-
-      // Release MMX registers, so that they can be used for floating point
-      // operations.
-      "emms\n"
-
       "test %%ebx, %%ebx\n"
       "jnz  35f\n"                 // Returning to createTrustedThread()