[libunwind][AArch64] Protect PC within libunwind's context. #113368
Conversation
|
@llvm/pr-subscribers-libunwind Author: Daniel Kiss (DanielKristofKiss) ChangesLibunwind manages the registers/context including the program counter which is used effectively as a return address. The register set is internal to libunwind and this change is not visible on the APIs. Full diff: https://github.com/llvm/llvm-project/pull/113368.diff 3 Files Affected:
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]
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions hpp,h -- libunwind/include/__libunwind_config.h libunwind/src/Registers.hppView the diff from clang-format here.diff --git a/libunwind/include/__libunwind_config.h b/libunwind/include/__libunwind_config.h
index 0c4f8ec1b..c285ff656 100644
--- a/libunwind/include/__libunwind_config.h
+++ b/libunwind/include/__libunwind_config.h
@@ -73,19 +73,19 @@
# define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC
# elif defined(__aarch64__)
# define _LIBUNWIND_TARGET_AARCH64 1
-# if defined(_LIBUNWIND_AARCH64_PC_PROTECTION)
-# define _LIBUNWIND_CONTEXT_SIZE 67
-# else
-# define _LIBUNWIND_CONTEXT_SIZE 66
-# endif
+#if defined(_LIBUNWIND_AARCH64_PC_PROTECTION)
+#define _LIBUNWIND_CONTEXT_SIZE 67
+#else
+#define _LIBUNWIND_CONTEXT_SIZE 66
+#endif
# if defined(__SEH__)
# define _LIBUNWIND_CURSOR_SIZE 164
# else
-# if defined(_LIBUNWIND_AARCH64_PC_PROTECTION)
-# define _LIBUNWIND_CURSOR_SIZE 79
-# else
-# define _LIBUNWIND_CURSOR_SIZE 78
-# endif
+#if defined(_LIBUNWIND_AARCH64_PC_PROTECTION)
+#define _LIBUNWIND_CURSOR_SIZE 79
+#else
+#define _LIBUNWIND_CURSOR_SIZE 78
+#endif
# endif
# define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM64
# elif defined(__arm__)
|
|
sorry for the debug here, locally I can't reproduce the test failure( macos m1, same toolchain, same build steps ) |
|
I'm not especially familiar with how pointer authentication works with exception unwinding, but this looks OK to me with some minor comments. |
libunwind/src/Registers.hpp
Outdated
| 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 volatile("hint 0xc" // autia1716 |
There was a problem hiding this comment.
This would be better on one line, same as the pacia1716 below.
| 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 |
There was a problem hiding this comment.
"ensure the" should be "ensures that"
libunwind/src/Registers.hpp
Outdated
| } | ||
| #if defined(_LIBUNWIND_IS_NATIVE_ONLY) | ||
| // Authenticate the given pointer and return with the raw value | ||
| // if the authentication is succeeded. |
There was a problem hiding this comment.
"authentication is succeeded" should be either "authentication succeeded" or "authentication is successful".
libunwind/src/Registers.hpp
Outdated
| @@ -1823,9 +1823,48 @@ 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 | |||
There was a problem hiding this comment.
"used effectively" should be "effectively used", or just "used".
libunwind/src/Registers.hpp
Outdated
| // 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()); |
There was a problem hiding this comment.
This potentially introduces a signing gadget to libunwind as you are not checking the result of auth. On systems without FPAC / PAuth2 this is a problem.
There was a problem hiding this comment.
A system without TBI and FPAC could be problematic as signing adds PAC to the top bits ( result = ((ptr<63:56> EOR PAC<63:56>) : bit55 : ptr<54:0>); )
While XPAC's Strip) throws those way. (original_ptr = extfield<63:bottom_PAC_bit> : A<bottom_PAC_bit-1:0>;)
I had a version 88b5127 for that cross checked the auth and xpac result but it failed on bots as they don't have FPAC nor TBI and even PAC disabled but there are bits in the top part of the LR.
xpac drop those bits always regardless of PAC status.
I'm planning to make this configurable, e.g when compiled with -mbranch-protection.
I'm happy to hear more ideas.
There was a problem hiding this comment.
Since we're already having support for pointer authentication in frontend, I think it would make sense to use pauth builtins (e.g. __builtin_ptrauth_auth_and_resign). Then backend will do necessary things depending on the target features (e.g. if FPAC is enabled)
There was a problem hiding this comment.
I have created a version that supports pauth_intrinsics too and made the NOP variant similar to it as possible.
In some configs we can't use the armv8.3 instruction set so for those scenarios I left the NOP space instructions version.
I had to split the signed PC into 2 parts as there is no guarantee for the passes LR doesn't contains significant bits in the location of the PAC bits. Now any value could be in LR as libunwind will preserve it.
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 craft outside of libunwind. The register value is internal to libunwind and the change is not visible on the the APIs.
DanielKristofKiss
force-pushed
the
unwind_pac
branch
from
88b5127 to
114455c
Compare
|
Sorry for the delay, now that we've got the qualifier and various other features upstreamed, I'll be upstreaming our ptrauth hardening for the various runtime libraries in the not too distant future (I'm technically on vacation this week, but it may go up this week, otherwise it should be up as a PR sometime next week) |
libunwind/src/Registers.hpp
Outdated
| } | ||
|
|
||
| // Sign and store the new PC. | ||
| inline void updatePC(uint64_t value) { |
There was a problem hiding this comment.
This is a direct signing oracle.
There was a problem hiding this comment.
This shouldn't manifest as a function as only internally used and should be always inlined, could be improved.
| inline void updatePC(uint64_t value) { | |
| __attribute__((always_inline)) | |
| inline void updatePC(uint64_t value) { |
|
Sorry, I missed this Now that the ptrauth qualifier is available I'm going to be preparing and pushing the darwin libunwind+personality function changes, which are very aggressive in there protection of data and pointers |
Added a descriminator to the buffer. Signed the second part of the PC with the first part. misc fix:wq use hint instead of xplaclri
also move define the descriminator instead of magic value.
| void setIP(uint64_t value) { _registers.__pc = value; } | ||
| uint64_t getIP() const { return authPC(&_registers, getDiscriminator()); } | ||
| void setIP(uint64_t value) { | ||
| // First authenticate the current value of the IP to ensure the context |
There was a problem hiding this comment.
Will you please expand how this would work? As far as I can see, the value above comes from outside (e.g. from setRegister call). Is this just saved unsigned PC that is located somewhere on the stack? So it could be substituted and then organized in ROP-like chain during unwinding process?
|
I've finally got our downstream hardening to the point where it should be sane: #143230 |
Libunwind manages the registers/context including the program counter which is used effectively as a return address.
__libunwind_Registers_arm64_jumpto can go anywhere where the given buffer 's PC points to.
To increase the robustness of libunwind let's protect the stored address with PAC. Let's use the A key and the base address of the context/registers as modifier. After this patch the PC must be signed therefore the context harder to craft outside of libunwind.
The register set is internal to libunwind and this change is not visible on the APIs.