[PAC][libunwind][AArch64] Keep LR signed when stored in context struct #171717
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| // and initial signing scheme using __sp as a modifier and potentially | ||
| // involving a second modifier (as described by (__ra_sign.__state & 2)). | ||
|
|
||
| ldr x14, [x0, #0x0F8] // x14 = __sp |
There was a problem hiding this comment.
This mimics existing behavior of Apple's arm64e and Linux's pauthtest introduced in #143230. Particularly, if the pointer is signed, we re-sign that so it is signed with the original signing scheme (rather then with &__pc address as a modifier). After that, we authenticate re-signed pointer at return.
This is bulky and we can probably do it more elegant, for example, do a single auth instead of re-sign and almost immediate auth after that. Here are the reasons why I failed to implement this simpler approach - would be glad to see everyone's thoughts on this.
When editing the jumpto function, I was focused on maintaining the following three properties of current implementation.
- We should not do any loads from stack after
spis restored (see d5323f6). - The only scratch registers which are allowed to be clobbered are x16 and x17, all other registers are restored from the context structure.
- Since we are using hint-encoded instructions to maintain compatibility with old hardware, authenticated return is done with regular
retafter anauti*instruction. I assumed that we must keep these two instructions coupled without inserting anything else between them to reduce the time whenlrremains unsigned.
So, if we want to do a single auth instead of re-sign and auth while meeting the three statements above, we must do auti{a|b}1716 and immediate ret x17. It means that both x16 and x17 are filled with auth-related data to the very end of the function, and we cannot use them to restore sp to meet p.1 above. Using re-sign and additional auth resolves the issue since in this case we use auti{a|b}sp when sp has already been restored.
If we could weaken property 2 (e.g. allow x14 and x15 to be clobbered) or property 3 (e.g. allow code between auti* and ret), we might make the whole implementation much simpler. Particularly, instead of doing a re-sign with almost immediate auth, we might just do one auth. The three limitations listed below do not allow us to have a nice implementation because we need to deal with three conditions for choosing between 5 possible configurations of RA sign state (not signed, signed with IA and no PC modifier, signed with IB and no PC modifier, signed with IA and PC modifier, signed with IB and PC modifier).
kovdan01
force-pushed
the
pauth-libunwind-keep-lr-signed
branch
2 times, most recently
from
71b2db9 to
846b55e
Compare
kovdan01
force-pushed
the
pauth-libunwind-keep-lr-signed
branch
5 times, most recently
from
083ec8a to
26bb649
Compare
There are two ways of return address signing: pac-ret (enabled via `-mbranch-protection=pac-ret`) and ptrauth-returns (enabled as part of Apple's arm64e or experimental pauthtest ABI on Linux). Previously, signed LR was handled in libunwind as follows: 1. For pac-ret, the signed LR value was authenticated, and the resulting unsigned LR value was stored in the context structure. 2. For ptrauth-returns (which is assumed to be a part of a full-fledged PAuth ABI like arm64e or pauthtest), the signed LR value was re-signed using the same key (IB) and address of the `__pc` field in the context structure as a modifier. This patch unifies the signed LR handling logic by keeping LR signed for pac-ret similarly to ptrauth-returns. It makes LR substitution in the context structure harder. Note that LR signed state or signing scheme for pac-ret might differ between stack frames. The behavior differs from ptrauth-returns, which has a fixed signing scheme and is enabled as a part of bigger PAuth ABI which is either present everywhere or not. In order to handle different signing schemes across stack frames, new subfields `__state`, `__second_modifier` and `__use_b_key` of the field `__ra_sign` in the context structure are used. When stored in the context structure, the pointer is resigned with maintaining original key and using the address of the `__pc` field in the structure as a modifier.
kovdan01
force-pushed
the
pauth-libunwind-keep-lr-signed
branch
from
26bb649 to
ef7d080
Compare
|
@llvm/pr-subscribers-libunwind Author: Daniil Kovalev (kovdan01) ChangesThere are two ways of return address signing: pac-ret (enabled via Previously, signed LR was handled in libunwind as follows:
This patch unifies the signed LR handling logic by keeping LR signed for pac-ret similarly to ptrauth-returns. It makes LR substitution in the context structure harder. Note that LR signed state or signing scheme for pac-ret might differ between stack frames. The behavior differs from ptrauth-returns, which has a fixed signing scheme and is enabled as a part of bigger PAuth ABI which is either present everywhere or not. In order to handle different signing schemes across stack frames, new subfields When stored in the context structure, the pointer is resigned with maintaining original key and using the address of the Patch is 34.56 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/171717.diff 9 Files Affected:
diff --git a/libunwind/include/__libunwind_config.h b/libunwind/include/__libunwind_config.h
index 980d11ef5d4f2..3327272c2a6bc 100644
--- a/libunwind/include/__libunwind_config.h
+++ b/libunwind/include/__libunwind_config.h
@@ -73,11 +73,11 @@
# define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC
# elif defined(__aarch64__)
# define _LIBUNWIND_TARGET_AARCH64 1
-#define _LIBUNWIND_CONTEXT_SIZE 67
+#define _LIBUNWIND_CONTEXT_SIZE 69
# if defined(__SEH__)
# define _LIBUNWIND_CURSOR_SIZE 164
# else
-#define _LIBUNWIND_CURSOR_SIZE 79
+#define _LIBUNWIND_CURSOR_SIZE 81
# endif
# define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM64
# elif defined(__arm__)
diff --git a/libunwind/include/libunwind.h b/libunwind/include/libunwind.h
index 56ca7110274a3..6a56548867642 100644
--- a/libunwind/include/libunwind.h
+++ b/libunwind/include/libunwind.h
@@ -642,6 +642,12 @@ enum {
// reserved block
UNW_AARCH64_RA_SIGN_STATE = 34,
+ // The following two registers are not real Dwarf registers and use numbers
+ // which are not occupied by real Dwarf registers according to
+ // https://github.com/ARM-software/abi-aa/blob/2025Q1/aadwarf64/aadwarf64.rst
+ UNW_AARCH64_RA_SIGN_SECOND_MODIFIER = 128,
+ UNW_AARCH64_RA_SIGN_USE_B_KEY = 129,
+
// FP/vector registers
UNW_AARCH64_V0 = 64,
UNW_AARCH64_V1 = 65,
diff --git a/libunwind/src/DwarfInstructions.hpp b/libunwind/src/DwarfInstructions.hpp
index d2822e8be29ef..ad1555db3abd2 100644
--- a/libunwind/src/DwarfInstructions.hpp
+++ b/libunwind/src/DwarfInstructions.hpp
@@ -75,10 +75,8 @@ class DwarfInstructions {
__builtin_unreachable();
}
#if defined(_LIBUNWIND_TARGET_AARCH64)
- static bool isReturnAddressSigned(A &addressSpace, R registers, pint_t cfa,
- PrologInfo &prolog);
- static bool isReturnAddressSignedWithPC(A &addressSpace, R registers,
- pint_t cfa, PrologInfo &prolog);
+ static pint_t getRASignState(A &addressSpace, const R ®isters, pint_t cfa,
+ const PrologInfo &prolog);
#endif
};
@@ -176,34 +174,13 @@ v128 DwarfInstructions<A, R>::getSavedVectorRegister(
}
#if defined(_LIBUNWIND_TARGET_AARCH64)
template <typename A, typename R>
-bool DwarfInstructions<A, R>::isReturnAddressSigned(A &addressSpace,
- R registers, pint_t cfa,
- PrologInfo &prolog) {
- pint_t raSignState;
- auto regloc = prolog.savedRegisters[UNW_AARCH64_RA_SIGN_STATE];
- if (regloc.location == CFI_Parser<A>::kRegisterUnused)
- raSignState = static_cast<pint_t>(regloc.value);
- else
- raSignState = getSavedRegister(addressSpace, registers, cfa, regloc);
-
- // Only bit[0] is meaningful.
- return raSignState & 0x01;
-}
-
-template <typename A, typename R>
-bool DwarfInstructions<A, R>::isReturnAddressSignedWithPC(A &addressSpace,
- R registers,
- pint_t cfa,
- PrologInfo &prolog) {
- pint_t raSignState;
+typename A::pint_t
+DwarfInstructions<A, R>::getRASignState(A &addressSpace, const R ®isters,
+ pint_t cfa, const PrologInfo &prolog) {
auto regloc = prolog.savedRegisters[UNW_AARCH64_RA_SIGN_STATE];
if (regloc.location == CFI_Parser<A>::kRegisterUnused)
- raSignState = static_cast<pint_t>(regloc.value);
- else
- raSignState = getSavedRegister(addressSpace, registers, cfa, regloc);
-
- // Only bit[1] is meaningful.
- return raSignState & 0x02;
+ return static_cast<pint_t>(regloc.value);
+ return getSavedRegister(addressSpace, registers, cfa, regloc);
}
#endif
@@ -302,54 +279,25 @@ int DwarfInstructions<A, R>::stepWithDwarf(A &addressSpace,
isSignalFrame = cieInfo.isSignalFrame;
-#if defined(_LIBUNWIND_TARGET_AARCH64) && \
- !defined(_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING)
- // There are two ways of return address signing: pac-ret (enabled via
- // -mbranch-protection=pac-ret) and ptrauth-returns (enabled as part of
- // Apple's arm64e or experimental pauthtest ABI on Linux). The code
- // below handles signed RA for pac-ret, while ptrauth-returns uses
- // different logic.
- // TODO: unify logic for both cases, see
- // https://github.com/llvm/llvm-project/issues/160110
- //
+#if defined(_LIBUNWIND_TARGET_AARCH64)
// If the target is aarch64 then the return address may have been signed
- // using the v8.3 pointer authentication extensions. The original
- // return address needs to be authenticated before the return address is
- // restored. autia1716 is used instead of autia as autia1716 assembles
- // to a NOP on pre-v8.3a architectures.
- if ((R::getArch() == REGISTERS_ARM64) &&
- isReturnAddressSigned(addressSpace, registers, cfa, prolog) &&
+ // using the v8.3 pointer authentication extensions. In order to
+ // store signed return address in the registers context structure, we
+ // need to save the signing scheme for this address.
+ pint_t raSignState = getRASignState(addressSpace, registers, cfa, prolog);
+ bool isReturnAddressSigned = (raSignState & 1);
+ if ((R::getArch() == REGISTERS_ARM64) && isReturnAddressSigned &&
returnAddress != 0) {
#if !defined(_LIBUNWIND_IS_NATIVE_ONLY)
return UNW_ECROSSRASIGNING;
#else
- register unsigned long long x17 __asm("x17") = returnAddress;
- register unsigned long long x16 __asm("x16") = cfa;
-
- // We use the hint versions of the authentication instructions below to
- // ensure they're assembled by the compiler even for targets with no
- // FEAT_PAuth/FEAT_PAuth_LR support.
- if (isReturnAddressSignedWithPC(addressSpace, registers, cfa, prolog)) {
- register unsigned long long x15 __asm("x15") =
- prolog.ptrAuthDiversifier;
- if (cieInfo.addressesSignedWithBKey) {
- asm("hint 0x27\n\t" // pacm
- "hint 0xe"
- : "+r"(x17)
- : "r"(x16), "r"(x15)); // autib1716
- } else {
- asm("hint 0x27\n\t" // pacm
- "hint 0xc"
- : "+r"(x17)
- : "r"(x16), "r"(x15)); // autia1716
- }
- } else {
- if (cieInfo.addressesSignedWithBKey)
- asm("hint 0xe" : "+r"(x17) : "r"(x16)); // autib1716
- else
- asm("hint 0xc" : "+r"(x17) : "r"(x16)); // autia1716
+ newRegisters.setRegister(UNW_AARCH64_RA_SIGN_STATE, raSignState);
+ if (newRegisters.isReturnAddressSignedWithPC()) {
+ newRegisters.setRegister(UNW_AARCH64_RA_SIGN_SECOND_MODIFIER,
+ prolog.ptrAuthDiversifier);
}
- returnAddress = x17;
+ newRegisters.setRegister(UNW_AARCH64_RA_SIGN_USE_B_KEY,
+ cieInfo.addressesSignedWithBKey ? 1 : 0);
#endif
}
#endif
diff --git a/libunwind/src/Registers.hpp b/libunwind/src/Registers.hpp
index 45a2b0921ea3b..2245ccba61e4f 100644
--- a/libunwind/src/Registers.hpp
+++ b/libunwind/src/Registers.hpp
@@ -1877,49 +1877,179 @@ class _LIBUNWIND_HIDDEN Registers_arm64 {
uint64_t getSP() const { return _registers.__sp; }
void setSP(uint64_t value) { _registers.__sp = value; }
+
uint64_t getIP() const {
uint64_t value = _registers.__pc;
-#if defined(_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING)
+
+ if (!isReturnAddressSigned())
+ return value;
+
+#if !defined(_LIBUNWIND_IS_NATIVE_ONLY)
+ abortCrossRASigning();
+#else
// Note the value of the PC was signed to its address in the register state
// but everyone else expects it to be sign by the SP, so convert on return.
- value = (uint64_t)ptrauth_auth_and_resign((void *)_registers.__pc,
- ptrauth_key_return_address,
- &_registers.__pc,
- ptrauth_key_return_address,
- getSP());
+ register uint64_t x17 __asm("x17") = value;
+ register uint64_t x16 __asm("x16") =
+ reinterpret_cast<uint64_t>(&_registers.__pc);
+ register uint64_t x14 __asm("x14") = getSP();
+ if (isReturnAddressSignedWithPC()) {
+ register uint64_t x15 __asm("x15") =
+ _registers.__ra_sign.__second_modifier;
+ if (isReturnAddressSignedWithBKey()) {
+ asm("hint 0xe \n\t" // autib1716
+ "mov x16, x14\n\t"
+ "hint 0x27 \n\t" // pacm
+ "hint 0xa " // pacib1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x15), "r"(x14));
+ } else {
+ asm("hint 0xc \n\t" // autia1716
+ "mov x16, x14\n\t"
+ "hint 0x27 \n\t" // pacm
+ "hint 0x8 " // pacia1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x15), "r"(x14));
+ }
+ } else {
+ if (isReturnAddressSignedWithBKey()) {
+ asm("hint 0xe \n\t" // autib1716
+ "mov x16, x14\n\t"
+ "hint 0xa " // pacib1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x14));
+ } else {
+ asm("hint 0xc \n\t" // autia1716
+ "mov x16, x14\n\t"
+ "hint 0x8 " // pacia1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x14));
+ }
+ }
+ return x17;
#endif
- return value;
}
+
void setIP(uint64_t value) {
-#if defined(_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING)
+ if (!isReturnAddressSigned()) {
+ _registers.__pc = value;
+ return;
+ }
+
+#if !defined(_LIBUNWIND_IS_NATIVE_ONLY)
+ abortCrossRASigning();
+#else
// Note the value which was set should have been signed with the SP.
// We then resign with the slot we are being stored in to so that both SP
// and LR can't be spoofed at the same time.
- value = (uint64_t)ptrauth_auth_and_resign((void *)value,
- ptrauth_key_return_address,
- getSP(),
- ptrauth_key_return_address,
- &_registers.__pc);
+ register uint64_t x17 __asm("x17") = value;
+ register uint64_t x16 __asm("x16") = getSP();
+ register uint64_t x14 __asm("x14") =
+ reinterpret_cast<uint64_t>(&_registers.__pc);
+ if (isReturnAddressSignedWithPC()) {
+ register uint64_t x15 __asm("x15") =
+ _registers.__ra_sign.__second_modifier;
+ if (isReturnAddressSignedWithBKey()) {
+ asm("hint 0x27 \n\t" // pacm
+ "hint 0xe \n\t" // autib1716
+ "mov x16, x14\n\t"
+ "hint 0xa " // pacib1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x15), "r"(x14));
+ } else {
+ asm("hint 0x27 \n\t" // pacm
+ "hint 0xc \n\t" // autia1716
+ "mov x16, x14\n\t"
+ "hint 0x8 " // pacia1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x15), "r"(x14));
+ }
+ } else {
+ if (isReturnAddressSignedWithBKey()) {
+ asm("hint 0xe \n\t" // autib1716
+ "mov x16, x14\n\t"
+ "hint 0xa " // pacib1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x14));
+ } else {
+ asm("hint 0xc \n\t" // autia1716
+ "mov x16, x14\n\t"
+ "hint 0x8 " // pacia1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x14));
+ }
+ }
+ _registers.__pc = x17;
#endif
- _registers.__pc = value;
}
+
uint64_t getFP() const { return _registers.__fp; }
void setFP(uint64_t value) { _registers.__fp = value; }
-#if defined(_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING)
+ // NOTE: For full-fledged PAuth ABIs like Apple's arm64e and Linux's
+ // pauthtest link_reg_t is __ptrauth-qualified. So, LR is re-signed with
+ // link_reg_t signing scheme after it is authenticated with this function.
+ // When just pac-ret is used, link_reg_t is not __ptrauth-qualified and LR
+ // remains unsigned after authentication.
+ // TODO: avoid exposing unsigned LR in absence of full-fledged PAuth ABI.
void
loadAndAuthenticateLinkRegister(reg_t inplaceAuthedLinkRegister,
link_reg_t *referenceAuthedLinkRegister) {
- // If we are in an arm64/arm64e frame, then the PC should have been signed
- // with the SP
- *referenceAuthedLinkRegister =
- (uint64_t)ptrauth_auth_data((void *)inplaceAuthedLinkRegister,
- ptrauth_key_return_address,
- _registers.__sp);
- }
+ if (!isReturnAddressSigned()) {
+ *referenceAuthedLinkRegister = inplaceAuthedLinkRegister;
+ return;
+ }
+
+#if !defined(_LIBUNWIND_IS_NATIVE_ONLY)
+ abortCrossRASigning();
+#else
+ register reg_t x17 __asm("x17") = inplaceAuthedLinkRegister;
+ register reg_t x16 __asm("x16") = getSP();
+ if (isReturnAddressSignedWithPC()) {
+ register reg_t x15 __asm("x15") = _registers.__ra_sign.__second_modifier;
+ if (isReturnAddressSignedWithBKey()) {
+ asm("hint 0x27\n\t" // pacm
+ "hint 0xe " // autib1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x15));
+ } else {
+ asm("hint 0x27\n\t" // pacm
+ "hint 0xc " // autia1716
+ : "+r"(x17)
+ : "r"(x16), "r"(x15));
+ }
+ } else {
+ if (isReturnAddressSignedWithBKey()) {
+ asm("hint 0xe" : "+r"(x17) : "r"(x16)); // autib1716
+ } else {
+ asm("hint 0xc" : "+r"(x17) : "r"(x16)); // autia1716
+ }
+ }
+ *referenceAuthedLinkRegister = x17;
#endif
+ }
+
+ bool isReturnAddressSigned() const {
+ return _registers.__ra_sign.__state & 1;
+ }
+ bool isReturnAddressSignedWithPC() const {
+ return _registers.__ra_sign.__state & 2;
+ }
+ bool isReturnAddressSignedWithBKey() const {
+ return _registers.__ra_sign.__use_b_key;
+ }
private:
+#if !defined(_LIBUNWIND_IS_NATIVE_ONLY)
+ void abortCrossRASigning() const {
+ // We should never go here since non-null RA signed state is either set
+ // by architecture-specific __unw_getcontext or by stepWithDwarf which
+ // already contains a corresponding check and should have already
+ // emitted the UNW_ECROSSRASIGNING error.
+ _LIBUNWIND_ABORT("UNW_ECROSSRASIGNING");
+ }
+#endif
+
uint64_t lazyGetVG() const;
void zaDisable() const {
@@ -1945,7 +2075,12 @@ class _LIBUNWIND_HIDDEN Registers_arm64 {
uint64_t __lr = 0; // Link register x30
uint64_t __sp = 0; // Stack pointer x31
uint64_t __pc = 0; // Program counter
- uint64_t __ra_sign_state = 0; // RA sign state register
+ struct RASign {
+ uint64_t __state = 0; // RA sign state register
+ uint64_t __second_modifier = 0; // Additional modifier used for RA
+ // signing with FEAT_PAuth_LR
+ uint64_t __use_b_key = 0; // 0 for IA key, 1 for IB key
+ } __ra_sign;
};
struct Misc {
@@ -1971,14 +2106,13 @@ inline Registers_arm64::Registers_arm64(const void *registers) {
static_assert((check_fit<Registers_arm64, unw_context_t>::does_fit),
"arm64 registers do not fit into unw_context_t");
memcpy(&_registers, registers, sizeof(_registers));
- static_assert(sizeof(GPRs) == 0x110,
- "expected VFP registers to be at offset 272");
+ static_assert(sizeof(GPRs) == 0x120,
+ "expected VFP registers to be at offset 288");
memcpy(_vectorHalfRegisters,
static_cast<const uint8_t *>(registers) + sizeof(GPRs),
sizeof(_vectorHalfRegisters));
_misc_registers.__vg = 0;
-#if defined(_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING)
// We have to do some pointer authentication fixups after this copy,
// and as part of that we need to load the source pc without
// authenticating so that we maintain the signature for the resigning
@@ -1987,7 +2121,6 @@ inline Registers_arm64::Registers_arm64(const void *registers) {
memmove(&pcRegister, ((uint8_t *)&_registers) + offsetof(GPRs, __pc),
sizeof(pcRegister));
setIP(pcRegister);
-#endif
}
inline Registers_arm64::Registers_arm64(const Registers_arm64 &other) {
@@ -2008,12 +2141,16 @@ inline bool Registers_arm64::validRegister(int regNum) const {
return true;
if (regNum == UNW_REG_SP)
return true;
+ if (regNum == UNW_AARCH64_RA_SIGN_STATE)
+ return true;
+ if (regNum == UNW_AARCH64_RA_SIGN_SECOND_MODIFIER)
+ return true;
+ if (regNum == UNW_AARCH64_RA_SIGN_USE_B_KEY)
+ return true;
if (regNum < 0)
return false;
if (regNum > 95)
return false;
- if (regNum == UNW_AARCH64_RA_SIGN_STATE)
- return true;
if (regNum == UNW_AARCH64_VG)
return true;
if ((regNum > 32) && (regNum < 64))
@@ -2041,7 +2178,11 @@ inline uint64_t Registers_arm64::getRegister(int regNum) const {
if (regNum == UNW_REG_SP || regNum == UNW_AARCH64_SP)
return _registers.__sp;
if (regNum == UNW_AARCH64_RA_SIGN_STATE)
- return _registers.__ra_sign_state;
+ return _registers.__ra_sign.__state;
+ if (regNum == UNW_AARCH64_RA_SIGN_SECOND_MODIFIER)
+ return _registers.__ra_sign.__second_modifier;
+ if (regNum == UNW_AARCH64_RA_SIGN_USE_B_KEY)
+ return _registers.__ra_sign.__use_b_key;
if (regNum == UNW_AARCH64_FP)
return getFP();
if (regNum == UNW_AARCH64_LR)
@@ -2059,7 +2200,11 @@ inline void Registers_arm64::setRegister(int regNum, uint64_t value) {
else if (regNum == UNW_REG_SP || regNum == UNW_AARCH64_SP)
_registers.__sp = value;
else if (regNum == UNW_AARCH64_RA_SIGN_STATE)
- _registers.__ra_sign_state = value;
+ _registers.__ra_sign.__state = value;
+ else if (regNum == UNW_AARCH64_RA_SIGN_SECOND_MODIFIER)
+ _registers.__ra_sign.__second_modifier = value;
+ else if (regNum == UNW_AARCH64_RA_SIGN_USE_B_KEY)
+ _registers.__ra_sign.__use_b_key = value;
else if (regNum == UNW_AARCH64_FP)
setFP(value);
else if (regNum == UNW_AARCH64_LR)
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index afa0cae790377..60ef722ae08e5 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -1065,7 +1065,7 @@ class UnwindCursor : public AbstractUnwindCursor{
const UnwindInfoSections §s,
uint32_t fdeSectionOffsetHint = 0);
int stepWithDwarfFDE(bool stage2) {
-#if defined(_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING)
+#if defined(_LIBUNWIND_TARGET_AARCH64)
typename R::reg_t rawPC = this->getReg(UNW_REG_IP);
typename R::link_reg_t pc;
_registers.loadAndAuthenticateLinkRegister(rawPC, &pc);
@@ -2735,7 +2735,8 @@ void UnwindCursor<A, R>::setInfoBasedOnIPRegister(bool isReturnAddress) {
#endif
typename R::link_reg_t pc;
-#if defined(_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING)
+#if defined(_LIBUNWIND_TARGET_AARCH64) && \
+ !(defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32))
_registers.loadAndAuthenticateLinkRegister(rawPC, &pc);
#else
pc = rawPC;
@@ -3305,7 +3306,8 @@ void UnwindCursor<A, R>::getInfo(unw_proc_info_t *info) {
template <typename A, typename R>
bool UnwindCursor<A, R>::getFunctionName(char *buf, size_t bufLen,
unw_word_t *offset) {
-#if defined(_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING)
+#if defined(_LIBUNWIND_TARGET_AARCH64) && \
+ !(defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32))
typename R::reg_t rawPC = this->getReg(UNW_REG_IP);
typename R::link_reg_t pc;
_registers.loadAndAuthenticateLinkRegister(rawPC, &pc);
diff --git a/libunwind/src/UnwindLevel1.c b/libunwind/src/UnwindLevel1.c
index 73a27928e91d1..813b17dec98a8 100644
--- a/libunwind/src/UnwindLevel1.c
+++ b/libun...
[truncated]
|
ojhunt
left a comment
ojhunt
left a comment
There was a problem hiding this comment.
I just did an initial, very high level, scan through this and have a few questions in comments.
Most of this is the pure assembly parts of unwinding which I'm not super comfortable reviewing. If you feel it's necessary I can spend some time with pen and paper going through it, but I'm probably much slower than other folk due to my paranoia of missing details.
My only significant concern is the conditional signing behavior. It's not the worst (you're choosing between schemas rather the auth vs no-auth, but it would be nice for there to be some kinds of strict guarantees around the use of the validity and system concerns of these variables and schemas. Im theory the use b key, and similar
Maybe @ahmedbougacha has thoughts on this?
| // lr is signed | ||
| ldr x1, [x0, #0x108] // x1 = __ra_sign.__state | ||
| and x1, x1, #2 | ||
| cbnz x1, .Lauth_with_pc | ||
|
|
||
| // lr is signed without pc as a second modifier | ||
| ldr x1, [x0, #0x118] // x1 = __ra_sign.__use_b_key | ||
| cbnz x1, .Lauth_with_b_key |
There was a problem hiding this comment.
This is worrying to me - is there a reason you want/need to support multiple signing schemas here? We've found such variations create potential (I don't recall ever actual) attacks and have tried to reduce the existence of them.
There was a problem hiding this comment.
This is worrying to me - is there a reason you want/need to support multiple signing schemas here?
@ojhunt The -mbranch-protection=pac-ret by default uses A key and no additional PC-based discrimination, but it can also use B key (with pac-ret+b-key), additional PC-based discrimination (with pac-ret+pc) or both (with pac-ret+b-key+pc). So we have to deal with LR values signing with different schemes. And if we want to keep the "original" signing scheme, we need to have these conditionals.
Also, note that pac-ret, unlike arm64e, might be present for some stack frames and not present for other ones. We might even have different pac-ret schemes for different stack frames. And I've actually tried to compile toolchain with one pac-ret scheme and executables with a different pac-ret scheme, and exception handling did actually work properly (with this patch applied).
There was a problem hiding this comment.
I guess you're choosing between two different signing schemes rather than signed vs unsigned, but nonetheless you should really make the schema flag be authenticated as well
There was a problem hiding this comment.
between two different signing schemes
Between 4 signing schemes actually :)
nonetheless you should really make the schema flag be authenticated as well
Hmm, this makes sense, but it should probably be more complex than authenticating just these flags stored in the context structure. We obtain these values from parsing Dwarf expressions, so if we want the flags to be signed, they need to be also signed in Dwarf-related structures as well. And I suppose that it would be not trivial.
Do you see any harm from security perspective compared to current state of things if we keep the schema flags unsigned? From my point of view, even though the schema flags are now unsigned, things become strictly more secure for pac-ret and remain unchanged for arm64e. Or am I missing smth?
I suppose that if we are doing no harm compared to current state of things, we can think of authenticating these schema flags in a separate PR because this one is already pretty big
@ojhunt I would be very happy if someone with such kind of paranoia could do that check :) I've checked the logic by myself multiple times and also have run llvm-test-suite with multiple pac-ret configurations, but still it's not a strict guarantee of absence of errors |
3 participants
There are two ways of return address signing: pac-ret (enabled via
-mbranch-protection=pac-ret) and ptrauth-returns (enabled as part of Apple's arm64e or experimental pauthtest ABI on Linux).Previously, signed LR was handled in libunwind as follows:
For pac-ret, the signed LR value was authenticated, and the resulting unsigned LR value was stored in the context structure.
For ptrauth-returns (which is assumed to be a part of a full-fledged PAuth ABI like arm64e or pauthtest), the signed LR value was re-signed using the same key (IB) and address of the
__pcfield in the context structure as a modifier.This patch unifies the signed LR handling logic by keeping LR signed for pac-ret similarly to ptrauth-returns. It makes LR substitution in the context structure harder.
Note that LR signed state or signing scheme for pac-ret might differ between stack frames. The behavior differs from ptrauth-returns, which has a fixed signing scheme and is enabled as a part of bigger PAuth ABI which is either present everywhere or not. In order to handle different signing schemes across stack frames, new subfields
__state,__second_modifierand__use_b_keyof the field__ra_signin the context structure are used.When stored in the context structure, the pointer is resigned with maintaining original key and using the address of the
__pcfield in the structure as a modifier.