setjmp/longjmp: avoid invalid values in the stack pointer.
arm64 was already being careful, but x86/x86-64 and 32-bit ARM could be
caught by a signal in a state where the stack pointer was mangled.
For 32-bit ARM I've taken care with the link register too, to avoid
potential issues with unwinding.
Bug: http://b/152210274
Test: treehugger
Change-Id: I1ce285b017a633c732dbe04743368f4cae27af85
diff --git a/libc/arch-arm/bionic/setjmp.S b/libc/arch-arm/bionic/setjmp.S
index 5fbcaf3..2579143 100644
--- a/libc/arch-arm/bionic/setjmp.S
+++ b/libc/arch-arm/bionic/setjmp.S
@@ -87,28 +87,6 @@
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 @@
// 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 @@
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 @@
.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 @@
// 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 351516f..d2fafdb 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 @@
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 @@
// 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 1e1ce58..1a3eb4b 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 @@
// 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 @@
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 1d34561..ba3f05f 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 @@
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 @@
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 e6b6819..4b1482a 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 @@
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
+}