mirror of
https://git.tartarus.org/simon/putty.git
synced 2025-04-01 11:12:50 -05:00
965057d6d6
Colin Watson reported that a build failure occurred in the AArch64 Debian build of PuTTY 0.83: gcc now defaults to enabling branch protection using AArch64 pointer authentication, if the target architecture version supports it. Debian's base supported architecture does not, but Armv8.4-A does. So when I changed the compile flags for enable_dit.c to add -march=armv8.4-a, it didn't _just_ allow me to write the 'msr dit, %0' instruction in my asm statement; it also unexpectedly turned on pointer authentication in the containing function, which caused a SIGILL when running on a pre-Armv8.4-A CPU, because although the code correctly skipped the instruction that set DIT, it was already inside enable_dit() at that point and couldn't avoid going through the unsupported 'retaa' instruction which tries to check an auth code on the return address. An obvious approach would be to add -mbranch-protection=none to the compile flags for enable_dit.c. Another approach is to leave the _compiler_ flags alone, and change the architecture in the assembler, either via a fiddly -Wa,... option or by putting a .arch directive inside the asm statement. But both have downsides. Turning off branch protection is fine for the Debian build, but has the unwanted side effect of turning it off (in that one function) even in builds targeting a later architecture which _did_ want branch protection. And changing the assembler's architecture risks changing it _down_ instead of up, again perhaps invalidating other instructions generated by the compiler (like if some later security feature is introduced that gcc also wants to turn on by default). So instead I've taken the much simpler approach of not bothering to change the target architecture at all, and instead generating the move into DIT by hardcoding its actual instruction encoding. This meant I also had to force the input value into a specific register, but I don't think that does any harm (not _even_ wasting an extra instruction in codegen). Now we should avoid interfering with any security features the compiler wants to turn on or off: all of that should be independent of the instruction I really wanted.
29 lines
1.1 KiB
C
29 lines
1.1 KiB
C
/*
|
|
* Enable the PSTATE.DIT flag in AArch64, if available.
|
|
*
|
|
* This guarantees that data-processing instructions (or rather, a
|
|
* long list of specific ones) will have data-independent timing
|
|
* (hence the name). In other words, you want to turn this bit on if
|
|
* you're trying to do constant-time crypto.
|
|
*
|
|
* For maximum performance you'd want to turn this bit back off when
|
|
* doing any CPU-intensive stuff that _isn't_ cryptographic. That
|
|
* seems like a small concern in this code base, and carries the risk
|
|
* of losing track of whether it was on or not, so here we just enable
|
|
* it for the whole process. That's why there's only an enable_dit()
|
|
* function in this file and not a disable_dit() to go with it.
|
|
*/
|
|
|
|
#include "ssh.h"
|
|
|
|
void enable_dit(void)
|
|
{
|
|
if (!platform_dit_available())
|
|
return;
|
|
register uint64_t one asm("x8");
|
|
one = 1;
|
|
// This is the binary encoding of "msr dit, x8". You can check via, e.g.,
|
|
// echo "msr dit,x8" | llvm-mc -triple aarch64 -mattr=+dit -show-encoding
|
|
asm volatile(".inst 0xd51b42a8" :: "r"(one));
|
|
}
|