[lldb] [debugserver] Preserve signing bits on lr in debugserver

[jasonmolenda]

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.

[llvmbot]

@llvm/pr-subscribers-lldb

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/67384.diff

1 Files Affected:

• (modified) lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp (+18-2)

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(

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff feb7b1914d513c709b9e024dfed709bb889cc853 5e588f4163c5d6790ac6b8836558aa3a5bf1ccfc -- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

View the diff from clang-format here.

diff --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index e02ef6353640..e3bd91e8d446 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -180,10 +180,8 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
                    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
+                   m_state.context.gpr.__fp, m_state.context.gpr.__lr,
+                   m_state.context.gpr.__sp, m_state.context.gpr.__pc
 #endif
     );