[Lldb-commits] [lldb] [lldb] [debugserver] Preserve signing bits on lr in debugserver (PR #67384)

[Jason Molenda via lldb-commits]

https://github.com/jasonmolenda created https://github.com/llvm/llvm-project/pull/67384

In https://reviews.llvm.org/D136620 I changed debugserver to stop using the kernel-provided functions
arm_thread_state64_get_{pc,lr,sp,fp} to postprocess those four registers on aarch64 systems after we thread_get_state() them. The kernel stores these four registers with signing internally, either from the inferior process' actual signing, or its own.

When a program had crashed by doing an authenticated BL to an address with improper signing, the inferior process would crash and that improperly signed pc would be given to debugserver via thread_get_state. debugserver would run that through arm_thread_state64_get_pc() and then debugserver would crash when authenticating & stripping the value, on newer Mac hardware.

To avoid debugserver crashing on a crashed inferior process, I switched from using these system functions to strip the values, to simply clearing the bits outright in debugserver.

However, lr is a special case where the inferior may have signed this value (against the stack pointer value at the time).  Or it may not yet have any authentication bits, right after a BL.  In the latter case, the kernel will add its own auth bits for while it is stored inside the kernel.  In the case of a user lr value, we cannot authenticate it in debugserver without knowing the sp value it was signed against (and the way it is signed is not specified by the ABI) so an "improperly" signed lr (whatever that means) won't cause debugserver to crash.

debugserver can thread_get_state the inferior's lr, run it through arm_thread_state64_get_lr(), and get the actual signed 64-bit value that the inferior process is using.  And the specifics of how that lr is signed may be important for debugging the process, instead of how I am currently clearing the auth bits outright.

This patch reverts that change for lr only, and also adds a new logging to debugserver specifically for the four sp/fp/lr/pc values that thread_get_state hands to us, before we process them at all.

>From 5e588f4163c5d6790ac6b8836558aa3a5bf1ccfc Mon Sep 17 00:00:00 2001
From: Jason Molenda <jason at molenda.com>
Date: Mon, 25 Sep 2023 16:01:22 -0700
Subject: [PATCH] [lldb] [debugserver] Preserve signing bits on lr in
 debugserver

In https://reviews.llvm.org/D136620 I changed debugserver to
stop using the kernel-provided functions
arm_thread_state64_get_{pc,lr,sp,fp} to postprocess those four
registers on aarch64 systems after we thread_get_state() them.
The kernel stores these four registers with signing internally,
either from the inferior process' actual signing, or its own.

When a program had crashed by doing an authenticated BL to an address
with improper signing, the inferior process would crash and that
improperly signed pc would be given to debugserver via thread_get_state.
debugserver would run that through arm_thread_state64_get_pc() and
then debugserver would crash when authenticating & stripping the
value, on newer Mac hardware.

To avoid debugserver crashing on a crashed inferior process, I
switched from using these system functions to strip the values, to
simply clearing the bits outright in debugserver.

However, lr is a special case where the inferior may have signed
this value (against the stack pointer value at the time).  Or it
may not yet have any authentication bits, right after a BL.  In the
latter case, the kernel will add its own auth bits for while it is
stored inside the kernel.  In the case of a user lr value, we cannot
authenticate it in debugserver without knowing the sp value it was
signed against (and the way it is signed is not specified by the
ABI) so an "improperly" signed lr (whatever that means) won't cause
debugserver to crash.

debugserver can thread_get_state the inferior's lr, run it through
arm_thread_state64_get_lr(), and get the actual signed 64-bit value
that the inferior process is using.  And the specifics of how that
lr is signed may be important for debugging the process, instead of
how I am currently clearing the auth bits outright.

This patch reverts that change for lr only, and also adds a new
logging to debugserver specifically for the four sp/fp/lr/pc values
that thread_get_state hands to us, before we process them at all.
---
 .../source/MacOSX/arm64/DNBArchImplARM64.cpp  | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index bdecae81d31a71a..e02ef6353640a44 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -169,6 +169,23 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
                          (thread_state_t)&m_state.context.gpr, &count);
   if (DNBLogEnabledForAny(LOG_THREAD)) {
     uint64_t *x = &m_state.context.gpr.__x[0];
+    DNBLogThreaded("thread_get_state signed regs "
+                   "\n   fp=%16.16llx"
+                   "\n   lr=%16.16llx"
+                   "\n   sp=%16.16llx"
+                   "\n   pc=%16.16llx",
+#if __has_feature(ptrauth_calls) && defined(__LP64__)
+                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_fp),
+                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_lr),
+                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_sp),
+                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_pc)
+#else
+                   m_state.context.gpr.__fp,
+                   m_state.context.gpr.__lr, 
+                   m_state.context.gpr.__sp,
+                   m_state.context.gpr.__pc
+#endif
+    );
 
 #if __has_feature(ptrauth_calls) && defined(__LP64__)
     uint64_t log_fp = clear_pac_bits(
@@ -2173,8 +2190,7 @@ bool DNBArchMachARM64::GetRegisterValue(uint32_t set, uint32_t reg,
               reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_pc));
           break;
         case gpr_lr:
-          value->value.uint64 = clear_pac_bits(
-              reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_lr));
+          value->value.uint64 = arm_thread_state64_get_lr(m_state.context.gpr);
           break;
         case gpr_sp:
           value->value.uint64 = clear_pac_bits(