[HWASan] Intercept setjmp/longjmp on x86_64.

[xiangzhangllvm]

compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S

38

We should insert endbr64 for CET enable program. It has no effect for non-CET program.

50–55

Why not use JB_xxx which defined in the compiler-rt ? That is more readable

// Save callee save registers.
  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)

59

here too

63

here too

65

we 'd better consider Shadow Stack for CET

// Check if Shadow Stack is enabled.
  testl $X86_FEATURE_1_SHSTK, %fs:FEATURE_1_OFFSET
  jz .L_skip_ssp

// Get the current Shadow-Stack-Pointer and save it.
  rdsspq %rax
  movq %rax, SHADOW_STACK_POINTER_OFFSET(%rdi)

// Make a tail call to __sigjmp_save; it takes the same args.
.L_skip_ssp:
  jmp __sigjmp_save

[xiangzhangllvm]

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

68

Here should be no space

[morehouse]

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

68

This spaces are inserted by clang-format. Looks funny here, but not sure I want to override clang-format on every change.

compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S

50–55

I considered doing this, but it seems inline assembly can't use those macros, so we don't get to use them in our longjmp implementation.

So rather than using macros in setjmp and constants in longjmp, I just used constants everywhere.

FWIW, the aarch64 implementation doesn't use macros either.

65

Is there a way I can test CET in QEMU?

I don't want to complicate things without a way to verify it works.

[eugenis]

LGTM modulo the CET discussion

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

68

+1 we do what the computer tells us to do for code formatting

[xiangzhangllvm]

compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S

65

Not sure it can be tested in QEMU, I didn't do that test. but current standard setjmp/longjmp has support it in glibc.

The C/C++ source code can be compiled with CET option, but asm code can not, we'd better consider it.

Or, at least, add a "TODO" here, if you worry about the testing.

[morehouse]

compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S

65

TODO is already present on line 29.

[morehouse]

compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S

65

Other assembly for x86_64 will also need to be adapted for CET (e.g., vfork interceptor).

I propose we wait until we can test properly and then update everything together.

[xiangzhangllvm]

LGTM , thanks for your work for HWASAN !

[uabelho]

Hi @morehouse,

I see warnings like this with this patch:

23:54:36 [297/1283] Building ASM object compiler-rt/lib/hwasan/CMakeFiles/RTHwasan_dynamic.x86_64.dir/hwasan_setjmp_x86_64.S.o
23:54:36 <instantiation>:3:3: warning: __sigsetjmp changed binding to STB_WEAK
23:54:36   .weak __sigsetjmp
23:54:36   ^
23:54:36 <instantiation>:3:3: warning: _setjmp changed binding to STB_WEAK
23:54:36   .weak _setjmp
23:54:36   ^

I use gcc 9.3.0 with glibc 2.17 and linux 3.10.

Is there anything that should/could be done about the warnings?

[morehouse]

In D109790#3011586, @uabelho wrote:

Hi @morehouse,

I see warnings like this with this patch:

23:54:36 [297/1283] Building ASM object compiler-rt/lib/hwasan/CMakeFiles/RTHwasan_dynamic.x86_64.dir/hwasan_setjmp_x86_64.S.o
23:54:36 <instantiation>:3:3: warning: __sigsetjmp changed binding to STB_WEAK
23:54:36   .weak __sigsetjmp
23:54:36   ^
23:54:36 <instantiation>:3:3: warning: _setjmp changed binding to STB_WEAK
23:54:36   .weak _setjmp
23:54:36   ^

I use gcc 9.3.0 with glibc 2.17 and linux 3.10.

Is there anything that should/could be done about the warnings?

I'm unable to reproduce locally, but it looks like we've also had this warning on the aarch64 buildbots for a while.

Does https://reviews.llvm.org/D110178 silence the warning for you?

[uabelho]

In D109790#3012660, @morehouse wrote:

In D109790#3011586, @uabelho wrote:

Hi @morehouse,

I see warnings like this with this patch:

23:54:36 [297/1283] Building ASM object compiler-rt/lib/hwasan/CMakeFiles/RTHwasan_dynamic.x86_64.dir/hwasan_setjmp_x86_64.S.o
23:54:36 <instantiation>:3:3: warning: __sigsetjmp changed binding to STB_WEAK
23:54:36   .weak __sigsetjmp
23:54:36   ^
23:54:36 <instantiation>:3:3: warning: _setjmp changed binding to STB_WEAK
23:54:36   .weak _setjmp
23:54:36   ^

I use gcc 9.3.0 with glibc 2.17 and linux 3.10.

Is there anything that should/could be done about the warnings?

I'm unable to reproduce locally, but it looks like we've also had this warning on the aarch64 buildbots for a while.

Does https://reviews.llvm.org/D110178 silence the warning for you?

Yes I just tested with the patch and the warnings went away. Thanks!

compiler-rt/lib/hwasan/CMakeLists.txt

set(HWASAN_RTL_SOURCES
   hwasan_fuchsia.cpp
   hwasan_globals.cpp
   hwasan_interceptors.cpp
   hwasan_interceptors_vfork.S
   hwasan_linux.cpp
   hwasan_memintrinsics.cpp
   hwasan_poisoning.cpp
   hwasan_report.cpp
-  hwasan_setjmp.S
+  hwasan_setjmp_aarch64.S
+  hwasan_setjmp_x86_64.S
   hwasan_tag_mismatch_aarch64.S
   hwasan_thread.cpp
   hwasan_thread_list.cpp
   hwasan_type_test.cpp
   )
 
 set(HWASAN_RTL_CXX_SOURCES
   hwasan_new_delete.cpp

compiler-rt/lib/hwasan/hwasan.h

 #define HWASAN_FREE_HOOK(ptr)       \
   do {                            \
     if (&__sanitizer_free_hook) { \
       __sanitizer_free_hook(ptr); \
     }                             \
     RunFreeHooks(ptr);            \
   } while (false)
 
-#if HWASAN_WITH_INTERCEPTORS && defined(__aarch64__)
+#if HWASAN_WITH_INTERCEPTORS
 // For both bionic and glibc __sigset_t is an unsigned long.
 typedef unsigned long __hw_sigset_t;
 // Setjmp and longjmp implementations are platform specific, and hence the
-// interception code is platform specific too.  As yet we've only implemented
-// the interception for AArch64.
-typedef unsigned long long __hw_register_buf[22];
+// interception code is platform specific too.
+#  if defined(__aarch64__)
+constexpr size_t kHwRegisterBufSize = 22;
+#  elif defined(__x86_64__)
+constexpr size_t kHwRegisterBufSize = 8;
+#  endif
+typedef unsigned long long __hw_register_buf[kHwRegisterBufSize];
 struct __hw_jmp_buf_struct {
   // NOTE: The machine-dependent definition of `__sigsetjmp'
   // assume that a `__hw_jmp_buf' begins with a `__hw_register_buf' and that
   // `__mask_was_saved' follows it.  Do not move these members or add others
   // before it.
   //
   // We add a __magic field to our struct to catch cases where libc's setjmp
   // populated the jmp_buf instead of our interceptor.
   __hw_register_buf __jmpbuf; // Calling environment.
   unsigned __mask_was_saved : 1;  // Saved the signal mask?
   unsigned __magic : 31;      // Used to distinguish __hw_jmp_buf from jmp_buf.
   __hw_sigset_t __saved_mask; // Saved signal mask.
 };
 typedef struct __hw_jmp_buf_struct __hw_jmp_buf[1];
 typedef struct __hw_jmp_buf_struct __hw_sigjmp_buf[1];
 constexpr unsigned kHwJmpBufMagic = 0x248ACE77;
-#endif // HWASAN_WITH_INTERCEPTORS && __aarch64__
+#endif  // HWASAN_WITH_INTERCEPTORS
 
 #define ENSURE_HWASAN_INITED()      \
   do {                              \
     CHECK(!hwasan_init_is_running); \
     if (!hwasan_inited) {           \
       __hwasan_init();              \
     }                               \
   } while (0)
 
 #endif  // HWASAN_H

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

ThreadStartArg *A = reinterpret_cast<ThreadStartArg *> (MmapOrDie(
       GetPageSizeCached(), "pthread_create"));
   *A = {callback, param};
   int res = REAL(pthread_create)(th, attr, &HwasanThreadStartFunc, A);
   return res;
 }
 
 DEFINE_REAL(int, vfork)
 DECLARE_EXTERN_INTERCEPTOR_AND_WRAPPER(int, vfork)
-#endif // HWASAN_WITH_INTERCEPTORS
 
-#if HWASAN_WITH_INTERCEPTORS && defined(__aarch64__)
 // Get and/or change the set of blocked signals.
 extern "C" int sigprocmask(int __how, const __hw_sigset_t *__restrict __set,
                            __hw_sigset_t *__restrict __oset);
 #define SIG_BLOCK 0
 #define SIG_SETMASK 2
 extern "C" int __sigjmp_save(__hw_sigjmp_buf env, int savemask) {
   env[0].__magic = kHwJmpBufMagic;
   env[0].__mask_was_saved =
       (savemask && sigprocmask(SIG_BLOCK, (__hw_sigset_t *)0,
                                &env[0].__saved_mask) == 0);
   return 0;
 }
 
 static void __attribute__((always_inline))
 InternalLongjmp(__hw_register_buf env, int retval) {
+#    if defined(__aarch64__)
+  constexpr size_t kSpIndex = 13;
+#    elif defined(__x86_64__)
+  constexpr size_t kSpIndex = 6;
+#    endif
+
   // Clear all memory tags on the stack between here and where we're going.
-  unsigned long long stack_pointer = env[13];
+  unsigned long long stack_pointer = env[kSpIndex];
   // The stack pointer should never be tagged, so we don't need to clear the
   // tag for this function call.
   __hwasan_handle_longjmp((void *)stack_pointer);
 
   // Run code for handling a longjmp.
   // Need to use a register that isn't going to be loaded from the environment
   // buffer -- hence why we need to specify the register to use.
   // Must implement this ourselves, since we don't know the order of registers
   // in different libc implementations and many implementations mangle the
   // stack pointer so we can't use it without knowing the demangling scheme.
+#    if defined(__aarch64__)
   register long int retval_tmp asm("x1") = retval;
   register void *env_address asm("x0") = &env[0];
   asm volatile("ldp  x19, x20, [%0, #0<<3];"
                "ldp  x21, x22, [%0, #2<<3];"
                "ldp  x23, x24, [%0, #4<<3];"
                "ldp  x25, x26, [%0, #6<<3];"
                "ldp  x27, x28, [%0, #8<<3];"
                "ldp  x29, x30, [%0, #10<<3];"
                "ldp   d8,  d9, [%0, #14<<3];"
                "ldp  d10, d11, [%0, #16<<3];"
                "ldp  d12, d13, [%0, #18<<3];"
                "ldp  d14, d15, [%0, #20<<3];"
                "ldr  x5, [%0, #13<<3];"
                "mov  sp, x5;"
                // Return the value requested to return through arguments.
                // This should be in x1 given what we requested above.
                "cmp  %1, #0;"
                "mov  x0, #1;"
                "csel x0, %1, x0, ne;"
                "br x30;"
                : "+r"(env_address)
                : "r"(retval_tmp));
+#    elif defined(__x86_64__)
+  register long int retval_tmp asm("%rsi") = retval;
+  register void *env_address asm("%rdi") = &env[0];
+  asm volatile(
+      // Restore registers.
+      "mov (0*8)(%0),%%rbx;"
+      "mov (1*8)(%0),%%rbp;"
+      "mov (2*8)(%0),%%r12;"
+      "mov (3*8)(%0),%%r13;"
+      "mov (4*8)(%0),%%r14;"
+      "mov (5*8)(%0),%%r15;"
+      "mov (6*8)(%0),%%rsp;"
+      "mov (7*8)(%0),%%rdx;"
+      // Return 1 if retval is 0.
+      "mov $1,%%rax;"
+      "test %1,%1;"
+      "cmovnz %1,%%rax;"
+      "jmp *%%rdx;" ::"r"(env_address),
+      "r"(retval_tmp));
+#    endif
 }
 
 INTERCEPTOR(void, siglongjmp, __hw_sigjmp_buf env, int val) {
   if (env[0].__magic != kHwJmpBufMagic) {
     Printf(
         "WARNING: Unexpected bad jmp_buf. Either setjmp was not called or "
         "there is a bug in HWASan.\n");
     return REAL(siglongjmp)(env, val);
Printf(
         "there is a bug in HWASan.\n");
     return REAL(longjmp)(env, val);
   }
   InternalLongjmp(env[0].__jmpbuf, val);
 }
 #undef SIG_BLOCK
 #undef SIG_SETMASK
 
-#endif // HWASAN_WITH_INTERCEPTORS && __aarch64__
+#  endif  // HWASAN_WITH_INTERCEPTORS
 
 namespace __hwasan {
 
 int OnExit() {
   // FIXME: ask frontend whether we need to return failure.
   return 0;
 }
 
 } // namespace __hwasan
 
 namespace __hwasan {
 
 void InitializeInterceptors() {
   static int inited = 0;
   CHECK_EQ(inited, 0);
 
 #if HWASAN_WITH_INTERCEPTORS
 #if defined(__linux__)
-#      if defined(__aarch64__)
   INTERCEPT_FUNCTION(__libc_longjmp);
   INTERCEPT_FUNCTION(longjmp);
   INTERCEPT_FUNCTION(siglongjmp);
-#      endif  // __aarch64__
   INTERCEPT_FUNCTION(vfork);
 #endif  // __linux__
   INTERCEPT_FUNCTION(pthread_create);
 #endif
 
   inited = 1;
 }
 } // namespace __hwasan
 
 #endif  // #if !SANITIZER_FUCHSIA

compiler-rt/lib/hwasan/hwasan_setjmp_aarch64.S

-//===-- hwasan_setjmp.S --------------------------------------------------------===//
+//===-- hwasan_setjmp_aarch64.S -------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is a part of HWAddressSanitizer.

 // 3) (no modification of any other saved register, but that's not really going
 // to occur, and hence isn't as much of a worry).
 //
 // There's essentially no way to ensure that the compiler will not modify the
 // stack pointer when compiling a C function.
 // Hence we have to write this function in assembly.
 
 .section .text
-.file "hwasan_setjmp.S"
+.file "hwasan_setjmp_aarch64.S"
 
 .global __interceptor_setjmp
 ASM_TYPE_FUNCTION(__interceptor_setjmp)
 __interceptor_setjmp:
   CFI_STARTPROC
   BTI_C
   mov x1, #0
   b __interceptor_sigsetjmp

compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S

+//===-- hwasan_setjmp_x86_64.S --------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// setjmp interceptor for x86_64.
+//
+//===----------------------------------------------------------------------===//
+
+#include "sanitizer_common/sanitizer_asm.h"
+
+#if HWASAN_WITH_INTERCEPTORS && defined(__x86_64__)
+#include "sanitizer_common/sanitizer_platform.h"
+
+// We want to save the context of the calling function.
+// That requires
+// 1) No modification of the return address by this function.
+// 2) No modification of the stack pointer by this function.
+// 3) (no modification of any other saved register, but that's not really going
+// to occur, and hence isn't as much of a worry).
+//
+// There's essentially no way to ensure that the compiler will not modify the
+// stack pointer when compiling a C function.
+// Hence we have to write this function in assembly.
+//
+// TODO: Handle Intel CET.
+
+.section .text
+.file "hwasan_setjmp_x86_64.S"
+
+.global __interceptor_setjmp
+ASM_TYPE_FUNCTION(__interceptor_setjmp)
+__interceptor_setjmp:
+  CFI_STARTPROC
+  xorl %esi, %esi
+  jmp __interceptor_sigsetjmp
+  CFI_ENDPROC
+ASM_SIZE(__interceptor_setjmp)
+
+.global __interceptor_sigsetjmp
+ASM_TYPE_FUNCTION(__interceptor_sigsetjmp)
+__interceptor_sigsetjmp:
+  CFI_STARTPROC
+
+  // Save callee save registers.
+  mov %rbx, (0*8)(%rdi)
+  mov %rbp, (1*8)(%rdi)
+  mov %r12, (2*8)(%rdi)
+  mov %r13, (3*8)(%rdi)
+  mov %r14, (4*8)(%rdi)
+  mov %r15, (5*8)(%rdi)
+
+  // Save SP as it was in caller's frame.
+  lea 8(%rsp), %rdx
+  mov %rdx, (6*8)(%rdi)
+
+  // Save return address.
+  mov (%rsp), %rax
+  mov %rax, (7*8)(%rdi)
+
+  jmp __sigjmp_save
+
+  CFI_ENDPROC
+ASM_SIZE(__interceptor_sigsetjmp)
+
+
+.macro WEAK_ALIAS first second
+  .globl \second
+  .equ \second\(), \first
+  .weak \second
+.endm
+
+WEAK_ALIAS __interceptor_sigsetjmp, __sigsetjmp
+WEAK_ALIAS __interceptor_setjmp, _setjmp
+#endif
+
+// We do not need executable stack.
+NO_EXEC_STACK_DIRECTIVE

compiler-rt/lib/hwasan/hwasan_type_test.cpp

 #include "interception/interception.h"
 #include "sanitizer_common/sanitizer_platform_limits_posix.h"
 #include "hwasan.h"
 #include <setjmp.h>
 
 #define CHECK_TYPE_SIZE_FITS(TYPE) \
   COMPILER_CHECK(sizeof(__hw_##TYPE) <= sizeof(TYPE))
 
-#if HWASAN_WITH_INTERCEPTORS && defined(__aarch64__)
+#if HWASAN_WITH_INTERCEPTORS
 CHECK_TYPE_SIZE_FITS(jmp_buf);
 CHECK_TYPE_SIZE_FITS(sigjmp_buf);
 #endif

compiler-rt/test/hwasan/TestCases/longjmp-setjmp-interception.c

 // RUN: %clang_hwasan -g %s -o %t
 // RUN: not %run %t 0 2>&1 | FileCheck %s
 // RUN: not %run %t -33 2>&1 | FileCheck %s
-// Only implemented for interceptor ABI on AArch64.
-// REQUIRES: aarch64-target-arch
+// REQUIRES: pointer-tagging
 
 #include <assert.h>
 #include <setjmp.h>
 #include <stdio.h>
 #include <stdlib.h>
 
 /* Testing longjmp/setjmp should test that accesses to scopes jmp'd over are
    caught.  */