[libunwind] Fix problems caused by combining BTI and GCS#102322
Merged
john-brawn-arm merged 2 commits intollvm:mainfrom Aug 8, 2024
Merged
[libunwind] Fix problems caused by combining BTI and GCS#102322john-brawn-arm merged 2 commits intollvm:mainfrom
john-brawn-arm merged 2 commits intollvm:mainfrom
Conversation
The libunwind assembly files need adjustment in order to work correctly when both BTI and GCS are both enabled (which will be the case when using -mbranch-protection=standard): * __libunwind_Registers_arm64_jumpto can't use br to jump to the return location, instead we need to use gcspush then ret. * Because we indirectly call __libunwind_Registers_arm64_jumpto it needs to start with bit jc. * We need to set the GCS GNU property bit when it's enabled.
Member
|
@llvm/pr-subscribers-libunwind Author: John Brawn (john-brawn-arm) ChangesThe libunwind assembly files need adjustment in order to work correctly when both BTI and GCS are both enabled (which will be the case when using -mbranch-protection=standard):
Full diff: https://github.com/llvm/llvm-project/pull/102322.diff 2 Files Affected:
diff --git a/libunwind/src/UnwindRegistersRestore.S b/libunwind/src/UnwindRegistersRestore.S
index b407221ca45f8..7783c94088f68 100644
--- a/libunwind/src/UnwindRegistersRestore.S
+++ b/libunwind/src/UnwindRegistersRestore.S
@@ -629,6 +629,10 @@ Lnovec:
#elif defined(__aarch64__)
+#if defined(__ARM_FEATURE_GCS_DEFAULT)
+.arch_extension gcs
+#endif
+
//
// extern "C" void __libunwind_Registers_arm64_jumpto(Registers_arm64 *);
//
@@ -680,7 +684,17 @@ DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_arm64_jumpto)
ldr x16, [x0, #0x0F8]
ldp x0, x1, [x0, #0x000] // restore x0,x1
mov sp,x16 // restore sp
- br x30 // jump to pc
+#if defined(__ARM_FEATURE_GCS_DEFAULT)
+ // If GCS is enabled we need to push the address we're returning to onto the
+ // GCS stack. We can't just return using br, as there won't be a BTI landing
+ // pad instruction at the destination.
+ mov x16, #1
+ chkfeat x16
+ cbnz x16, Lnogcs
+ gcspushm x30
+Lnogcs:
+#endif
+ ret // jump to pc
#elif defined(__arm__) && !defined(__APPLE__)
diff --git a/libunwind/src/assembly.h b/libunwind/src/assembly.h
index fb07d04071af3..f8e83e138eff5 100644
--- a/libunwind/src/assembly.h
+++ b/libunwind/src/assembly.h
@@ -82,7 +82,22 @@
#define PPC64_OPD2
#endif
-#if defined(__aarch64__) && defined(__ARM_FEATURE_BTI_DEFAULT)
+#if defined(__aarch64__)
+#if defined(__ARM_FEATURE_GCS_DEFAULT) && defined(__ARM_FEATURE_BTI_DEFAULT)
+// Set BTI, PAC, and GCS gnu property bits
+#define GNU_PROPERTY 7
+// We indirectly branch to __libunwind_Registers_arm64_jumpto from
+// __unw_phase2_resume, so we need to use bti jc.
+#define AARCH64_BTI bti jc
+#elif defined(__ARM_FEATURE_GCS_DEFAULT)
+// Set GCS gnu property bit
+#define GNU_PROPERTY 4
+#elif defined(__ARM_FEATURE_BTI_DEFAULT)
+// Set BTI and PAC gnu property bits
+#define GNU_PROPERTY 3
+#define AARCH64_BTI bti c
+#endif
+#ifdef GNU_PROPERTY
.pushsection ".note.gnu.property", "a" SEPARATOR \
.balign 8 SEPARATOR \
.long 4 SEPARATOR \
@@ -91,12 +106,12 @@
.asciz "GNU" SEPARATOR \
.long 0xc0000000 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_AND */ \
.long 4 SEPARATOR \
- .long 3 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_BTI AND */ \
- /* GNU_PROPERTY_AARCH64_FEATURE_1_PAC */ \
+ .long GNU_PROPERTY SEPARATOR \
.long 0 SEPARATOR \
.popsection SEPARATOR
-#define AARCH64_BTI bti c
-#else
+#endif
+#endif
+#if !defined(AARCH64_BTI)
#define AARCH64_BTI
#endif
|
You can test this locally with the following command:git-clang-format --diff d06303ffc1f3b2023532fd426734e9435f87d038 d64241402d6fc32d34fea649bb65186d259fc8e8 --extensions h -- libunwind/src/assembly.hView the diff from clang-format here.diff --git a/libunwind/src/assembly.h b/libunwind/src/assembly.h
index f8e83e138e..ff004fe887 100644
--- a/libunwind/src/assembly.h
+++ b/libunwind/src/assembly.h
@@ -98,17 +98,12 @@
#define AARCH64_BTI bti c
#endif
#ifdef GNU_PROPERTY
- .pushsection ".note.gnu.property", "a" SEPARATOR \
- .balign 8 SEPARATOR \
- .long 4 SEPARATOR \
- .long 0x10 SEPARATOR \
- .long 0x5 SEPARATOR \
- .asciz "GNU" SEPARATOR \
- .long 0xc0000000 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_AND */ \
- .long 4 SEPARATOR \
- .long GNU_PROPERTY SEPARATOR \
- .long 0 SEPARATOR \
- .popsection SEPARATOR
+.pushsection ".note.gnu.property",
+ "a" SEPARATOR.balign 8 SEPARATOR.long 4 SEPARATOR.long 0x10 SEPARATOR
+ .long 0x5 SEPARATOR.asciz "GNU" SEPARATOR
+ .long 0xc0000000 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_AND */
+ .long 4 SEPARATOR.long GNU_PROPERTY SEPARATOR.long 0 SEPARATOR
+ .popsection SEPARATOR
#endif
#endif
#if !defined(AARCH64_BTI)
@@ -117,12 +112,13 @@
#if !defined(__aarch64__)
#ifdef __ARM_FEATURE_PAC_DEFAULT
- .eabi_attribute Tag_PAC_extension, 2
- .eabi_attribute Tag_PACRET_use, 1
+ .eabi_attribute Tag_PAC_extension,
+ 2 .eabi_attribute Tag_PACRET_use,
+ 1
#endif
#ifdef __ARM_FEATURE_BTI_DEFAULT
- .eabi_attribute Tag_BTI_extension, 1
- .eabi_attribute Tag_BTI_use, 1
+ .eabi_attribute Tag_BTI_extension,
+ 1 .eabi_attribute Tag_BTI_use, 1
#endif
#endif
|
DanielKristofKiss
approved these changes
Aug 7, 2024
Member
DanielKristofKiss
left a comment
DanielKristofKiss
left a comment
There was a problem hiding this comment.
Thanks!
LGTM with a NIT.
Please wait for someone from the unwind group to ack it too.
| #define GNU_PROPERTY 7 | ||
| // We indirectly branch to __libunwind_Registers_arm64_jumpto from | ||
| // __unw_phase2_resume, so we need to use bti jc. | ||
| #define AARCH64_BTI bti jc |
Member
There was a problem hiding this comment.
__unw_getcontext will also get the bti jc so slightly weakens the BTI protection but since paciasp is also bti jc might not be a big problem.
Co-authored-by: Daniel Kiss <daniel.kristof.kiss@gmail.com>
MaskRay
approved these changes
Aug 7, 2024
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 12, 2024
The libunwind assembly files need adjustment in order to work correctly when both BTI and GCS are both enabled (which will be the case when using -mbranch-protection=standard): * __libunwind_Registers_arm64_jumpto can't use br to jump to the return location, instead we need to use gcspush then ret. * Because we indirectly call __libunwind_Registers_arm64_jumpto it needs to start with bti jc. * We need to set the GCS GNU property bit when it's enabled. --------- Co-authored-by: Daniel Kiss <daniel.kristof.kiss@gmail.com> (cherry picked from commit 3952910)
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 20, 2024
The libunwind assembly files need adjustment in order to work correctly when both BTI and GCS are both enabled (which will be the case when using -mbranch-protection=standard): * __libunwind_Registers_arm64_jumpto can't use br to jump to the return location, instead we need to use gcspush then ret. * Because we indirectly call __libunwind_Registers_arm64_jumpto it needs to start with bti jc. * We need to set the GCS GNU property bit when it's enabled. --------- Co-authored-by: Daniel Kiss <daniel.kristof.kiss@gmail.com> (cherry picked from commit 3952910)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The libunwind assembly files need adjustment in order to work correctly when both BTI and GCS are both enabled (which will be the case when using -mbranch-protection=standard):