Normalize ptrauth handling in sanitizer runtime

[asl]

1. Include ptrauth.h if ptrauth_intrinsics language feature is specified (per ptrauth spec, this is what enables ptrauh.h usage and functions like ptrauth_strip)
2. For PAC-RET fallback implement two changes:
   1. Switch to macro, so we can ignore key argument
   2. Ensure the unsigned value is erased from LR, so the possibility of gadget reuse is reduced.

Fixes #100467

[asl]

@sylvestre will you please double check if this fixes the issue you're seeing?

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Anton Korobeynikov (asl)

Changes

1. Include ptrauth.h if ptrauth_intrinsics language feature is specified (per ptrauth spec, this is what enables ptrauh.h usage)
2. For PAC-RET fallback implement two changes:

• Switch to macro, so we can ignore key argument
• Ensure the unsigned value is erased from LR, so the possibility of gadget reuse is reduced.

Fixes #100467

---

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

1 Files Affected:

• (modified) compiler-rt/lib/sanitizer_common/sanitizer_ptrauth.h (+18-16)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_ptrauth.h b/compiler-rt/lib/sanitizer_common/sanitizer_ptrauth.h
index 5200354694851..d228dd33cf938 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_ptrauth.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_ptrauth.h
@@ -9,24 +9,26 @@
 #ifndef SANITIZER_PTRAUTH_H
 #define SANITIZER_PTRAUTH_H
 
-#if __has_feature(ptrauth_calls)
+#if __has_feature(ptrauth_intrinsics)
 #include <ptrauth.h>
 #elif defined(__ARM_FEATURE_PAC_DEFAULT) && !defined(__APPLE__)
-inline unsigned long ptrauth_strip(void* __value, unsigned int __key) {
-  // On the stack the link register is protected with Pointer
-  // Authentication Code when compiled with -mbranch-protection.
-  // Let's stripping the PAC unconditionally because xpaclri is in
-  // the NOP space so will do nothing when it is not enabled or not available.
-  unsigned long ret;
-  asm volatile(
-      "mov x30, %1\n\t"
-      "hint #7\n\t"  // xpaclri
-      "mov %0, x30\n\t"
-      : "=r"(ret)
-      : "r"(__value)
-      : "x30");
-  return ret;
-}
+// On the stack the link register is protected with Pointer
+// Authentication Code when compiled with -mbranch-protection.
+// Let's stripping the PAC unconditionally because xpaclri is in
+// the NOP space so will do nothing when it is not enabled or not available.
+#define ptrauth_strip(__value, __key)     \
+  ({                                      \
+      unsigned long ret;                  \
+      asm volatile(                       \
+        "mov x30, %1\n\t"                 \
+        "hint #7\n\t"                     \
+        "mov %0, x30\n\t"                 \
+        "mov x30, xzr\n\t"                \
+        : "=r"(ret)                       \
+        : "r"(__value)                    \
+        : "x30");                         \
+      ret;                                \
+  })
 #define ptrauth_auth_data(__value, __old_key, __old_data) __value
 #define ptrauth_string_discriminator(__string) ((int)0)
 #else

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[kovdan01]

LGTM

[asl]

/cherry-pick cc4f989

[llvmbot]

/pull-request #100634

[ZequanWu]

With the fix, this is still broken due to invalid pointer conversion:

clang++: warning: argument unused during compilation: '--unwindlib=none' [-Wunused-command-line-argument]
/usr/local/google/home/ayzhao/src/chromium/src/third_party/llvm/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cpp:210:12: error: incompatible integer to pointer conversion assigning to 'void *' from 'unsigned long'
  210 |   Vtable = ptrauth_strip(Vtable, ptrauth_key_cxx_vtable_pointer);
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/ayzhao/src/chromium/src/third_party/llvm/compiler-rt/lib/ubsan/../sanitizer_common/sanitizer_ptrauth.h:20:5: note: expanded from macro 'ptrauth_strip'
   20 |     ({                                  \
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   21 |       unsigned long ret;                \
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   22 |       asm volatile(                     \
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   23 |           "mov x30, %1\n\t"             \
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   24 |           "hint #7\n\t"                 \
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   25 |           "mov %0, x30\n\t"             \
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   26 |           "mov x30, xzr\n\t"            \
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   27 |           : "=r"(ret)                   \
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   28 |           : "r"(__value)                \
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   29 |           : "x30");                     \
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   30 |       ret;                              \
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   31 |     })
      |     ~~
1 error generated.

[asl]

@ZequanWu

With the fix, this is still broken due to invalid pointer conversion:

Oh. Thanks for noticing this! I believe we fixed this issue... But apparently, not final version of patch was submitted. So, to double check, the final version has void *ret variable, does it work for you?

Apparently, there are no buildbots that cover this configuration...

[alanzhao1]

@ZequanWu

With the fix, this is still broken due to invalid pointer conversion:

Oh. Thanks for noticing this! I believe we fixed this issue... But apparently, not final version of patch was submitted. So, to double check, the final version has void *ret variable, does it work for you?

Apparently, there are no buildbots that cover this configuration...

A quick search shows that the preference seems to be to cast the result of ptrauth_strip to the desired type where it's used. I created #100665