[libc-commits] [lldb] [llvm] [libcxx] [flang] [libc] [clang-tools-extra] [clang] [compiler-rt] [libunwind] [libunwind] Replace process_vm_readv with SYS_rt_sigprocmask (PR #74791)

Jordan R AW via libc-commits libc-commits at lists.llvm.org
Thu Jan 4 11:23:53 PST 2024


https://github.com/ajordanr-google updated https://github.com/llvm/llvm-project/pull/74791

>From 9d4665cf1ddda98129af2f6ff575989cec49af6d Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Fri, 8 Dec 2023 00:09:59 +0000
Subject: [PATCH 01/13] [libunwind] Replace process_vm_readv with pipe

process_vm_readv is generally considered dangerous from a syscall
perspective, and is frequently blanket banned in seccomp filters such as
those in Chromium and ChromiumOS. We can get the same behaviour during
the invalid PC address case with pipes and write/read.

Testing to ensure that process_vm_readv does not appear, I ran the
output of check-unwind on an ARM64 device under strace. Previously,
bad_unwind_info in particular would use process_vm_readv, but with this
commit, it now uses pipe2:

```
strace test/Output/bad_unwind_info.pass.cpp.dir/t.tmp.exe \
  |& grep process_vm_readv
strace test/Output/bad_unwind_info.pass.cpp.dir/t.tmp.exe \
  |& grep pipe2
```
---
 libunwind/src/UnwindCursor.hpp | 50 ++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 647a5a9c9d92d9..5e4e376220daa0 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -33,6 +33,7 @@
 #if defined(_LIBUNWIND_TARGET_LINUX) &&                                        \
     (defined(_LIBUNWIND_TARGET_AARCH64) || defined(_LIBUNWIND_TARGET_RISCV) || \
      defined(_LIBUNWIND_TARGET_S390X))
+#include <fcntl.h>
 #include <sys/syscall.h>
 #include <sys/uio.h>
 #include <unistd.h>
@@ -2700,19 +2701,18 @@ 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.
+  // directly accessing it could cause a segfault. Use pipe/write/read to read
+  // the memory safely instead.
+  int pipefd[2];
+  if (pipe2(pipefd, O_CLOEXEC | O_NONBLOCK) == -1)
+    return false;
   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);
+  const auto bufferSize = sizeof instructions;
+  if (write(pipefd[1], reinterpret_cast<void *>(pc), bufferSize) != bufferSize)
+    return false;
+  const auto bytesRead = read(pipefd[0], instructions, bufferSize);
   // Look for instructions: mov x8, #0x8b; svc #0x0
-  if (bytesRead != sizeof instructions || instructions[0] != 0xd2801168 ||
+  if (bytesRead != bufferSize || instructions[0] != 0xd2801168 ||
       instructions[1] != 0xd4000001)
     return false;
 
@@ -2762,17 +2762,20 @@ int UnwindCursor<A, R>::stepThroughSigReturn(Registers_arm64 &) {
 template <typename A, typename R>
 bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_riscv &) {
   const pint_t pc = static_cast<pint_t>(getReg(UNW_REG_IP));
+  int pipefd[2];
+  if (pipe2(pipefd, O_CLOEXEC | O_NONBLOCK) == -1)
+    return false;
   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);
+  const auto bufferSize = sizeof instructions;
+  if (write(pipefd[1], reinterpret_cast<void *>(pc), bufferSize) != bufferSize)
+    return false;
+  const auto bytesRead = read(pipefd[0], instructions, bufferSize);
   // Look for the two instructions used in the sigreturn trampoline
   // __vdso_rt_sigreturn:
   //
   // 0x08b00893 li a7,0x8b
   // 0x00000073 ecall
-  if (bytesRead != sizeof instructions || instructions[0] != 0x08b00893 ||
+  if (bytesRead != bufferSize || instructions[0] != 0x08b00893 ||
       instructions[1] != 0x00000073)
     return false;
 
@@ -2822,13 +2825,18 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_s390x &) {
   // onto the stack.
   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
+  // directly accessing it could cause a segfault. Use pipe/write/read 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 = process_vm_readv(getpid(), &local_iov, 1, &remote_iov, 1, 0);
-  if (bytesRead == sizeof inst && (inst == 0x0a77 || inst == 0x0aad)) {
+  int pipefd[2];
+  if (pipe2(pipefd, O_CLOEXEC | O_NONBLOCK) == -1)
+    return false;
+  uint16_t inst;
+  const auto bufferSize = sizeof inst;
+  if (write(pipefd[1], reinterpret_cast<void *>(pc), bufferSize) != bufferSize)
+    return false;
+  const auto bytesRead = read(pipefd[0], &inst, bufferSize);
+  if (bytesRead == bufferSize && (inst == 0x0a77 || inst == 0x0aad)) {
     _info = {};
     _info.start_ip = pc;
     _info.end_ip = pc + 2;

>From 602b3f29a76d274eb4b082a976fc5107891dca91 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Fri, 8 Dec 2023 22:41:17 +0000
Subject: [PATCH 02/13] fixup! [libunwind] Replace process_vm_readv with pipe

---
 libunwind/src/UnwindCursor.hpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 5e4e376220daa0..c36be5a6c0ad1f 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2827,7 +2827,6 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_s390x &) {
   // The PC might contain an invalid address if the unwind info is bad, so
   // directly accessing it could cause a segfault. Use pipe/write/read to
   // read the memory safely instead.
-  uint16_t inst;
   int pipefd[2];
   if (pipe2(pipefd, O_CLOEXEC | O_NONBLOCK) == -1)
     return false;

>From 41025a192caae9bfa4533c66425a3bd9b01cdedd Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Wed, 13 Dec 2023 21:35:55 +0000
Subject: [PATCH 03/13] fixup! [libunwind] Replace process_vm_readv with pipe

---
 libunwind/src/UnwindCursor.hpp | 66 ++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index c36be5a6c0ad1f..165434dde74a7d 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -558,6 +558,26 @@ class UnwindCursor : public AbstractUnwindCursor {
       _info.handler = 0;
     return UNW_STEP_SUCCESS;
   }
+  bool isReadableAddr(const void *addr) const {
+    // This code is heavily based on Abseil's 'address_is_readable.cc',
+    // which is Copyright Abseil Authors (2017), and provided under
+    // the Apache License 2.0.
+
+    // We have to check that addr is a nullptr because sigprocmask allows that
+    // as an argument without failure.
+    if (!addr)
+      return false;
+    // Align to 8-bytes.
+    const auto uintptr_addr = reinterpret_cast<uintptr_t>(addr) & ~uintptr_t{7};
+    const auto sigsetaddr = reinterpret_cast<sigset_t *>(uintptr_addr);
+    [[maybe_unused]] int Result = sigprocmask(/*how=*/-1, sigsetaddr, nullptr);
+    // Because our "how" is invalid, this syscall should always fail, and our
+    // errno should always be EINVAL or an EFAULT. EFAULT is not guaranteed
+    // by the POSIX standard, so this is (for now) Linux specific.
+    assert(Result == -1);
+    assert(errno == EFAULT || errno == EINVAL);
+    return errno != EFAULT;
+  }
 
   A                   &_addressSpace;
   unw_proc_info_t      _info;
@@ -2701,19 +2721,13 @@ 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 pipe/write/read to read
-  // the memory safely instead.
-  int pipefd[2];
-  if (pipe2(pipefd, O_CLOEXEC | O_NONBLOCK) == -1)
-    return false;
-  uint32_t instructions[2];
-  const auto bufferSize = sizeof instructions;
-  if (write(pipefd[1], reinterpret_cast<void *>(pc), bufferSize) != bufferSize)
+  // directly accessing it could cause a SIGSEGV.
+  if (!isReadableAddr(static_cast<void *>(pc)) ||
+      !isReadableAddr(static_cast<void *>(pc + 4)))
     return false;
-  const auto bytesRead = read(pipefd[0], instructions, bufferSize);
+  auto *instructions = static_cast<uint32_t *>(pc);
   // Look for instructions: mov x8, #0x8b; svc #0x0
-  if (bytesRead != bufferSize || instructions[0] != 0xd2801168 ||
-      instructions[1] != 0xd4000001)
+  if (instructions[0] != 0xd2801168 || instructions[1] != 0xd4000001)
     return false;
 
   _info = {};
@@ -2762,21 +2776,18 @@ int UnwindCursor<A, R>::stepThroughSigReturn(Registers_arm64 &) {
 template <typename A, typename R>
 bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_riscv &) {
   const pint_t pc = static_cast<pint_t>(getReg(UNW_REG_IP));
-  int pipefd[2];
-  if (pipe2(pipefd, O_CLOEXEC | O_NONBLOCK) == -1)
-    return false;
-  uint32_t instructions[2];
-  const auto bufferSize = sizeof instructions;
-  if (write(pipefd[1], reinterpret_cast<void *>(pc), bufferSize) != bufferSize)
+  // The PC might contain an invalid address if the unwind info is bad, so
+  // directly accessing it could cause a SIGSEGV.
+  if (!isReadableAddr(static_cast<void *>(pc)) ||
+      !isReadableAddr(static_cast<void *>(pc + 4)))
     return false;
-  const auto bytesRead = read(pipefd[0], instructions, bufferSize);
+  const auto *instructions = static_cast<uint32_t *>(pc);
   // Look for the two instructions used in the sigreturn trampoline
   // __vdso_rt_sigreturn:
   //
   // 0x08b00893 li a7,0x8b
   // 0x00000073 ecall
-  if (bytesRead != bufferSize || instructions[0] != 0x08b00893 ||
-      instructions[1] != 0x00000073)
+  if (instructions[0] != 0x08b00893 || instructions[1] != 0x00000073)
     return false;
 
   _info = {};
@@ -2825,17 +2836,12 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_s390x &) {
   // onto the stack.
   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 pipe/write/read to
-  // read the memory safely instead.
-  int pipefd[2];
-  if (pipe2(pipefd, O_CLOEXEC | O_NONBLOCK) == -1)
-    return false;
-  uint16_t inst;
-  const auto bufferSize = sizeof inst;
-  if (write(pipefd[1], reinterpret_cast<void *>(pc), bufferSize) != bufferSize)
+  // directly accessing it could cause a SIGSEGV.
+  if (!isReadableAddr(static_cast<void *>(pc)) ||
+      !isReadableAddr(static_cast<void *>(pc + 2)))
     return false;
-  const auto bytesRead = read(pipefd[0], &inst, bufferSize);
-  if (bytesRead == bufferSize && (inst == 0x0a77 || inst == 0x0aad)) {
+  const auto inst = *reinterpret_cast<uint16_t *>(pc);
+  if (inst == 0x0a77 || inst == 0x0aad) {
     _info = {};
     _info.start_ip = pc;
     _info.end_ip = pc + 2;

>From bc7bb5a93a3c674dd55836bcd749bb18c4f7147d Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Wed, 13 Dec 2023 21:41:51 +0000
Subject: [PATCH 04/13] fixup! [libunwind] Replace process_vm_readv with pipe

---
 libunwind/src/UnwindCursor.hpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 165434dde74a7d..f5050d327f6f04 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -12,6 +12,8 @@
 #define __UNWINDCURSOR_HPP__
 
 #include "cet_unwind.h"
+#include <errno.h>
+#include <signal.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -33,7 +35,6 @@
 #if defined(_LIBUNWIND_TARGET_LINUX) &&                                        \
     (defined(_LIBUNWIND_TARGET_AARCH64) || defined(_LIBUNWIND_TARGET_RISCV) || \
      defined(_LIBUNWIND_TARGET_S390X))
-#include <fcntl.h>
 #include <sys/syscall.h>
 #include <sys/uio.h>
 #include <unistd.h>
@@ -2725,7 +2726,7 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_arm64 &) {
   if (!isReadableAddr(static_cast<void *>(pc)) ||
       !isReadableAddr(static_cast<void *>(pc + 4)))
     return false;
-  auto *instructions = static_cast<uint32_t *>(pc);
+  auto *instructions = reinterpret_cast<uint32_t *>(pc);
   // Look for instructions: mov x8, #0x8b; svc #0x0
   if (instructions[0] != 0xd2801168 || instructions[1] != 0xd4000001)
     return false;
@@ -2781,7 +2782,7 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_riscv &) {
   if (!isReadableAddr(static_cast<void *>(pc)) ||
       !isReadableAddr(static_cast<void *>(pc + 4)))
     return false;
-  const auto *instructions = static_cast<uint32_t *>(pc);
+  const auto *instructions = reinterpret_cast<uint32_t *>(pc);
   // Look for the two instructions used in the sigreturn trampoline
   // __vdso_rt_sigreturn:
   //

>From 440d9dcedd09c78c8d87fa04c0f61fb0570afb8b Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Wed, 13 Dec 2023 21:51:28 +0000
Subject: [PATCH 05/13] fixup! [libunwind] Replace process_vm_readv with pipe

---
 libunwind/src/UnwindCursor.hpp | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index f5050d327f6f04..436b6fc3bc5a3c 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2723,10 +2723,10 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_arm64 &) {
   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 SIGSEGV.
-  if (!isReadableAddr(static_cast<void *>(pc)) ||
-      !isReadableAddr(static_cast<void *>(pc + 4)))
+  if (!isReadableAddr(static_cast<const void *>(pc)) ||
+      !isReadableAddr(static_cast<const void *>(pc + 4)))
     return false;
-  auto *instructions = reinterpret_cast<uint32_t *>(pc);
+  auto *instructions = reinterpret_cast<const uint32_t *>(pc);
   // Look for instructions: mov x8, #0x8b; svc #0x0
   if (instructions[0] != 0xd2801168 || instructions[1] != 0xd4000001)
     return false;
@@ -2779,10 +2779,10 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_riscv &) {
   const pint_t pc = static_cast<pint_t>(getReg(UNW_REG_IP));
   // The PC might contain an invalid address if the unwind info is bad, so
   // directly accessing it could cause a SIGSEGV.
-  if (!isReadableAddr(static_cast<void *>(pc)) ||
-      !isReadableAddr(static_cast<void *>(pc + 4)))
+  if (!isReadableAddr(static_cast<const void *>(pc)) ||
+      !isReadableAddr(static_cast<const void *>(pc + 4)))
     return false;
-  const auto *instructions = reinterpret_cast<uint32_t *>(pc);
+  const auto *instructions = reinterpret_cast<const uint32_t *>(pc);
   // Look for the two instructions used in the sigreturn trampoline
   // __vdso_rt_sigreturn:
   //
@@ -2838,10 +2838,10 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_s390x &) {
   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 SIGSEGV.
-  if (!isReadableAddr(static_cast<void *>(pc)) ||
-      !isReadableAddr(static_cast<void *>(pc + 2)))
+  if (!isReadableAddr(static_cast<const void *>(pc)) ||
+      !isReadableAddr(static_cast<const void *>(pc + 2)))
     return false;
-  const auto inst = *reinterpret_cast<uint16_t *>(pc);
+  const auto inst = *reinterpret_cast<const uint16_t *>(pc);
   if (inst == 0x0a77 || inst == 0x0aad) {
     _info = {};
     _info.start_ip = pc;

>From fce7cba503f788645804070559c66a4095e94424 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Wed, 13 Dec 2023 23:52:58 +0000
Subject: [PATCH 06/13] fixup! [libunwind] Replace process_vm_readv with pipe

---
 libunwind/src/UnwindCursor.hpp | 54 ++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 436b6fc3bc5a3c..67a6ab353a16ed 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -559,26 +559,6 @@ class UnwindCursor : public AbstractUnwindCursor {
       _info.handler = 0;
     return UNW_STEP_SUCCESS;
   }
-  bool isReadableAddr(const void *addr) const {
-    // This code is heavily based on Abseil's 'address_is_readable.cc',
-    // which is Copyright Abseil Authors (2017), and provided under
-    // the Apache License 2.0.
-
-    // We have to check that addr is a nullptr because sigprocmask allows that
-    // as an argument without failure.
-    if (!addr)
-      return false;
-    // Align to 8-bytes.
-    const auto uintptr_addr = reinterpret_cast<uintptr_t>(addr) & ~uintptr_t{7};
-    const auto sigsetaddr = reinterpret_cast<sigset_t *>(uintptr_addr);
-    [[maybe_unused]] int Result = sigprocmask(/*how=*/-1, sigsetaddr, nullptr);
-    // Because our "how" is invalid, this syscall should always fail, and our
-    // errno should always be EINVAL or an EFAULT. EFAULT is not guaranteed
-    // by the POSIX standard, so this is (for now) Linux specific.
-    assert(Result == -1);
-    assert(errno == EFAULT || errno == EINVAL);
-    return errno != EFAULT;
-  }
 
   A                   &_addressSpace;
   unw_proc_info_t      _info;
@@ -1012,6 +992,7 @@ class UnwindCursor : public AbstractUnwindCursor{
     R dummy;
     return stepThroughSigReturn(dummy);
   }
+  bool isReadableAddr(const pint_t addr) const;
 #if defined(_LIBUNWIND_TARGET_AARCH64)
   bool setInfoForSigReturn(Registers_arm64 &);
   int stepThroughSigReturn(Registers_arm64 &);
@@ -2723,8 +2704,7 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_arm64 &) {
   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 SIGSEGV.
-  if (!isReadableAddr(static_cast<const void *>(pc)) ||
-      !isReadableAddr(static_cast<const void *>(pc + 4)))
+  if (!isReadableAddr(pc) || !isReadableAddr(pc + 4))
     return false;
   auto *instructions = reinterpret_cast<const uint32_t *>(pc);
   // Look for instructions: mov x8, #0x8b; svc #0x0
@@ -2779,8 +2759,7 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_riscv &) {
   const pint_t pc = static_cast<pint_t>(getReg(UNW_REG_IP));
   // The PC might contain an invalid address if the unwind info is bad, so
   // directly accessing it could cause a SIGSEGV.
-  if (!isReadableAddr(static_cast<const void *>(pc)) ||
-      !isReadableAddr(static_cast<const void *>(pc + 4)))
+  if (!isReadableAddr(pc) || !isReadableAddr(pc + 4))
     return false;
   const auto *instructions = reinterpret_cast<const uint32_t *>(pc);
   // Look for the two instructions used in the sigreturn trampoline
@@ -2838,8 +2817,7 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_s390x &) {
   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 SIGSEGV.
-  if (!isReadableAddr(static_cast<const void *>(pc)) ||
-      !isReadableAddr(static_cast<const void *>(pc + 2)))
+  if (!isReadableAddr(pc) || !isReadableAddr(pc + 2))
     return false;
   const auto inst = *reinterpret_cast<const uint16_t *>(pc);
   if (inst == 0x0a77 || inst == 0x0aad) {
@@ -2988,6 +2966,30 @@ bool UnwindCursor<A, R>::getFunctionName(char *buf, size_t bufLen,
                                          buf, bufLen, offset);
 }
 
+#if defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN)
+template <typename A, typename R>
+bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
+  // This code is heavily based on Abseil's 'address_is_readable.cc',
+  // which is Copyright Abseil Authors (2017), and provided under
+  // the Apache License 2.0.
+
+  // We have to check that addr is nullptr (0) because sigprocmask allows that
+  // as an argument without failure.
+  if (addr == 0)
+    return false;
+  // Align to 8-bytes.
+  const auto alignedAddr = addr & ~pint_t{7};
+  const auto sigsetAddr = reinterpret_cast<sigset_t *>(alignedAddr);
+  [[maybe_unused]] int Result = sigprocmask(/*how=*/-1, sigsetAddr, nullptr);
+  // Because our "how" is invalid, this syscall should always fail, and our
+  // errno should always be EINVAL or an EFAULT. EFAULT is not guaranteed
+  // by the POSIX standard, so this is (for now) Linux specific.
+  assert(Result == -1);
+  assert(errno == EFAULT || errno == EINVAL);
+  return errno != EFAULT;
+}
+#endif
+
 #if defined(_LIBUNWIND_USE_CET)
 extern "C" void *__libunwind_cet_get_registers(unw_cursor_t *cursor) {
   AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;

>From 23c0dd1a782e98d556f8b9218db1b56bfca392a4 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Mon, 18 Dec 2023 22:54:31 +0000
Subject: [PATCH 07/13] fixup! [libunwind] Replace process_vm_readv with pipe

---
 libunwind/src/UnwindCursor.hpp          | 19 ++++++++++++++-----
 libunwind/test/bad_unwind_info.pass.cpp |  2 +-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 67a6ab353a16ed..aa3c3626647910 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2973,14 +2973,23 @@ bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
   // which is Copyright Abseil Authors (2017), and provided under
   // the Apache License 2.0.
 
-  // We have to check that addr is nullptr (0) because sigprocmask allows that
-  // as an argument without failure.
-  if (addr == 0)
-    return false;
   // Align to 8-bytes.
   const auto alignedAddr = addr & ~pint_t{7};
   const auto sigsetAddr = reinterpret_cast<sigset_t *>(alignedAddr);
-  [[maybe_unused]] int Result = sigprocmask(/*how=*/-1, sigsetAddr, nullptr);
+  // We have to check that addr is nullptr because sigprocmask allows that
+  // as an argument without failure.
+  if (!sigsetAddr)
+    return false;
+
+  // We MUST use the raw sigprocmask syscall here, as wrappers may try to
+  // access sigsetAddr which may cause a SIGSEGV. The raw syscall however is
+  // safe. Additionally, we need to pass the kernel_sigset_size, which is
+  // different from libc sizeof(sigset_t). Some archs have sigset_t
+  // defined as unsigned long, so let's use that.
+  const auto approxKernelSigsetSize = sizeof(unsigned long);
+  [[maybe_unused]] int Result =
+      syscall(SYS_rt_sigprocmask, /*how=*/~0, sigsetAddr, sigsetAddr,
+              approxKernelSigsetSize);
   // Because our "how" is invalid, this syscall should always fail, and our
   // errno should always be EINVAL or an EFAULT. EFAULT is not guaranteed
   // by the POSIX standard, so this is (for now) Linux specific.
diff --git a/libunwind/test/bad_unwind_info.pass.cpp b/libunwind/test/bad_unwind_info.pass.cpp
index b3284e8daed71f..2e7cf15f0dcab8 100644
--- a/libunwind/test/bad_unwind_info.pass.cpp
+++ b/libunwind/test/bad_unwind_info.pass.cpp
@@ -26,7 +26,7 @@
 __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"
+          "mov     x8, #12\n"
           "stp     x30, x8, [sp, #-16]!\n"
           ".cfi_def_cfa_offset 16\n"
           "// purposely use incorrect offset for x30\n"

>From aa402a4e97db1803fe3da899761604b254644e1f Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Mon, 18 Dec 2023 23:36:03 +0000
Subject: [PATCH 08/13] fixup! [libunwind] Replace process_vm_readv with pipe

---
 libunwind/src/UnwindCursor.hpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index aa3c3626647910..58dcd840141bf9 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2987,7 +2987,7 @@ bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
   // different from libc sizeof(sigset_t). Some archs have sigset_t
   // defined as unsigned long, so let's use that.
   const auto approxKernelSigsetSize = sizeof(unsigned long);
-  [[maybe_unused]] int Result =
+  [[maybe_unused]] const int Result =
       syscall(SYS_rt_sigprocmask, /*how=*/~0, sigsetAddr, sigsetAddr,
               approxKernelSigsetSize);
   // Because our "how" is invalid, this syscall should always fail, and our

>From 2346a2acf9b2ad15ea47fe5330c82bed79aff009 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Tue, 19 Dec 2023 19:17:30 +0000
Subject: [PATCH 09/13] fixup! fixup! [libunwind] Replace process_vm_readv with
 pipe

---
 libunwind/src/UnwindCursor.hpp | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 58dcd840141bf9..cfaf2e4aa2211b 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2970,8 +2970,7 @@ bool UnwindCursor<A, R>::getFunctionName(char *buf, size_t bufLen,
 template <typename A, typename R>
 bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
   // This code is heavily based on Abseil's 'address_is_readable.cc',
-  // which is Copyright Abseil Authors (2017), and provided under
-  // the Apache License 2.0.
+  // which is Copyright Abseil Authors (2017).
 
   // Align to 8-bytes.
   const auto alignedAddr = addr & ~pint_t{7};
@@ -2980,19 +2979,20 @@ bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
   // as an argument without failure.
   if (!sigsetAddr)
     return false;
-
-  // We MUST use the raw sigprocmask syscall here, as wrappers may try to
-  // access sigsetAddr which may cause a SIGSEGV. The raw syscall however is
+  // We MUST use a raw syscall here, as wrappers may try to access
+  // sigsetAddr which may cause a SIGSEGV. A raw syscall however is
   // safe. Additionally, we need to pass the kernel_sigset_size, which is
-  // different from libc sizeof(sigset_t). Some archs have sigset_t
-  // defined as unsigned long, so let's use that.
-  const auto approxKernelSigsetSize = sizeof(unsigned long);
-  [[maybe_unused]] const int Result =
-      syscall(SYS_rt_sigprocmask, /*how=*/~0, sigsetAddr, sigsetAddr,
-              approxKernelSigsetSize);
+  // different from libc sizeof(sigset_t). 8 seems to work for both 64bit and
+  // 32bit archs.
+  const auto approxKernelSigsetSize = 8;
+  int Result = syscall(SYS_rt_sigprocmask, /*how=*/~0, sigsetAddr, nullptr,
+                       approxKernelSigsetSize);
+  (void)Result;
   // Because our "how" is invalid, this syscall should always fail, and our
   // errno should always be EINVAL or an EFAULT. EFAULT is not guaranteed
-  // by the POSIX standard, so this is (for now) Linux specific.
+  // by the POSIX standard. Additionally, this relies on the Linux kernel
+  // to check copy_from_user before checking if the "how" argument is
+  // invalid.
   assert(Result == -1);
   assert(errno == EFAULT || errno == EINVAL);
   return errno != EFAULT;

>From 058521c1f60570c45e2e7018ffc51d66ae977eb0 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Tue, 19 Dec 2023 19:20:55 +0000
Subject: [PATCH 10/13] fixup! fixup! [libunwind] Replace process_vm_readv with
 pipe

---
 libunwind/src/UnwindCursor.hpp | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index cfaf2e4aa2211b..fd816f0dd28888 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2704,7 +2704,7 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_arm64 &) {
   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 SIGSEGV.
-  if (!isReadableAddr(pc) || !isReadableAddr(pc + 4))
+  if (!isReadableAddr(pc))
     return false;
   auto *instructions = reinterpret_cast<const uint32_t *>(pc);
   // Look for instructions: mov x8, #0x8b; svc #0x0
@@ -2759,7 +2759,7 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_riscv &) {
   const pint_t pc = static_cast<pint_t>(getReg(UNW_REG_IP));
   // The PC might contain an invalid address if the unwind info is bad, so
   // directly accessing it could cause a SIGSEGV.
-  if (!isReadableAddr(pc) || !isReadableAddr(pc + 4))
+  if (!isReadableAddr(pc))
     return false;
   const auto *instructions = reinterpret_cast<const uint32_t *>(pc);
   // Look for the two instructions used in the sigreturn trampoline
@@ -2817,7 +2817,7 @@ bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_s390x &) {
   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 SIGSEGV.
-  if (!isReadableAddr(pc) || !isReadableAddr(pc + 2))
+  if (!isReadableAddr(pc))
     return false;
   const auto inst = *reinterpret_cast<const uint16_t *>(pc);
   if (inst == 0x0a77 || inst == 0x0aad) {
@@ -2972,9 +2972,7 @@ bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
   // This code is heavily based on Abseil's 'address_is_readable.cc',
   // which is Copyright Abseil Authors (2017).
 
-  // Align to 8-bytes.
-  const auto alignedAddr = addr & ~pint_t{7};
-  const auto sigsetAddr = reinterpret_cast<sigset_t *>(alignedAddr);
+  const auto sigsetAddr = reinterpret_cast<sigset_t *>(addr);
   // We have to check that addr is nullptr because sigprocmask allows that
   // as an argument without failure.
   if (!sigsetAddr)

>From 9e79919d132a4f8e84c577e5d1a466d6a11fc133 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Tue, 19 Dec 2023 23:04:47 +0000
Subject: [PATCH 11/13] fixup! fixup! [libunwind] Replace process_vm_readv with
 pipe

---
 libunwind/src/UnwindCursor.hpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index fd816f0dd28888..302f141feede19 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2980,11 +2980,11 @@ bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
   // We MUST use a raw syscall here, as wrappers may try to access
   // sigsetAddr which may cause a SIGSEGV. A raw syscall however is
   // safe. Additionally, we need to pass the kernel_sigset_size, which is
-  // different from libc sizeof(sigset_t). 8 seems to work for both 64bit and
-  // 32bit archs.
-  const auto approxKernelSigsetSize = 8;
-  int Result = syscall(SYS_rt_sigprocmask, /*how=*/~0, sigsetAddr, nullptr,
-                       approxKernelSigsetSize);
+  // different from libc sizeof(sigset_t). For the majority of architectures,
+  // it's 64 bits (_NSIG), and libc NSIG is _NSIG + 1.
+  const auto kernelSigsetSize = NSIG / 8;
+  const int Result = syscall(SYS_rt_sigprocmask, /*how=*/~0, sigsetAddr,
+                             nullptr, kernelSigsetSize);
   (void)Result;
   // Because our "how" is invalid, this syscall should always fail, and our
   // errno should always be EINVAL or an EFAULT. EFAULT is not guaranteed

>From 770c0d31c84ea6ea7c852b262fe707aa22ac177e Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Tue, 19 Dec 2023 23:10:42 +0000
Subject: [PATCH 12/13] fixup! fixup! [libunwind] Replace process_vm_readv with
 pipe

---
 libunwind/test/bad_unwind_info.pass.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libunwind/test/bad_unwind_info.pass.cpp b/libunwind/test/bad_unwind_info.pass.cpp
index 2e7cf15f0dcab8..b3284e8daed71f 100644
--- a/libunwind/test/bad_unwind_info.pass.cpp
+++ b/libunwind/test/bad_unwind_info.pass.cpp
@@ -26,7 +26,7 @@
 __attribute__((naked)) void bad_unwind_info() {
 #if defined(__aarch64__)
   __asm__("// not using 0 because unwinder was already resilient to that\n"
-          "mov     x8, #12\n"
+          "mov     x8, #4\n"
           "stp     x30, x8, [sp, #-16]!\n"
           ".cfi_def_cfa_offset 16\n"
           "// purposely use incorrect offset for x30\n"

>From 70ce62e9b2a718811a252059eb229ed3ce47d890 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead <ajordanr at google.com>
Date: Thu, 21 Dec 2023 22:47:06 +0000
Subject: [PATCH 13/13] fixup! fixup! [libunwind] Replace process_vm_readv with
 pipe

---
 libunwind/src/UnwindCursor.hpp | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 302f141feede19..8517d328bd058b 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2969,31 +2969,31 @@ bool UnwindCursor<A, R>::getFunctionName(char *buf, size_t bufLen,
 #if defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN)
 template <typename A, typename R>
 bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
-  // This code is heavily based on Abseil's 'address_is_readable.cc',
-  // which is Copyright Abseil Authors (2017).
+  // We use SYS_rt_sigprocmask, inspired by Abseil's AddressIsReadable.
 
   const auto sigsetAddr = reinterpret_cast<sigset_t *>(addr);
   // We have to check that addr is nullptr because sigprocmask allows that
   // as an argument without failure.
   if (!sigsetAddr)
     return false;
+  const auto saveErrno = errno;
   // We MUST use a raw syscall here, as wrappers may try to access
   // sigsetAddr which may cause a SIGSEGV. A raw syscall however is
   // safe. Additionally, we need to pass the kernel_sigset_size, which is
   // different from libc sizeof(sigset_t). For the majority of architectures,
   // it's 64 bits (_NSIG), and libc NSIG is _NSIG + 1.
   const auto kernelSigsetSize = NSIG / 8;
-  const int Result = syscall(SYS_rt_sigprocmask, /*how=*/~0, sigsetAddr,
-                             nullptr, kernelSigsetSize);
-  (void)Result;
+  [[maybe_unused]] const int Result = syscall(
+      SYS_rt_sigprocmask, /*how=*/~0, sigsetAddr, nullptr, kernelSigsetSize);
   // Because our "how" is invalid, this syscall should always fail, and our
-  // errno should always be EINVAL or an EFAULT. EFAULT is not guaranteed
-  // by the POSIX standard. Additionally, this relies on the Linux kernel
-  // to check copy_from_user before checking if the "how" argument is
+  // errno should always be EINVAL or an EFAULT. This relies on the Linux
+  // kernel to check copy_from_user before checking if the "how" argument is
   // invalid.
   assert(Result == -1);
   assert(errno == EFAULT || errno == EINVAL);
-  return errno != EFAULT;
+  const auto readable = errno != EFAULT;
+  errno = saveErrno;
+  return readable;
 }
 #endif
 



More information about the libc-commits mailing list