[libunwind] [libunwind][AArch64] Protect PC within libunwind's context. (PR #113368)

Daniel Kiss via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 24 11:37:41 PDT 2024


https://github.com/DanielKristofKiss updated https://github.com/llvm/llvm-project/pull/113368

>From 3dd2f4da57eb164e67fd54b66c09cc8b771ee24f Mon Sep 17 00:00:00 2001
From: Daniel Kiss <daniel.kiss at arm.com>
Date: Wed, 16 Oct 2024 14:48:25 -0700
Subject: [PATCH 1/2] [libunwind][AArch64] Protect PC within libunwind's
 context.

Libunwind manages the regiser context including the program counter
which is used effectively as return address.
To increase the robustness of libunwind let's protect the stored address
with PAC. Since there is no unwind info for this let's use the A key and
the base address of the context/registers as modifier.
__libunwind_Registers_arm64_jumpto can go anywhere where the given buffer
's PC points to. After this patch it needs a signed PC therefore the context
 is more harder to cract outside of libunwind.

The register value is internal to libunwind and the change is not visible
on the the APIs.
w
---
 libunwind/src/Registers.hpp            | 91 ++++++++++++++++++++++++--
 libunwind/src/UnwindRegistersRestore.S |  8 ++-
 libunwind/src/UnwindRegistersSave.S    |  8 ++-
 3 files changed, 99 insertions(+), 8 deletions(-)

diff --git a/libunwind/src/Registers.hpp b/libunwind/src/Registers.hpp
index 861e6b5f6f2c58..91bd95b1306169 100644
--- a/libunwind/src/Registers.hpp
+++ b/libunwind/src/Registers.hpp
@@ -1823,9 +1823,61 @@ extern "C" void *__libunwind_cet_get_jump_target() {
 #endif
 
 class _LIBUNWIND_HIDDEN Registers_arm64 {
+protected:
+  /// The program counter is used effectively as a return address
+  /// when the context is restored therefore protect it with PAC.
+  /// The base address of the context is used with the A key for
+  /// authentication and signing. Return address authentication is
+  /// still managed according to the unwind info.
+  inline uint64_t getAuthSalt() const {
+    return reinterpret_cast<uint64_t>(this);
+  }
+#if defined(_LIBUNWIND_IS_NATIVE_ONLY)
+  // Authenticate the given pointer and return with the raw value
+  // if the authentication is succeeded.
+  inline uint64_t auth(uint64_t ptr, uint64_t salt) const {
+    register uint64_t x17 __asm("x17") = ptr;
+    register uint64_t x16 __asm("x16") = salt;
+    asm("hint  0xc" // autia1716
+        : "+r"(x17)
+        : "r"(x16)
+        :);
+
+    uint64_t checkValue = ptr;
+    // Support for machines without FPAC.
+    // Strip the upper bits with `XPACLRI` and compare with the
+    // authenticated value.
+    asm("mov   x30, %[checkValue]     \r\n"
+        "hint  0x7                    \r\n"
+        "mov   %[checkValue], x30     \r\n"
+        : [checkValue] "+r"(checkValue)
+        :
+        : "x30");
+    if (x17 != checkValue)
+      _LIBUNWIND_ABORT("IP PAC authentication failure");
+    return x17;
+  }
+
+  // Sign the PC with the A-KEY and the current salt.
+  inline void updatePC(uint64_t value) {
+    register uint64_t x17 __asm("x17") = value;
+    register uint64_t x16 __asm("x16") = getAuthSalt();
+    asm("hint 0x8" : "+r"(x17) : "r"(x16)); // pacia1716
+    _registers.__pc = x17;
+  }
+#else //! defined(_LIBUNWIND_IS_NATIVE_ONLY)
+  // Remote unwinding is not supported by this protection.
+  inline uint64_t auth(uint64_t ptr, uint64_t salt) const { return ptr; }
+  inline void updatePC(uint64_t value) { _registers.__pc = value; }
+#endif
+
 public:
   Registers_arm64();
   Registers_arm64(const void *registers);
+  Registers_arm64(const Registers_arm64 &other);
+  Registers_arm64(const Registers_arm64 &&other) = delete;
+  Registers_arm64 &operator=(const Registers_arm64 &other);
+  Registers_arm64 &operator=(Registers_arm64 &&other) = delete;
 
   bool        validRegister(int num) const;
   uint64_t    getRegister(int num) const;
@@ -1845,8 +1897,14 @@ class _LIBUNWIND_HIDDEN Registers_arm64 {
 
   uint64_t  getSP() const         { return _registers.__sp; }
   void      setSP(uint64_t value) { _registers.__sp = value; }
-  uint64_t  getIP() const         { return _registers.__pc; }
-  void      setIP(uint64_t value) { _registers.__pc = value; }
+  uint64_t getIP() const { return auth(_registers.__pc, getAuthSalt()); }
+  void setIP(uint64_t value) {
+    // First authenticate the current value of the IP to ensure the context
+    // is still valid. This also ensure the setIP can't be used for signing
+    // arbitrary values.
+    auth(_registers.__pc, getAuthSalt());
+    updatePC(value);
+  }
   uint64_t  getFP() const         { return _registers.__fp; }
   void      setFP(uint64_t value) { _registers.__fp = value; }
 
@@ -1862,8 +1920,8 @@ class _LIBUNWIND_HIDDEN Registers_arm64 {
 
   GPRs    _registers;
   double  _vectorHalfRegisters[32];
-  // Currently only the lower double in 128-bit vectore registers
-  // is perserved during unwinding.  We could define new register
+  // Currently only the lower double in 128-bit vector registers
+  // is preserved during unwinding.  We could define new register
   // numbers (> 96) which mean whole vector registers, then this
   // struct would need to change to contain whole vector registers.
 };
@@ -1874,6 +1932,8 @@ inline Registers_arm64::Registers_arm64(const void *registers) {
   memcpy(&_registers, registers, sizeof(_registers));
   static_assert(sizeof(GPRs) == 0x110,
                 "expected VFP registers to be at offset 272");
+  // getcontext signs the PC with the base address of the context.
+  updatePC(auth(_registers.__pc, reinterpret_cast<uint64_t>(registers)));
   memcpy(_vectorHalfRegisters,
          static_cast<const uint8_t *>(registers) + sizeof(GPRs),
          sizeof(_vectorHalfRegisters));
@@ -1882,6 +1942,25 @@ inline Registers_arm64::Registers_arm64(const void *registers) {
 inline Registers_arm64::Registers_arm64() {
   memset(&_registers, 0, sizeof(_registers));
   memset(&_vectorHalfRegisters, 0, sizeof(_vectorHalfRegisters));
+  // We don't know the PC but let's sign to indicate we have a valid
+  // register set.
+  updatePC(0);
+}
+
+inline Registers_arm64::Registers_arm64(const Registers_arm64 &other) {
+  memcpy(&_registers, &other._registers, sizeof(_registers));
+  memcpy(&_vectorHalfRegisters, &other._vectorHalfRegisters,
+         sizeof(_vectorHalfRegisters));
+  updatePC(other.getIP());
+}
+
+inline Registers_arm64 &
+Registers_arm64::operator=(const Registers_arm64 &other) {
+  memcpy(&_registers, &other._registers, sizeof(_registers));
+  memcpy(&_vectorHalfRegisters, &other._vectorHalfRegisters,
+         sizeof(_vectorHalfRegisters));
+  updatePC(other.getIP());
+  return *this;
 }
 
 inline bool Registers_arm64::validRegister(int regNum) const {
@@ -1902,7 +1981,7 @@ inline bool Registers_arm64::validRegister(int regNum) const {
 
 inline uint64_t Registers_arm64::getRegister(int regNum) const {
   if (regNum == UNW_REG_IP || regNum == UNW_AARCH64_PC)
-    return _registers.__pc;
+    return getIP();
   if (regNum == UNW_REG_SP || regNum == UNW_AARCH64_SP)
     return _registers.__sp;
   if (regNum == UNW_AARCH64_RA_SIGN_STATE)
@@ -1918,7 +1997,7 @@ inline uint64_t Registers_arm64::getRegister(int regNum) const {
 
 inline void Registers_arm64::setRegister(int regNum, uint64_t value) {
   if (regNum == UNW_REG_IP || regNum == UNW_AARCH64_PC)
-    _registers.__pc = value;
+    setIP(value);
   else if (regNum == UNW_REG_SP || regNum == UNW_AARCH64_SP)
     _registers.__sp = value;
   else if (regNum == UNW_AARCH64_RA_SIGN_STATE)
diff --git a/libunwind/src/UnwindRegistersRestore.S b/libunwind/src/UnwindRegistersRestore.S
index 180a66582f41b5..2b11aadeb2d779 100644
--- a/libunwind/src/UnwindRegistersRestore.S
+++ b/libunwind/src/UnwindRegistersRestore.S
@@ -676,7 +676,13 @@ DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_arm64_jumpto)
   ldp    d28,d29, [x0, #0x1F0]
   ldr    d30,     [x0, #0x200]
   ldr    d31,     [x0, #0x208]
-
+  // Authenticate return address with the address of the context.
+  mov    x16,  x0
+  mov    x17, x30
+  hint 0xc // autia1716
+  mov    x30, x17
+  mov    x16, xzr
+  mov    x17, xzr
   // Finally, restore sp. This must be done after the last read from the
   // context struct, because it is allocated on the stack, and an exception
   // could clobber the de-allocated portion of the stack after sp has been
diff --git a/libunwind/src/UnwindRegistersSave.S b/libunwind/src/UnwindRegistersSave.S
index fab234fcd6f318..c346806dc53609 100644
--- a/libunwind/src/UnwindRegistersSave.S
+++ b/libunwind/src/UnwindRegistersSave.S
@@ -744,7 +744,13 @@ DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext)
   str    x30,     [x0, #0x0F0]
   mov    x1,sp
   str    x1,      [x0, #0x0F8]
-  str    x30,     [x0, #0x100]    // store return address as pc
+  // Sign the return address as pc with the address of the context
+  mov    x17, x30
+  mov    x16, x0
+  hint 0x8 // pacia1716
+  str    x17,     [x0, #0x100]    // store return address as pc
+  mov    x17, xzr
+  mov    x16, xzr
   // skip cpsr
   stp    d0, d1,  [x0, #0x110]
   stp    d2, d3,  [x0, #0x120]

>From 1423c03b1cc31749027b84f9ec063e0b5ba02c3a Mon Sep 17 00:00:00 2001
From: Daniel Kiss <daniel.kiss at arm.com>
Date: Thu, 24 Oct 2024 20:35:44 +0200
Subject: [PATCH 2/2] add volatile to asm() and some line break.

---
 libunwind/src/Registers.hpp | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libunwind/src/Registers.hpp b/libunwind/src/Registers.hpp
index 91bd95b1306169..154d967b07caec 100644
--- a/libunwind/src/Registers.hpp
+++ b/libunwind/src/Registers.hpp
@@ -1838,18 +1838,18 @@ class _LIBUNWIND_HIDDEN Registers_arm64 {
   inline uint64_t auth(uint64_t ptr, uint64_t salt) const {
     register uint64_t x17 __asm("x17") = ptr;
     register uint64_t x16 __asm("x16") = salt;
-    asm("hint  0xc" // autia1716
-        : "+r"(x17)
-        : "r"(x16)
-        :);
+    asm volatile ("hint  0xc" // autia1716
+                  : "+r"(x17)
+                  : "r"(x16)
+                  :);
 
     uint64_t checkValue = ptr;
     // Support for machines without FPAC.
     // Strip the upper bits with `XPACLRI` and compare with the
     // authenticated value.
-    asm("mov   x30, %[checkValue]     \r\n"
-        "hint  0x7                    \r\n"
-        "mov   %[checkValue], x30     \r\n"
+    asm volatile ("mov   x30, %[checkValue]     \r\n" \
+                  "hint  0x7                    \r\n" \
+                  "mov   %[checkValue], x30     \r\n" \
         : [checkValue] "+r"(checkValue)
         :
         : "x30");
@@ -1862,7 +1862,7 @@ class _LIBUNWIND_HIDDEN Registers_arm64 {
   inline void updatePC(uint64_t value) {
     register uint64_t x17 __asm("x17") = value;
     register uint64_t x16 __asm("x16") = getAuthSalt();
-    asm("hint 0x8" : "+r"(x17) : "r"(x16)); // pacia1716
+    asm volatile("hint 0x8" : "+r"(x17) : "r"(x16)); // pacia1716
     _registers.__pc = x17;
   }
 #else //! defined(_LIBUNWIND_IS_NATIVE_ONLY)



More information about the cfe-commits mailing list