[PATCH 0/1] MR11155: Draft: ntdll: Don't validate arm64 pointer signing when unwinding.
When unwinding through a "pac_sign_lr" opcode, we need to strip out pointer authentication bits from the address in the LR register. Previously this was done with the "autib1716" instruction, which does validate that the address in x17 is properly signed with the B key, given the reference address in x16 (SP). However - Clang has a bug [1] since Clang 19, that has caused it to generate incorrect prologues (if building with "-mbranch-protection=standard"), doing return address signing using the A key, while the unwind info says that the address should be signed using the B key. (The unwind info has no way of signaling which key to use, but the normative form uses the B key.) It also turns out that Windows doesn't do any validation of the signing of the return address when unwinding; Windows can correctly execute and do unwinding in binaries built with the buggy Clang - which might explain why the bug has passed unnoticed for so long. Therefore, doing the same and reducing the strictness here in Wine seems reasonable (in addition to doing it to work around the Clang bug). In addition to failing on unwinding in user binaries built return address signing (e.g. "-mbranch-protection=standard") with a buggy Clang, this issue also hits cases if Wine itself has been built with "-mbranch-protection=standard" in CROSSCFLAGS, with a buggy version of Clang. This is the case on e.g. Ubuntu 26.04, making those builds of Wine fail (either directly on startup, or failing only if attempting to do unwinding in user code), if running on a CPU that supports pointer authentication. --- One reason for initially handling "pac_sign_lr" with the "autib1617" instruction was that this instruction can be encoded as a hint instruction, which any CPU can execute, and CPUs not implementing pointer authentication treats it as a nop. If doing the stripping out of pointer authentication using "xpaci", then we must only execute that instruction on CPUs that support it. There is no flag for IsProcessorFeaturePresent for checking for pointer authentication. (Also, we can't just assemble the instruction as such without enabling pointer authentication support in the assembler - but we can assemble it using ".inst".) Instead, do pointer signing using the regular "pacibsp" instruction (which can be encoded as a hint instruction) and check if it results in changed bits in the address. And in case the B key would happen to be disabled (resulting in no changed bits) while the code erroneously signs using the A key (if that one is enabled), also check if "paciasp" changes any bits. If either of them changes bits, we can be sure that pointer authentication is available and we can execute the "xpaci" instruction. [1] https://github.com/llvm/llvm-project/issues/203852 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11155
From: Martin Storsjö <martin@martin.st> When unwinding through a "pac_sign_lr" opcode, we need to strip out pointer authentication bits from the address in the LR register. Previously this was done with the "autib1716" instruction, which does validate that the address in x17 is properly signed with the B key, given the reference address in x16 (SP). However - Clang has a bug [1] since Clang 19, that has caused it to generate incorrect prologues (if building with "-mbranch-protection=standard"), doing return address signing using the A key, while the unwind info says that the address should be signed using the B key. (The unwind info has no way of signaling which key to use, but the normative form uses the B key.) It also turns out that Windows doesn't do any validation of the signing of the return address when unwinding; Windows can correctly execute and do unwinding in binaries built with the buggy Clang - which might explain why the bug has passed unnoticed for so long. Therefore, doing the same and reducing the strictness here in Wine seems reasonable (in addition to doing it to work around the Clang bug). In addition to failing on unwinding in user binaries built return address signing (e.g. "-mbranch-protection=standard") with a buggy Clang, this issue also hits cases if Wine itself has been built with "-mbranch-protection=standard" in CROSSCFLAGS, with a buggy version of Clang. This is the case on e.g. Ubuntu 26.04, making those builds of Wine fail (either directly on startup, or failing only if attempting to do unwinding in user code), if running on a CPU that supports pointer authentication. --- One reason for initially handling "pac_sign_lr" with the "autib1617" instruction was that this instruction can be encoded as a hint instruction, which any CPU can execute, and CPUs not implementing pointer authentication treats it as a nop. If doing the stripping out of pointer authentication using "xpaci", then we must only execute that instruction on CPUs that support it. There is no flag for IsProcessorFeaturePresent for checking for pointer authentication. (Also, we can't just assemble the instruction as such without enabling pointer authentication support in the assembler - but we can assemble it using ".inst".) Instead, do pointer signing using the regular "pacibsp" instruction (which can be encoded as a hint instruction) and check if it results in changed bits in the address. And in case the B key would happen to be disabled (resulting in no changed bits) while the code erroneously signs using the A key (if that one is enabled), also check if "paciasp" changes any bits. If either of them changes bits, we can be sure that pointer authentication is available and we can execute the "xpaci" instruction. [1] https://github.com/llvm/llvm-project/issues/203852 --- dlls/ntdll/unwind.c | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/dlls/ntdll/unwind.c b/dlls/ntdll/unwind.c index 5b79e48ad85..9babc97a55b 100644 --- a/dlls/ntdll/unwind.c +++ b/dlls/ntdll/unwind.c @@ -378,18 +378,40 @@ static void restore_any_reg( int reg, int count, int type, int pos, ARM64_NT_CON } } +DWORD64 strip_pac( DWORD64 ptr ); +/* Attempt to strip out pointer authentication from a pointer. Don't validate + * the pointer signing, but just strip it out. This requires the xpaci + * instruction, which requires support for pointer authentication extensions. + * There is no IsProcessorFeaturePresent flag for pointer authentication. + * Attempt to sign LR and see if it differs from the original form. If it + * doesn't differ, return the original address as is. If it does differ, + * assume that pointer authentication is in use, and use the xpaci + * instruction. + * + * Assembling the xpaci instruction requires enabling support for the pointer + * authentication extension; but emit it directly using .inst insteaad. + * + * Nominally, Windows only uses the B key for pointer authentication, however + * buggy versions of Clang may generate signing using the A key. Therefore, + * check if either of them changes the address. */ +__ASM_GLOBAL_FUNC( strip_pac, + "mov x1, x30\n\t" /* Back up the real LR */ + "hint #27\n\t" /* pacibsp */ + "mov x2, x30\n\t" /* B key signed LR */ + "mov x30, x1\n\t" /* Restore original LR */ + "hint #25\n\t" /* paciasp */ + "mov x3, x30\n\t" /* A key signed LR */ + "mov x30, x1\n\t" /* Restore original LR */ + "cmp x1, x2\n\t" /* Compare original LR with B signed */ + "ccmp x1, x3, #0, eq\n\t" /* Compare original LR with A signed */ + "b.eq 1f\n\t" /* If all are identical, return input x0 as is */ + /* If signing does change pointers - strip it out with xpaci. */ + ".inst 0xdac143e0\n\t" /* xpaci x0 */ + "1: ret\n\t" ) + static void do_pac_auth( ARM64_NT_CONTEXT *context ) { - register DWORD64 x17 __asm__( "x17" ) = context->Lr; - register DWORD64 x16 __asm__( "x16" ) = context->Sp; - - /* This is the autib1716 instruction. The hint instruction is used here - * as gcc does not assemble autib1716 for pre armv8.3a targets. For - * pre-armv8.3a targets, this is just treated as a hint instruction, which - * is ignored. */ - __asm__( "hint 0xe" : "+r"(x17) : "r"(x16) ); - - context->Lr = x17; + context->Lr = strip_pac( context->Lr ); } static void process_unwind_codes( BYTE *ptr, BYTE *end, ARM64_NT_CONTEXT *context, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11155
Seems like there are suggestions in https://github.com/llvm/llvm-project/issues/203852 to make it easier, so marking it as draft. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11155#note_143117
participants (2)
-
Martin Storsjö -
Martin Storsjö (＠mstorsjo)