Re: [PATCH 1/4] kselftests/arm64: add a basic Pointer Authentication test

From: Amit Kachhap
Date: Mon Aug 31 2020 - 04:05:47 EST


Hi Boyan,

On 8/28/20 6:46 PM, Boyan Karatotev wrote:
PAuth signs and verifies return addresses on the stack. It does so by
inserting a Pointer Authentication code (PAC) into some of the unused top
bits of an address. This is achieved by adding paciasp/autiasp instructions
at the beginning and end of a function.

This feature is partially backwards compatible with earlier versions of the
ARM architecture. To coerce the compiler into emitting fully backwards
compatible code the main file is compiled to target an earlier ARM version.
This allows the tests to check for the feature and print meaningful error
messages instead of crashing.

Add a test to verify that corrupting the return address results in a
SIGSEGV on return.

Cc: Shuah Khan <shuah@xxxxxxxxxx>
Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
Cc: Will Deacon <will@xxxxxxxxxx>
Signed-off-by: Boyan Karatotev <boyan.karatotev@xxxxxxx>
---
tools/testing/selftests/arm64/Makefile | 2 +-
.../testing/selftests/arm64/pauth/.gitignore | 1 +
tools/testing/selftests/arm64/pauth/Makefile | 22 ++++++++++++
tools/testing/selftests/arm64/pauth/helper.h | 10 ++++++
tools/testing/selftests/arm64/pauth/pac.c | 32 +++++++++++++++++
.../selftests/arm64/pauth/pac_corruptor.S | 36 +++++++++++++++++++
6 files changed, 102 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/arm64/pauth/.gitignore
create mode 100644 tools/testing/selftests/arm64/pauth/Makefile
create mode 100644 tools/testing/selftests/arm64/pauth/helper.h
create mode 100644 tools/testing/selftests/arm64/pauth/pac.c
create mode 100644 tools/testing/selftests/arm64/pauth/pac_corruptor.S

diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
index 93b567d23c8b..525506fd97b9 100644
--- a/tools/testing/selftests/arm64/Makefile
+++ b/tools/testing/selftests/arm64/Makefile
@@ -4,7 +4,7 @@
ARCH ?= $(shell uname -m 2>/dev/null || echo not)
ifneq (,$(filter $(ARCH),aarch64 arm64))
-ARM64_SUBTARGETS ?= tags signal
+ARM64_SUBTARGETS ?= tags signal pauth
else
ARM64_SUBTARGETS :=
endif
diff --git a/tools/testing/selftests/arm64/pauth/.gitignore b/tools/testing/selftests/arm64/pauth/.gitignore
new file mode 100644
index 000000000000..b557c916720a
--- /dev/null
+++ b/tools/testing/selftests/arm64/pauth/.gitignore
@@ -0,0 +1 @@
+pac
diff --git a/tools/testing/selftests/arm64/pauth/Makefile b/tools/testing/selftests/arm64/pauth/Makefile
new file mode 100644
index 000000000000..785c775e5e41
--- /dev/null
+++ b/tools/testing/selftests/arm64/pauth/Makefile
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2020 ARM Limited
+
+CFLAGS += -mbranch-protection=pac-ret

There is no CFLAGS validation present which may give error with non supported compiler.

Can you add a check something like,

+#check if the compiler works well
+pauth_cc_support := $(shell if ($(CC) $(CFLAGS) -march=armv8.3-a -E -x c /dev/null -o /dev/null 2>&1) then echo "1"; fi)
+
+ifeq ($(pauth_cc_support),1)
TEST_GEN_PROGS := pac
TEST_GEN_FILES := pac_corruptor.o
+endif

include ../../lib.mk

+ifeq ($(pauth_cc_support),1)
# pac* and aut* instructions are not available on architectures berfore
# ARMv8.3. Therefore target ARMv8.3 wherever they are used directly
$(OUTPUT)/pac_corruptor.o: pac_corruptor.S
@@ -19,4 +25,4 @@ $(OUTPUT)/pac_corruptor.o: pac_corruptor.S
# run on earlier targets and print a meaningful error messages
$(OUTPUT)/pac: pac.c $(OUTPUT)/pac_corruptor.o
$(CC) $^ -o $@ $(CFLAGS) -march=armv8.2-a
-
+endif

+
+TEST_GEN_PROGS := pac
+TEST_GEN_FILES := pac_corruptor.o
+
+include ../../lib.mk
+
+# pac* and aut* instructions are not available on architectures berfore
+# ARMv8.3. Therefore target ARMv8.3 wherever they are used directly
+$(OUTPUT)/pac_corruptor.o: pac_corruptor.S
+ $(CC) -c $^ -o $@ $(CFLAGS) -march=armv8.3-a
+
+# when -mbranch-protection is enabled and the target architecture is ARMv8.3 or
+# greater, gcc emits pac* instructions which are not in HINT NOP space,
+# preventing the tests from occurring at all. Compile for ARMv8.2 so tests can
+# run on earlier targets and print a meaningful error messages
+$(OUTPUT)/pac: pac.c $(OUTPUT)/pac_corruptor.o
+ $(CC) $^ -o $@ $(CFLAGS) -march=armv8.2-a
+
diff --git a/tools/testing/selftests/arm64/pauth/helper.h b/tools/testing/selftests/arm64/pauth/helper.h
new file mode 100644
index 000000000000..f777f88acf0a
--- /dev/null
+++ b/tools/testing/selftests/arm64/pauth/helper.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2020 ARM Limited */
+
+#ifndef _HELPER_H_
+#define _HELPER_H_
+
+void pac_corruptor(void);
+
+#endif
+

Please delete extra line at end of file here and other places too.

Other changes look fine for me. After the suggested changes please free to add,
Reviewed-by: Amit Daniel Kachhap <amit.kachhap@xxxxxxx>

Thanks,
Amit Daniel


diff --git a/tools/testing/selftests/arm64/pauth/pac.c b/tools/testing/selftests/arm64/pauth/pac.c
new file mode 100644
index 000000000000..ed445050f621
--- /dev/null
+++ b/tools/testing/selftests/arm64/pauth/pac.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 ARM Limited
+
+#include <sys/auxv.h>
+#include <signal.h>
+