Merge "setjmp/longjmp: avoid invalid values in the stack pointer." am: e71143e8c0 am: 0a8e61bcb0

Original change: https://android-review.googlesource.com/c/platform/bionic/+/1663232

Change-Id: I288904abfd061a53411286a212b2ebc1dcdec24a

5 files changed, 151 insertions, 110 deletions

diff --git a/libc/arch-arm/bionic/setjmp.S b/libc/arch-arm/bionic/setjmp.S
index 5fbcaf39f..2579143f6 100644
--- a/libc/arch-arm/bionic/setjmp.S
+++ b/libc/arch-arm/bionic/setjmp.S
@@ -87,28 +87,6 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(_setjmp)
   b sigsetjmp
 END(_setjmp)
 
-#define MANGLE_REGISTERS 1
-#define USE_CHECKSUM 1
-
-.macro m_mangle_registers reg
-#if MANGLE_REGISTERS
-  eor r4, r4, \reg
-  eor r5, r5, \reg
-  eor r6, r6, \reg
-  eor r7, r7, \reg
-  eor r8, r8, \reg
-  eor r9, r9, \reg
-  eor r10, r10, \reg
-  eor r11, r11, \reg
-  eor r13, r13, \reg
-  eor r14, r14, \reg
-#endif
-.endm
-
-.macro m_unmangle_registers reg
-  m_mangle_registers \reg
-.endm
-
 .macro m_calculate_checksum dst, src, scratch
   mov \dst, #0
   .irp i,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28
@@ -167,12 +145,30 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(sigsetjmp)
 
   // Save core registers.
   add r1, r0, #(_JB_CORE_BASE * 4)
-  m_mangle_registers r2
-
-  // ARM deprecates using sp in the register list for stmia.
-  stmia r1, {r4-r11, lr}
-  str sp, [r1, #(9 * 4)]
-  m_unmangle_registers r2
+  // Mangle the easy registers in-place, write them out in one go, and unmangle
+  // them again.
+  eor r4, r4, r2
+  eor r5, r5, r2
+  eor r6, r6, r2
+  eor r7, r7, r2
+  eor r8, r8, r2
+  eor r9, r9, r2
+  eor r10, r10, r2
+  eor r11, r11, r2
+  stmia r1, {r4-r11}
+  eor r4, r4, r2
+  eor r5, r5, r2
+  eor r6, r6, r2
+  eor r7, r7, r2
+  eor r8, r8, r2
+  eor r9, r9, r2
+  eor r10, r10, r2
+  eor r11, r11, r2
+  // We need to avoid invalid values in sp or lr (http://b/152210274).
+  eor r3, lr, r2
+  str r3, [r1, #(8 * 4)]
+  eor r3, sp, r2
+  str r3, [r1, #(9 * 4)]
 
   // Save floating-point registers.
   add r1, r0, #(_JB_FLOAT_BASE * 4)
@@ -182,11 +178,9 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(sigsetjmp)
   fmrx r1, fpscr
   str r1, [r0, #(_JB_FLOAT_STATE * 4)]
 
-#if USE_CHECKSUM
   // Calculate the checksum.
   m_calculate_checksum r12, r0, r2
   str r12, [r0, #(_JB_CHECKSUM * 4)]
-#endif
 
   mov r0, #0
   bx lr
@@ -201,14 +195,11 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(siglongjmp)
   .cfi_rel_offset r1, 4
   .cfi_rel_offset lr, 8
 
-#if USE_CHECKSUM
   // Check the checksum before doing anything.
   m_calculate_checksum r12, r0, r3
   ldr r2, [r0, #(_JB_CHECKSUM * 4)]
-
   teq r2, r12
   bne __bionic_setjmp_checksum_mismatch
-#endif
 
   // Fetch the signal flag.
   ldr r1, [r0, #(_JB_SIGFLAG * 4)]
@@ -245,10 +236,21 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(siglongjmp)
   // Restore core registers.
   add r2, r0, #(_JB_CORE_BASE * 4)
 
-  // ARM deprecates using sp in the register list for ldmia.
-  ldmia r2, {r4-r11, lr}
-  ldr sp, [r2, #(9 * 4)]
-  m_unmangle_registers r3
+  // Do all the easy registers in one go.
+  ldmia r2, {r4-r11}
+  eor r4, r4, r3
+  eor r5, r5, r3
+  eor r6, r6, r3
+  eor r7, r7, r3
+  eor r8, r8, r3
+  eor r9, r9, r3
+  eor r10, r10, r3
+  eor r11, r11, r3
+  // We need to avoid invalid values in sp or lr (http://b/152210274).
+  ldr r0, [r2, #(8 * 4)]
+  eor lr, r0, r3
+  ldr r0, [r2, #(9 * 4)]
+  eor sp, r0, r3
 
   // Save the return value/address and check the setjmp cookie.
   stmfd sp!, {r1, lr}
diff --git a/libc/arch-arm64/bionic/setjmp.S b/libc/arch-arm64/bionic/setjmp.S
index 351516fa0..d2fafdbe0 100644
--- a/libc/arch-arm64/bionic/setjmp.S
+++ b/libc/arch-arm64/bionic/setjmp.S
@@ -69,11 +69,8 @@
 #define _JB_CHECKSUM    (_JB_D8_D9 + 2)
 
 #define SCS_MASK (SCS_SIZE - 1)
-#define MANGLE_REGISTERS 1
-#define USE_CHECKSUM 1
 
 .macro m_mangle_registers reg, sp_reg
-#if MANGLE_REGISTERS
   eor x3, x3, \reg
   eor x19, x19, \reg
   eor x20, x20, \reg
@@ -88,7 +85,6 @@
   eor x29, x29, \reg
   eor x30, x30, \reg
   eor \sp_reg, \sp_reg, \reg
-#endif
 .endm
 
 .macro m_calculate_checksum dst, src, scratch
@@ -179,11 +175,9 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(sigsetjmp)
   stp d10, d11, [x0, #(_JB_D10_D11 * 8)]
   stp d8,  d9,  [x0, #(_JB_D8_D9   * 8)]
 
-#if USE_CHECKSUM
   // Calculate the checksum.
   m_calculate_checksum x12, x0, x2
   str x12, [x0, #(_JB_CHECKSUM * 8)]
-#endif
 
   mov w0, #0
   autiasp
@@ -194,14 +188,11 @@ END(sigsetjmp)
 // void siglongjmp(sigjmp_buf env, int value);
 ENTRY(siglongjmp)
 __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(siglongjmp)
-#if USE_CHECKSUM
   // Check the checksum before doing anything.
   m_calculate_checksum x12, x0, x2
   ldr x2, [x0, #(_JB_CHECKSUM * 8)]
-
   cmp x2, x12
   bne __bionic_setjmp_checksum_mismatch
-#endif
 
 #if __has_feature(hwaddress_sanitizer)
   stp x0, x30, [sp, #-16]!
diff --git a/libc/arch-x86/bionic/setjmp.S b/libc/arch-x86/bionic/setjmp.S
index 1e1ce58c3..1a3eb4b74 100644
--- a/libc/arch-x86/bionic/setjmp.S
+++ b/libc/arch-x86/bionic/setjmp.S
@@ -57,19 +57,6 @@
 #define _JB_SIGFLAG 8
 #define _JB_CHECKSUM 9
 
-.macro m_mangle_registers reg
-  xorl \reg,%edx
-  xorl \reg,%ebx
-  xorl \reg,%esp
-  xorl \reg,%ebp
-  xorl \reg,%esi
-  xorl \reg,%edi
-.endm
-
-.macro m_unmangle_registers reg
-  m_mangle_registers \reg
-.endm
-
 .macro m_calculate_checksum dst, src
   movl $0, \dst
   .irp i,0,1,2,3,4,5
@@ -129,14 +116,17 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(sigsetjmp)
 
   // Save the callee-save registers.
   movl 0(%esp),%edx
-  m_mangle_registers %eax
-  movl %edx,(_JB_EDX * 4)(%ecx)
-  movl %ebx,(_JB_EBX * 4)(%ecx)
-  movl %esp,(_JB_ESP * 4)(%ecx)
-  movl %ebp,(_JB_EBP * 4)(%ecx)
-  movl %esi,(_JB_ESI * 4)(%ecx)
-  movl %edi,(_JB_EDI * 4)(%ecx)
-  m_unmangle_registers %eax
+
+.macro m_mangle_register reg, offset
+  movl \reg,(\offset * 4)(%ecx)
+  xorl %eax,(\offset * 4)(%ecx)
+.endm
+  m_mangle_register %edx, _JB_EDX
+  m_mangle_register %ebx, _JB_EBX
+  m_mangle_register %esp, _JB_ESP
+  m_mangle_register %ebp, _JB_EBP
+  m_mangle_register %esi, _JB_ESI
+  m_mangle_register %edi, _JB_EDI
 
   m_calculate_checksum %eax, %ecx
   movl %eax, (_JB_CHECKSUM * 4)(%ecx)
@@ -174,18 +164,26 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(siglongjmp)
   movl 4(%esp),%edx
   movl 8(%esp),%eax
 
+  // Fetch the setjmp cookie and clear the signal flag bit.
   movl (_JB_SIGFLAG * 4)(%edx),%ecx
   andl $-2,%ecx
 
+  // Carefully unmangle esp/ebp without ever having an invalid value in the
+  // register (http://b/152210274).
+  movl (_JB_ESP * 4)(%edx),%edi
+  xorl %ecx,%edi
+  movl %edi,%esp
+  movl (_JB_EBP * 4)(%edx),%edi
+  xorl %ecx,%edi
+  movl %edi,%ebp
+
+  // The others don't matter as much, but we do need to finish using the cookie
+  // from %ecx before we clobber it, so we seed each register with the cookie.
   movl %ecx,%ebx
-  movl %ecx,%esp
-  movl %ecx,%ebp
   movl %ecx,%esi
   movl %ecx,%edi
   xorl (_JB_EDX * 4)(%edx),%ecx
   xorl (_JB_EBX * 4)(%edx),%ebx
-  xorl (_JB_ESP * 4)(%edx),%esp
-  xorl (_JB_EBP * 4)(%edx),%ebp
   xorl (_JB_ESI * 4)(%edx),%esi
   xorl (_JB_EDI * 4)(%edx),%edi
 
diff --git a/libc/arch-x86_64/bionic/setjmp.S b/libc/arch-x86_64/bionic/setjmp.S
index 1d345617d..ba3f05f81 100644
--- a/libc/arch-x86_64/bionic/setjmp.S
+++ b/libc/arch-x86_64/bionic/setjmp.S
@@ -64,25 +64,6 @@
 #define _JB_SIGMASK 9
 #define _JB_CHECKSUM 10
 
-#define MANGLE_REGISTERS 1
-
-.macro m_mangle_registers reg
-#if MANGLE_REGISTERS
-  xorq \reg,%rbx
-  xorq \reg,%rbp
-  xorq \reg,%r12
-  xorq \reg,%r13
-  xorq \reg,%r14
-  xorq \reg,%r15
-  xorq \reg,%rsp
-  xorq \reg,%r11
-#endif
-.endm
-
-.macro m_unmangle_registers reg
-  m_mangle_registers \reg
-.endm
-
 .macro m_calculate_checksum dst, src
   movq $0, \dst
   .irp i,0,1,2,3,4,5,6,7
@@ -127,20 +108,26 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(sigsetjmp)
   popq %rdi // Pop 'env'.
 
 2:
-  // Save the callee-save registers.
+  // Fetch the setjmp cookie and clear the signal flag bit.
   popq %rax
   andq $-2,%rax
   movq (%rsp),%r11
-  m_mangle_registers %rax
-  movq %rbx,(_JB_RBX * 8)(%rdi)
-  movq %rbp,(_JB_RBP * 8)(%rdi)
-  movq %r12,(_JB_R12 * 8)(%rdi)
-  movq %r13,(_JB_R13 * 8)(%rdi)
-  movq %r14,(_JB_R14 * 8)(%rdi)
-  movq %r15,(_JB_R15 * 8)(%rdi)
-  movq %rsp,(_JB_RSP * 8)(%rdi)
-  movq %r11,(_JB_PC  * 8)(%rdi)
-  m_unmangle_registers %rax
+
+  // Save the callee-save registers.
+
+.macro m_mangle_register reg, offset
+  movq \reg, (\offset * 8)(%rdi)
+  xorq %rax, (\offset * 8)(%rdi)  // %rax contains the cookie.
+.endm
+
+  m_mangle_register %rbx, _JB_RBX
+  m_mangle_register %rbp, _JB_RBP
+  m_mangle_register %r12, _JB_R12
+  m_mangle_register %r13, _JB_R13
+  m_mangle_register %r14, _JB_R14
+  m_mangle_register %r15, _JB_R15
+  m_mangle_register %rsp, _JB_RSP
+  m_mangle_register %r11, _JB_PC
 
   m_calculate_checksum %rax, %rdi
   movq %rax, (_JB_CHECKSUM * 8)(%rdi)
@@ -179,15 +166,22 @@ __BIONIC_WEAK_ASM_FOR_NATIVE_BRIDGE(siglongjmp)
   popq %rax // Pop 'value'.
 
   // Restore the callee-save registers.
-  movq (_JB_RBX * 8)(%r12),%rbx
-  movq (_JB_RBP * 8)(%r12),%rbp
-  movq (_JB_R13 * 8)(%r12),%r13
-  movq (_JB_R14 * 8)(%r12),%r14
-  movq (_JB_R15 * 8)(%r12),%r15
-  movq (_JB_RSP * 8)(%r12),%rsp
-  movq (_JB_PC  * 8)(%r12),%r11
-  movq (_JB_R12 * 8)(%r12),%r12
-  m_unmangle_registers %rcx
+
+.macro m_unmangle_register reg, offset
+  movq (\offset * 8)(%r12), %rdx  // Clobbers rdx.
+  xorq %rcx, %rdx                 // %rcx contains the cookie.
+  // Now it's safe to overwrite the register (http://b/152210274).
+  movq %rdx, \reg
+.endm
+
+  m_unmangle_register %rbx, _JB_RBX
+  m_unmangle_register %rbp, _JB_RBP
+  m_unmangle_register %r13, _JB_R13
+  m_unmangle_register %r14, _JB_R14
+  m_unmangle_register %r15, _JB_R15
+  m_unmangle_register %rsp, _JB_RSP
+  m_unmangle_register %r11, _JB_PC
+  m_unmangle_register %r12, _JB_R12
 
   // Check the cookie.
   pushq %rax
diff --git a/tests/setjmp_test.cpp b/tests/setjmp_test.cpp
index e6b681968..4b1482ac6 100644
--- a/tests/setjmp_test.cpp
+++ b/tests/setjmp_test.cpp
@@ -18,6 +18,8 @@
 
 #include <setjmp.h>
 #include <stdlib.h>
+#include <sys/syscall.h>
+#include <unistd.h>
 
 #include "BionicDeathTest.h"
 #include "SignalUtils.h"
@@ -268,3 +270,57 @@ TEST(setjmp, setjmp_stack) {
   if (value == 0) call_longjmp(buf);
   EXPECT_EQ(123, value);
 }
+
+TEST(setjmp, bug_152210274) {
+  // Ensure that we never have a mangled value in the stack pointer.
+#if defined(__BIONIC__)
+  struct sigaction sa = {.sa_flags = SA_SIGINFO, .sa_sigaction = [](int, siginfo_t*, void*) {}};
+  ASSERT_EQ(0, sigaction(SIGPROF, &sa, 0));
+
+  constexpr size_t kNumThreads = 20;
+
+  // Start a bunch of threads calling setjmp/longjmp.
+  auto jumper = [](void* arg) -> void* {
+    sigset_t set;
+    sigemptyset(&set);
+    sigaddset(&set, SIGPROF);
+    pthread_sigmask(SIG_UNBLOCK, &set, nullptr);
+
+    jmp_buf buf;
+    for (size_t count = 0; count < 100000; ++count) {
+      if (setjmp(buf) != 0) {
+        perror("setjmp");
+        abort();
+      }
+      if (*static_cast<pid_t*>(arg) == 100) longjmp(buf, 1);
+    }
+    return nullptr;
+  };
+  pid_t tids[kNumThreads] = {};
+  for (size_t i = 0; i < kNumThreads; ++i) {
+    pthread_t t;
+    ASSERT_EQ(0, pthread_create(&t, nullptr, jumper, &tids[i]));
+    tids[i] = pthread_gettid_np(t);
+  }
+
+  // Start the interrupter thread.
+  auto interrupter = [](void* arg) -> void* {
+    pid_t* tids = static_cast<pid_t*>(arg);
+    for (size_t count = 0; count < 1000; ++count) {
+      for (size_t i = 0; i < kNumThreads; i++) {
+        if (tgkill(getpid(), tids[i], SIGPROF) == -1 && errno != ESRCH) {
+          perror("tgkill failed");
+          abort();
+        }
+      }
+      usleep(100);
+    }
+    return nullptr;
+  };
+  pthread_t t;
+  ASSERT_EQ(0, pthread_create(&t, nullptr, interrupter, tids));
+  pthread_join(t, nullptr);
+#else
+  GTEST_LOG_(INFO) << "tests uses functions not in glibc";
+#endif
+}