Hi.
PuTTY 0.83 on arm64 is badly broken, crashing immediately with a SIGILL.
It looks like this is a known problem upstream and has been fixed, so
I'd like to backport the patch for it. Simon works at ARM on compilers,
so I trust that he has this right.
ok?
(This is on a Raspberry Pi 3, but I had the same results on other hardware
including VMs):
$ /usr/local/bin/puttygen --help
Illegal instruction (core dumped)
$ /usr/local/bin/plink
Illegal instruction (core dumped)
$ egdb -q --args /usr/local/bin/puttygen --help
Reading symbols from /usr/local/bin/puttygen...
(No debugging symbols found in /usr/local/bin/puttygen)
(gdb) run
Starting program: /usr/local/bin/puttygen --help
Program received signal SIGILL, Illegal instruction.
0x0000001f2f086480 in enable_dit ()
(gdb) bt
#0 0x0000001f2f086480 in enable_dit ()
#1 0x0000001f2f079fb0 in main ()
$ egdb -q /usr/local/bin/plink
Reading symbols from /usr/local/bin/plink...
(No debugging symbols found in /usr/local/bin/plink)
(gdb) run
Starting program: /usr/local/bin/plink
Program received signal SIGILL, Illegal instruction.
0x00000013e6206e54 in enable_dit ()
(gdb) bt
#0 0x00000013e6206e54 in enable_dit ()
#1 0x00000013e61bc0a4 in main ()
With patch:
$ /usr/local/bin/plink --version
plink: Release 0.83
Build platform: 64-bit Unix
Compiler: clang 16.0.6
Source commit: d2c178c49a0ae6fa9ef75ca84fb3c9d0d675ea85
$ /usr/local/bin/plink localhost
The host key is not cached for this server:
localhost (port 22)
You have no guarantee that the server is the computer you
think it is.
The server's ssh-ed25519 key fingerprint is:
ssh-ed25519 255 SHA256:hMEp6fbjRFwJ2njrdT3du0UtPVnxLDJOw96Zff4JTug
[etc]
Index: Makefile
===================================================================
RCS file: /export/cvs/ports/net/putty/Makefile,v
diff -u -p -r1.48 Makefile
--- Makefile 8 Feb 2025 16:09:47 -0000 1.48
+++ Makefile 16 Mar 2025 22:08:53 -0000
@@ -2,6 +2,7 @@ COMMENT-main= SSH and telnet client
COMMENT-gui= PuTTY GUI clients
V= 0.83
+REVISION= 0
DISTNAME= putty-$V
PKGNAME-main= ${DISTNAME}
PKGNAME-gui= ${DISTNAME:S/putty/putty-gui/}
Index: patches/patch-arm-enable_dit-sigill
===================================================================
RCS file: patches/patch-arm-enable_dit-sigill
diff -N patches/patch-arm-enable_dit-sigill
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-arm-enable_dit-sigill 16 Mar 2025 22:46:04 -0000
@@ -0,0 +1,83 @@
+From 965057d6d6c9de9fcf506c75b0a2fad22988c72b Mon Sep 17 00:00:00 2001
+From: Simon Tatham <ana...@pobox.com>
+Date: Sat, 15 Feb 2025 15:57:53 +0000
+Subject: [PATCH] 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.
+---
+ crypto/CMakeLists.txt | 11 +++++++++--
+ crypto/enable_dit.c | 6 +++++-
+ 2 files changed, 14 insertions(+), 3 deletions(-)
+
+diff -ru ../putty-0.83.orig/crypto/CMakeLists.txt ./crypto/CMakeLists.txt
+--- ../putty-0.83.orig/crypto/CMakeLists.txt Sat Feb 1 22:20:18 2025
++++ ./crypto/CMakeLists.txt Mon Mar 17 09:41:58 2025
+@@ -237,9 +237,16 @@
+ 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)
+diff -ru ../putty-0.83.orig/crypto/enable_dit.c ./crypto/enable_dit.c
+--- ../putty-0.83.orig/crypto/enable_dit.c Sat Feb 1 22:20:18 2025
++++ ./crypto/enable_dit.c Mon Mar 17 09:41:58 2025
+@@ -20,5 +20,9 @@
+ {
+ 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));
+ }
--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
