Skip to content

Commit

Permalink
Syscall: Fix Syscall::Call's X86-64 implementation for CFI unwinding
Browse files Browse the repository at this point in the history
The LEA instruction within the inline assembly statement was throwing
off glibc's backtrace() function, because it lost track of where the
stack was.  The easy fix for this is to convert SyscallAsm() to simply
use the standard C calling convention on X86-64, and make it into a
normal C function call so the compiler can ensure CFI information is
correct for us.

While here, there's no need to use the "call/pop/addq" trick to
compute a PC-relative address because we have %rip-based addressing.
So simply use "lea 2f(%rip), %rax" to compute the return address (and
avoid branch mispredictions from desync'ing the call stack).

BUG=424973

Review URL: https://codereview.chromium.org/661393004

Cr-Commit-Position: refs/heads/master@{#300374}
  • Loading branch information
mdempsky authored and Commit bot committed Oct 20, 2014
1 parent dddb7ae commit 77a224a
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 40 deletions.
3 changes: 0 additions & 3 deletions sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -213,15 +213,12 @@ BPF_TEST_C(BaselinePolicy, EPERM_getcwd, BaselinePolicy) {
BPF_ASSERT_EQ(EPERM, errno);
}

// TODO(jorgelo): re-enable this after crbug.com/424973 is fixed.
#if !defined(OS_CHROMEOS)
BPF_DEATH_TEST_C(BaselinePolicy,
SIGSYS_InvalidSyscall,
DEATH_SEGV_MESSAGE(GetErrorMessageContentForTests()),
BaselinePolicy) {
Syscall::InvalidCall();
}
#endif

// A failing test using this macro could be problematic since we perform
// system calls by passing "0" as every argument.
Expand Down
59 changes: 22 additions & 37 deletions sandbox/linux/seccomp-bpf/syscall.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,28 +89,28 @@ asm(// We need to be able to tell the kernel exactly where we made a
".align 16, 0x90\n"
".type SyscallAsm, @function\n"
"SyscallAsm:.cfi_startproc\n"
// Check if "%rax" is negative. If so, do not attempt to make a
// Check if "%rdi" is negative. If so, do not attempt to make a
// system call. Instead, compute the return address that is visible
// to the kernel after we execute "syscall". This address can be
// used as a marker that BPF code inspects.
"test %rax, %rax\n"
"test %rdi, %rdi\n"
"jge 1f\n"
// Always make sure that our code is position-independent, or the
// linker will throw a hissy fit on x86-64.
"call 0f; .cfi_adjust_cfa_offset 8\n"
"0:pop %rax; .cfi_adjust_cfa_offset -8\n"
"addq $2f-0b, %rax\n"
"lea 2f(%rip), %rax\n"
"ret\n"
// We declared all clobbered registers to the compiler. On x86-64,
// there really isn't much of a problem with register pressure. So,
// we can go ahead and directly copy the entries from the arguments
// array into the appropriate CPU registers.
"1:movq 0(%r12), %rdi\n"
"movq 8(%r12), %rsi\n"
"movq 16(%r12), %rdx\n"
"movq 24(%r12), %r10\n"
"movq 32(%r12), %r8\n"
"movq 40(%r12), %r9\n"
// Now we load the registers used to pass arguments to the system
// call: system call number in %rax, and arguments in %rdi, %rsi,
// %rdx, %r10, %r8, %r9. Note: These are all caller-save registers
// (only %rbx, %rbp, %rsp, and %r12-%r15 are callee-save), so no
// need to worry here about spilling registers or CFI directives.
"1:movq %rdi, %rax\n"
"movq 0(%rsi), %rdi\n"
"movq 16(%rsi), %rdx\n"
"movq 24(%rsi), %r10\n"
"movq 32(%rsi), %r8\n"
"movq 40(%rsi), %r9\n"
"movq 8(%rsi), %rsi\n"
// Enter the kernel.
"syscall\n"
// This is our "magic" return address that the BPF filter sees.
Expand Down Expand Up @@ -250,6 +250,12 @@ asm(// We need to be able to tell the kernel exactly where we made a
#endif
); // asm

#if defined(__x86_64__)
extern "C" {
intptr_t SyscallAsm(intptr_t nr, const intptr_t args[6]);
}
#endif

} // namespace

intptr_t Syscall::InvalidCall() {
Expand Down Expand Up @@ -300,28 +306,7 @@ intptr_t Syscall::Call(int nr,
: "0"(ret), "D"(args)
: "cc", "esp", "memory", "ecx", "edx");
#elif defined(__x86_64__)
intptr_t ret = nr;
{
register const intptr_t* data __asm__("r12") = args;
asm volatile(
"lea -128(%%rsp), %%rsp\n" // Avoid red zone.
"call SyscallAsm\n"
"lea 128(%%rsp), %%rsp\n"
// N.B. These are not the calling conventions normally used by the ABI.
: "=a"(ret)
: "0"(ret), "r"(data)
: "cc",
"rsp",
"memory",
"rcx",
"rdi",
"rsi",
"rdx",
"r8",
"r9",
"r10",
"r11");
}
intptr_t ret = SyscallAsm(nr, args);
#elif defined(__arm__)
intptr_t ret;
{
Expand Down

0 comments on commit 77a224a

Please sign in to comment.