[libunwind] 0be0a53 - [libunwind] Use process_vm_readv to avoid potential segfaults

[Shoaib Meenai via cfe-commits]

Author: Shoaib Meenai
Date: 2022-05-26T09:12:51-07:00
New Revision: 0be0a53df65cb402359c257922d80ab93d86fb40

URL: https://github.com/llvm/llvm-project/commit/0be0a53df65cb402359c257922d80ab93d86fb40
DIFF: https://github.com/llvm/llvm-project/commit/0be0a53df65cb402359c257922d80ab93d86fb40.diff

LOG: [libunwind] Use process_vm_readv to avoid potential segfaults

We've observed segfaults in libunwind when attempting to check for the
Linux aarch64 sigreturn frame, presumably because of bad unwind info
leading to an incorrect PC that we attempt to read from. Use
process_vm_readv to read the memory safely instead.

The s390x code path should likely follow suit, but I don't have the
hardware to be able to test that, so I didn't modify it here either.

Reviewed By: MaskRay, rprichard, #libunwind

Differential Revision: https://reviews.llvm.org/D126343

Added: 
    libunwind/test/bad_unwind_info.pass.cpp

Modified: 
    libunwind/src/UnwindCursor.hpp

Removed: 
    


################################################################################
diff  --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 01f0c404a2ac4..6e8fc45ebfa8f 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -32,6 +32,9 @@
 
 #if defined(_LIBUNWIND_TARGET_LINUX) &&                                        \
     (defined(_LIBUNWIND_TARGET_AARCH64) || defined(_LIBUNWIND_TARGET_S390X))
+#include <sys/syscall.h>
+#include <sys/uio.h>
+#include <unistd.h>
 #define _LIBUNWIND_CHECK_LINUX_SIGRETURN 1
 #endif
 
@@ -2620,16 +2623,28 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_arm64 &) {
   //
   // [1] https://github.com/torvalds/linux/blob/master/arch/arm64/kernel/vdso/sigreturn.S
   const pint_t pc = static_cast<pint_t>(this->getReg(UNW_REG_IP));
+  // The PC might contain an invalid address if the unwind info is bad, so
+  // directly accessing it could cause a segfault. Use process_vm_readv to read
+  // the memory safely instead. process_vm_readv was added in Linux 3.2, and
+  // AArch64 supported was added in Linux 3.7, so the syscall is guaranteed to
+  // be present. Unfortunately, there are Linux AArch64 environments where the
+  // libc wrapper for the syscall might not be present (e.g. Android 5), so call
+  // the syscall directly instead.
+  uint32_t instructions[2];
+  struct iovec local_iov = {&instructions, sizeof instructions};
+  struct iovec remote_iov = {reinterpret_cast<void *>(pc), sizeof instructions};
+  long bytesRead =
+      syscall(SYS_process_vm_readv, getpid(), &local_iov, 1, &remote_iov, 1, 0);
   // Look for instructions: mov x8, #0x8b; svc #0x0
-  if (_addressSpace.get32(pc) == 0xd2801168 &&
-      _addressSpace.get32(pc + 4) == 0xd4000001) {
-    _info = {};
-    _info.start_ip = pc;
-    _info.end_ip = pc + 4;
-    _isSigReturn = true;
-    return true;
-  }
-  return false;
+  if (bytesRead != sizeof instructions || instructions[0] != 0xd2801168 ||
+      instructions[1] != 0xd4000001)
+    return false;
+
+  _info = {};
+  _info.start_ip = pc;
+  _info.end_ip = pc + 4;
+  _isSigReturn = true;
+  return true;
 }
 
 template <typename A, typename R>

diff  --git a/libunwind/test/bad_unwind_info.pass.cpp b/libunwind/test/bad_unwind_info.pass.cpp
new file mode 100644
index 0000000000000..97427ea922737
--- /dev/null
+++ b/libunwind/test/bad_unwind_info.pass.cpp
@@ -0,0 +1,66 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// Ensure that libunwind doesn't crash on invalid info; the Linux aarch64
+// sigreturn frame check would previously attempt to access invalid memory in
+// this scenario.
+// REQUIRES: linux && (target={{aarch64-.+}} || target={{x86_64-.+}})
+
+// GCC doesn't support __attribute__((naked)) on AArch64.
+// UNSUPPORTED: gcc
+
+// Inline assembly is incompatible with MSAN.
+// UNSUPPORTED: msan
+
+#undef NDEBUG
+#include <assert.h>
+#include <libunwind.h>
+#include <stdio.h>
+
+__attribute__((naked)) void bad_unwind_info() {
+#if defined(__aarch64__)
+  __asm__("// not using 0 because unwinder was already resilient to that\n"
+          "mov     x8, #4\n"
+          "stp     x30, x8, [sp, #-16]!\n"
+          ".cfi_def_cfa_offset 16\n"
+          "// purposely use incorrect offset for x30\n"
+          ".cfi_offset x30, -8\n"
+          "bl      stepper\n"
+          "ldr     x30, [sp], #16\n"
+          ".cfi_def_cfa_offset 0\n"
+          ".cfi_restore x30\n"
+          "ret\n");
+#elif defined(__x86_64__)
+  __asm__("pushq   %rbx\n"
+          ".cfi_def_cfa_offset 16\n"
+          "movq    8(%rsp), %rbx\n"
+          "# purposely corrupt return value on stack\n"
+          "movq    $4, 8(%rsp)\n"
+          "callq   stepper\n"
+          "movq    %rbx, 8(%rsp)\n"
+          "popq    %rbx\n"
+          ".cfi_def_cfa_offset 8\n"
+          "ret\n");
+#else
+#error This test is only supported on aarch64 or x86-64
+#endif
+}
+
+extern "C" void stepper() {
+  unw_cursor_t cursor;
+  unw_context_t uc;
+  unw_getcontext(&uc);
+  unw_init_local(&cursor, &uc);
+  // stepping to bad_unwind_info should succeed
+  assert(unw_step(&cursor) > 0);
+  // stepping past bad_unwind_info should fail but not crash
+  assert(unw_step(&cursor) <= 0);
+}
+
+int main() { bad_unwind_info(); }