[libcxx-commits] [PATCH] D129856: [libunwind][SystemZ] Use process_vm_readv to avoid potential segfaults

[Ulrich Weigand via Phabricator via libcxx-commits]

uweigand created this revision.
uweigand added reviewers: smeenai, MaskRay, rprichard, libunwind.
Herald added subscribers: libcxx-commits, StephenFan, kristof.beyls.
Herald added projects: libunwind, All.
Herald added 1 blocking reviewer(s): libunwind.
uweigand requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Fix potential crashes during unwind when checking for signal frames and the current PC is invalid.

The same bug was fixed for aarch64 in https://reviews.llvm.org/D126343.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129856

Files:
  libunwind/src/UnwindCursor.hpp
  libunwind/test/bad_unwind_info.pass.cpp


Index: libunwind/test/bad_unwind_info.pass.cpp
===================================================================
--- libunwind/test/bad_unwind_info.pass.cpp
+++ libunwind/test/bad_unwind_info.pass.cpp
@@ -10,7 +10,7 @@
 // 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-.+}})
+// REQUIRES: linux && (target={{aarch64-.+}} || target={{s390x-.+}} || target={{x86_64-.+}})
 
 // GCC doesn't support __attribute__((naked)) on AArch64.
 // UNSUPPORTED: gcc
@@ -36,6 +36,20 @@
           ".cfi_def_cfa_offset 0\n"
           ".cfi_restore x30\n"
           "ret\n");
+#elif defined(__s390x__)
+  __asm__("stmg    %r14,%r15,112(%r15)\n"
+	  "mvghi   104(%r15),4\n"
+          "# purposely use incorrect offset for %r14\n"
+          ".cfi_offset 14, -56\n"
+          ".cfi_offset 15, -40\n"
+          "lay     %r15,-160(%r15)\n"
+          ".cfi_def_cfa_offset 320\n"
+          "brasl   %r14,stepper\n"
+          "lmg     %r14,%r15,272(%r15)\n"
+          ".cfi_restore 15\n"
+          ".cfi_restore 14\n"
+          ".cfi_def_cfa_offset 160\n"
+          "br      %r14\n");
 #elif defined(__x86_64__)
   __asm__("pushq   %rbx\n"
           ".cfi_def_cfa_offset 16\n"
@@ -48,7 +62,7 @@
           ".cfi_def_cfa_offset 8\n"
           "ret\n");
 #else
-#error This test is only supported on aarch64 or x86-64
+#error This test is only supported on aarch64, s390x, or x86-64
 #endif
 }
 
Index: libunwind/src/UnwindCursor.hpp
===================================================================
--- libunwind/src/UnwindCursor.hpp
+++ libunwind/src/UnwindCursor.hpp
@@ -2695,8 +2695,15 @@
   // own restorer function, though, or user-mode QEMU might write a trampoline
   // onto the stack.
   const pint_t pc = static_cast<pint_t>(this->getReg(UNW_REG_IP));
-  const uint16_t inst = _addressSpace.get16(pc);
-  if (inst == 0x0a77 || inst == 0x0aad) {
+  // 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.
+  uint16_t inst;
+  struct iovec local_iov = {&inst, sizeof inst};
+  struct iovec remote_iov = {reinterpret_cast<void *>(pc), sizeof inst};
+  long bytesRead =
+      syscall(SYS_process_vm_readv, getpid(), &local_iov, 1, &remote_iov, 1, 0);
+  if (bytesRead == sizeof inst && (inst == 0x0a77 || inst == 0x0aad)) {
     _info = {};
     _info.start_ip = pc;
     _info.end_ip = pc + 2;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129856.444980.patch
Type: text/x-patch
Size: 2613 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220715/3870f55b/attachment.bin>