1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-03-31 02:32:49 -05:00

Change strategy for the Arm instruction setting DIT.

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.
This commit is contained in:
Simon Tatham 2025-02-15 15:57:53 +00:00
parent 50e360fc74
commit 965057d6d6
2 changed files with 14 additions and 3 deletions

View File

@ -237,9 +237,16 @@ if(neon)
endif()
test_compile_with_flags(HAVE_ARM_DIT
GNU_FLAGS -march=armv8.4-a
TEST_SOURCE "
int main(void) { asm volatile(\"msr dit, %0\" :: \"r\"(1)); }"
#ifndef __aarch64__
#error make sure this only even tries to work on AArch64
#endif
#include <stdint.h>
int main(void) {
register uint64_t one asm(\"x8\");
one = 1;
asm volatile(\".inst 0xd51b42a8\" :: \"r\"(one));
}"
ADD_SOURCES_IF_SUCCESSFUL enable_dit.c)
set(HAVE_AES_NI ${HAVE_AES_NI} PARENT_SCOPE)

View File

@ -20,5 +20,9 @@ void enable_dit(void)
{
if (!platform_dit_available())
return;
asm volatile("msr dit, %0" :: "r"(1));
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));
}