* [PATCH v3 00/13] aarch64: branch protection support
@ 2020-05-15 14:40 Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 01/13] elf.h: Add PT_GNU_PROPERTY Szabolcs Nagy
` (12 more replies)
0 siblings, 13 replies; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
Indirect branch target identification (BTI, armv8.5-a) and return
address signing using pointer authentication (PAC-RET, armv8.3-a)
can be used for security hardening against some control flow hijack
attacks.
In gcc these are exposed via -mbranch-protection=bti+pac-ret which
is the same as -mbranch-protection=standard and gcc can be configured
via --enable-standard-branch-protection to use them by default.
BTI requires libc support: it is an opt-in feature per ELF module
via a GNU property note that the dynamic linker has to check and
mprotect the executable pages with PROT_BTI. And libc objects that
are statically linked into user binaries must be BTI compatible
for the GNU property note to be present. (The property note is
handled by linux for static linked executables and for the ld.so.)
PAC-RET does not require libc runtime support, (other than for
_mcount with current -pg code gen), but, just like BTI, it can be
used in libc binaries for security hardening.
v3:
- instead of END_FILE add note in sysdep.h.
- dropped the syscall template patch (END_FILE is not needed).
- PATCH 05: remove END_FILE macros.
- PATCH 05: clarify the GNU_PROPERTY macro and related defines.
- PATCH 09: separate hook for PT_GNU_PROPERTY handling.
- PATCH 09: modified rtld.c and dl-load.c accordingly.
- PATCH 09: rename linkmap->bti_guarded to linkmap->bti.
- PATCH 13: new patch, update _mcount for pac-ret.
- fixed TODOs except for the last two patches, which are written
for current gcc behaviour.
- I'm waiting for a review of PATCH 03 and welcome comments on
the rest of the set, which i consider done unless there are
changes on the gcc or linux side.
v2:
- removed --enable-branch-protection-standard configure option,
branch protection in glibc is enabled based on the compiler default.
- GNU property notes are disabled if compiler/linker has no support.
- pac-ret is enabled based on compiler defaults.
- PATCH 03: cleaner csu/abi-note.c and fix arm/abi-note.S.
- PATCH 04: new (bti config check).
- PATCH 09: drop the umount2 change.
- PATCH 10: use bool instead of int.
- PATCH 10: fix code style and comments.
- PATCH 10: add linux version requirement to description.
- PATCH 11: new (pac-ret config check).
- PATCH 12: only use pac-ret if HAVE_AARCH64_PAC_RET.
- PATCH 12: fix pac-ret use in dl-trampoline.S.
- PATCH 13: use static inline instead of macro, update description.
- addressed some of the reviews from Adhemerval, the remaining ones
are marked as TODO in the descriptions and will require another
test run or agreement on the design.
Ran cross tests in qemu using the linux for-next/bti-user branch of
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
FAIL: elf/tst-audit14
FAIL: elf/tst-audit15
FAIL: elf/tst-audit16
cross test issue. (/dev/stdout is pipe)
FAIL: elf/tst-ldconfig-ld_so_conf-update
cross test issue. (etc/ld.so.cache is missing)
XPASS: elf/tst-protected1a
XPASS: elf/tst-protected1b
UNSUPPORTED: iconv/tst-gconv-init-failure
FAIL: io/ftwtest
cross test issue. (symlink to nfs root)
FAIL: libio/tst-wfile-sync
cross test issue. (stdin is pipe, bug 24668)
UNSUPPORTED: math/test-fesetexcept-traps
UNSUPPORTED: math/test-fexcept-traps
UNSUPPORTED: math/test-nearbyint-except-2
UNSUPPORTED: misc/tst-pkey
FAIL: nptl/test-cond-printers
FAIL: nptl/test-condattr-printers
FAIL: nptl/test-mutex-printers
FAIL: nptl/test-mutexattr-printers
FAIL: nptl/test-rwlock-printers
FAIL: nptl/test-rwlockattr-printers
cross test issue.
FAIL: nptl/tst-cancel7
FAIL: nptl/tst-cancelx7
cross test issue. (racy test, bug 14232)
UNSUPPORTED: posix/tst-spawn4-compat
UNSUPPORTED: resolv/tst-resolv-ai_idn
UNSUPPORTED: resolv/tst-resolv-ai_idn-latin1
Summary of test results:
14 FAIL
4127 PASS
8 UNSUPPORTED
17 XFAIL
2 XPASS
Sudakshina Das (2):
aarch64: Add BTI support to assembly files
aarch64: enable BTI at runtime
Szabolcs Nagy (11):
elf.h: Add PT_GNU_PROPERTY
elf.h: add aarch64 property definitions
Rewrite abi-note.S in C.
aarch64: configure test for BTI support
aarch64: Rename place holder .S files to .c
aarch64: fix swapcontext for BTI
aarch64: fix RTLD_START for BTI
aarch64: configure check for pac-ret code generation
aarch64: Add pac-ret support to assembly files
aarch64: redefine RETURN_ADDRESS to strip PAC
aarch64: fix _mcount for pac-ret
config.h.in | 6 +
csu/{abi-note.S => abi-note.c} | 28 ++--
elf/dl-load.c | 13 ++
elf/elf.h | 7 +
elf/rtld.c | 6 +
sysdeps/aarch64/Makefile | 4 +
.../aarch64/{bsd-_setjmp.S => bsd-_setjmp.c} | 0
.../aarch64/{bsd-setjmp.S => bsd-setjmp.c} | 0
sysdeps/aarch64/configure | 81 ++++++++++
sysdeps/aarch64/configure.ac | 40 +++++
sysdeps/aarch64/crti.S | 10 ++
sysdeps/aarch64/crtn.S | 8 +
sysdeps/aarch64/dl-bti.c | 54 +++++++
sysdeps/aarch64/dl-machine.h | 5 +-
sysdeps/aarch64/dl-prop.h | 145 ++++++++++++++++++
sysdeps/aarch64/dl-tlsdesc.S | 11 ++
sysdeps/aarch64/dl-trampoline.S | 20 +++
sysdeps/aarch64/linkmap.h | 3 +
sysdeps/aarch64/machine-gmon.h | 2 +-
sysdeps/aarch64/{memmove.S => memmove.c} | 0
sysdeps/aarch64/start.S | 1 +
sysdeps/aarch64/sysdep.h | 57 ++++++-
sysdeps/arm/abi-note.S | 8 -
sysdeps/generic/dl-prop.h | 16 +-
sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 ++++
.../unix/sysv/linux/aarch64/cpu-features.c | 3 +
.../unix/sysv/linux/aarch64/cpu-features.h | 2 +
sysdeps/unix/sysv/linux/aarch64/swapcontext.S | 14 +-
sysdeps/x86/dl-prop.h | 6 +
30 files changed, 554 insertions(+), 28 deletions(-)
rename csu/{abi-note.S => abi-note.c} (88%)
rename sysdeps/aarch64/{bsd-_setjmp.S => bsd-_setjmp.c} (100%)
rename sysdeps/aarch64/{bsd-setjmp.S => bsd-setjmp.c} (100%)
create mode 100644 sysdeps/aarch64/dl-bti.c
create mode 100644 sysdeps/aarch64/dl-prop.h
rename sysdeps/aarch64/{memmove.S => memmove.c} (100%)
delete mode 100644 sysdeps/arm/abi-note.S
create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
--
2.17.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 01/13] elf.h: Add PT_GNU_PROPERTY
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 02/13] elf.h: add aarch64 property definitions Szabolcs Nagy
` (11 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
This program header type is already used in binaries on x86 and
aarch64 targets.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
elf/elf.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/elf/elf.h b/elf/elf.h
index 51e9968405..5b5ce37d9e 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -721,6 +721,7 @@ typedef struct
#define PT_GNU_EH_FRAME 0x6474e550 /* GCC .eh_frame_hdr segment */
#define PT_GNU_STACK 0x6474e551 /* Indicates stack executability */
#define PT_GNU_RELRO 0x6474e552 /* Read-only after relocation */
+#define PT_GNU_PROPERTY 0x6474e553 /* GNU property */
#define PT_LOSUNW 0x6ffffffa
#define PT_SUNWBSS 0x6ffffffa /* Sun Specific segment */
#define PT_SUNWSTACK 0x6ffffffb /* Stack segment */
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 02/13] elf.h: add aarch64 property definitions
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 01/13] elf.h: Add PT_GNU_PROPERTY Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-18 15:26 ` Florian Weimer via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 03/13] Rewrite abi-note.S in C Szabolcs Nagy
` (10 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
These property values are specified by the AArch64 ELF ABI and
binutils can create binaries marked with them.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
elf/elf.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/elf/elf.h b/elf/elf.h
index 5b5ce37d9e..197b557d15 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -1319,6 +1319,12 @@ typedef struct
/* Application-specific semantics, hi */
#define GNU_PROPERTY_HIUSER 0xffffffff
+/* AArch64 specific GNU properties. */
+#define GNU_PROPERTY_AARCH64_FEATURE_1_AND 0xc0000000
+
+#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1U << 0)
+#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC (1U << 1)
+
/* The x86 instruction sets indicated by the corresponding bits are
used in program. Their support in the hardware is optional. */
#define GNU_PROPERTY_X86_ISA_1_USED 0xc0000000
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 03/13] Rewrite abi-note.S in C.
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 01/13] elf.h: Add PT_GNU_PROPERTY Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 02/13] elf.h: add aarch64 property definitions Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-18 15:28 ` Florian Weimer via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 04/13] aarch64: configure test for BTI support Szabolcs Nagy
` (9 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
Using C code allows the compiler to add target specific object file
markings based on CFLAGS.
The arm specific abi-note.S is removed and similar object file fix
up will be avoided on AArch64 with standard branch-prtection.
---
csu/{abi-note.S => abi-note.c} | 28 ++++++++++++++++++----------
sysdeps/arm/abi-note.S | 8 --------
2 files changed, 18 insertions(+), 18 deletions(-)
rename csu/{abi-note.S => abi-note.c} (88%)
delete mode 100644 sysdeps/arm/abi-note.S
diff --git a/csu/abi-note.S b/csu/abi-note.c
similarity index 88%
rename from csu/abi-note.S
rename to csu/abi-note.c
index 2b4b5f8824..cd45f17345 100644
--- a/csu/abi-note.S
+++ b/csu/abi-note.c
@@ -53,6 +53,7 @@ offset length contents
identify the earliest release of that OS that supports this ABI.
See abi-tags (top level) for details. */
+#include <stdint.h>
#include <config.h>
#include <abi-tag.h> /* OS-specific ABI tag value */
@@ -60,13 +61,20 @@ offset length contents
name begins with `.note' and creates a PT_NOTE program header entry
pointing at it. */
- .section ".note.ABI-tag", "a"
- .p2align 2
- .long 1f - 0f /* name length */
- .long 3f - 2f /* data length */
- .long 1 /* note type */
-0: .asciz "GNU" /* vendor name */
-1: .p2align 2
-2: .long __ABI_TAG_OS /* note data: the ABI tag */
- .long __ABI_TAG_VERSION
-3: .p2align 2 /* pad out section */
+/* Note: Custom type is used as ElfW(Nhdr) is wrong on 64 bit targets. */
+
+__attribute__ ((used, aligned (4), section (".note.ABI-tag")))
+static const struct
+{
+ int32_t namesz;
+ int32_t descsz;
+ int32_t type;
+ char name[4];
+ int32_t desc[4];
+} __abi_tag = {
+ 4,
+ 16,
+ 1,
+ "GNU",
+ { __ABI_TAG_OS, __ABI_TAG_VERSION }
+};
diff --git a/sysdeps/arm/abi-note.S b/sysdeps/arm/abi-note.S
deleted file mode 100644
index 07bd4c4619..0000000000
--- a/sysdeps/arm/abi-note.S
+++ /dev/null
@@ -1,8 +0,0 @@
-/* Tag_ABI_align8_preserved: This code preserves 8-byte
- alignment in any callee. */
- .eabi_attribute 25, 1
-/* Tag_ABI_align8_needed: This code may require 8-byte alignment from
- the caller. */
- .eabi_attribute 24, 1
-
-#include <csu/abi-note.S>
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 04/13] aarch64: configure test for BTI support
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (2 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 03/13] Rewrite abi-note.S in C Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-25 18:41 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 05/13] aarch64: Add BTI support to assembly files Szabolcs Nagy
` (8 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
Check BTI support in the compiler and linker. The check also
requires READELF that understands the BTI GNU property note.
It is expected to succeed with gcc >=gcc-9 configured with
--enable-standard-branch-protection and binutils >=binutils-2.33.
---
config.h.in | 3 +++
sysdeps/aarch64/configure | 42 ++++++++++++++++++++++++++++++++++++
sysdeps/aarch64/configure.ac | 19 ++++++++++++++++
3 files changed, 64 insertions(+)
diff --git a/config.h.in b/config.h.in
index dea43df438..506b0c416c 100644
--- a/config.h.in
+++ b/config.h.in
@@ -109,6 +109,9 @@
/* AArch64 big endian ABI */
#undef HAVE_AARCH64_BE
+/* AArch64 BTI support enabled. */
+#undef HAVE_AARCH64_BTI
+
/* C-SKY ABI version. */
#undef CSKYABI
diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
index 5bd355a691..70477a7fa5 100644
--- a/sysdeps/aarch64/configure
+++ b/sysdeps/aarch64/configure
@@ -172,3 +172,45 @@ else
config_vars="$config_vars
default-abi = lp64"
fi
+
+# Only consider BTI supported if -mbranch-protection=bti is
+# on by default in the compiler and the linker produces
+# binaries with GNU property notes in PT_GNU_PROPERTY segment.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for BTI support" >&5
+$as_echo_n "checking for BTI support... " >&6; }
+if ${libc_cv_aarch64_bti+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat > conftest.c <<EOF
+void foo (void) { }
+EOF
+ libc_cv_aarch64_bti=no
+ if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; } \
+ && { ac_try='$READELF -lW conftest.so | grep -q GNU_PROPERTY'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; } \
+ && { ac_try='$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; }
+ then
+ libc_cv_aarch64_bti=yes
+ fi
+ rm -rf conftest.*
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
+$as_echo "$libc_cv_aarch64_bti" >&6; }
+if test $libc_cv_aarch64_bti = yes; then
+ $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
+
+fi
diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
index 7851dd4dac..798f494740 100644
--- a/sysdeps/aarch64/configure.ac
+++ b/sysdeps/aarch64/configure.ac
@@ -20,3 +20,22 @@ if test $libc_cv_aarch64_be = yes; then
else
LIBC_CONFIG_VAR([default-abi], [lp64])
fi
+
+# Only consider BTI supported if -mbranch-protection=bti is
+# on by default in the compiler and the linker produces
+# binaries with GNU property notes in PT_GNU_PROPERTY segment.
+AC_CACHE_CHECK([for BTI support], [libc_cv_aarch64_bti], [dnl
+ cat > conftest.c <<EOF
+void foo (void) { }
+EOF
+ libc_cv_aarch64_bti=no
+ if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c]) \
+ && AC_TRY_COMMAND([$READELF -lW conftest.so | grep -q GNU_PROPERTY]) \
+ && AC_TRY_COMMAND([$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"])
+ then
+ libc_cv_aarch64_bti=yes
+ fi
+ rm -rf conftest.*])
+if test $libc_cv_aarch64_bti = yes; then
+ AC_DEFINE(HAVE_AARCH64_BTI)
+fi
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 05/13] aarch64: Add BTI support to assembly files
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (3 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 04/13] aarch64: configure test for BTI support Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-25 18:49 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 06/13] aarch64: Rename place holder .S files to .c Szabolcs Nagy
` (7 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
From: Sudakshina Das <sudi.das@arm.com>
To enable building glibc with branch protection, assembly code
needs BTI landing pads and ELF object file markings in the form
of a GNU property note.
The landing pads are unconditionally added to all functions that
may be indirectly called. When the code segment is not mapped
with PROT_BTI these instructions are nops. They are kept in the
code when BTI is not supported so that the layout of performance
critical code is unchanged across configurations.
The GNU property notes are only added when there is support for
BTI in the toolchain, because old binutils does not handle the
notes right. (Does not know how to merge them nor to put them in
PT_GNU_PROPERTY segment instead of PT_NOTE, and some versions
of binutils emit warnings about the unknown GNU property. In
such cases the produced libc binaries would not have valid
ELF marking so BTI would not be enabled.)
Note: functions using ENTRY or ENTRY_ALIGN now start with an
additional BTI c, so alignment of the following code changes,
but ENTRY_ALIGN_AND_PAD was fixed so there is no change to the
existing code layout. Some string functions may need to be
tuned for optimal performance after this commit.
Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
sysdeps/aarch64/crti.S | 2 ++
sysdeps/aarch64/crtn.S | 2 ++
sysdeps/aarch64/dl-tlsdesc.S | 3 +++
sysdeps/aarch64/dl-trampoline.S | 2 ++
sysdeps/aarch64/start.S | 1 +
sysdeps/aarch64/sysdep.h | 34 ++++++++++++++++++++++++++++++++-
6 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
index 1728eac37a..c346bcad72 100644
--- a/sysdeps/aarch64/crti.S
+++ b/sysdeps/aarch64/crti.S
@@ -75,6 +75,7 @@ call_weak_fn:
.hidden _init
.type _init, %function
_init:
+ BTI_C
stp x29, x30, [sp, -16]!
mov x29, sp
#if PREINIT_FUNCTION_WEAK
@@ -89,5 +90,6 @@ _init:
.hidden _fini
.type _fini, %function
_fini:
+ BTI_C
stp x29, x30, [sp, -16]!
mov x29, sp
diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
index c3e97cc449..0c1ef112c2 100644
--- a/sysdeps/aarch64/crtn.S
+++ b/sysdeps/aarch64/crtn.S
@@ -37,6 +37,8 @@
/* crtn.S puts function epilogues in the .init and .fini sections
corresponding to the prologues in crti.S. */
+#include <sysdep.h>
+
.section .init,"ax",%progbits
ldp x29, x30, [sp], 16
RET
diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
index 557ad1d505..9d96c8632a 100644
--- a/sysdeps/aarch64/dl-tlsdesc.S
+++ b/sysdeps/aarch64/dl-tlsdesc.S
@@ -74,6 +74,7 @@
cfi_startproc
.align 2
_dl_tlsdesc_return:
+ BTI_C
DELOUSE (0)
ldr PTR_REG (0), [x0, #PTR_SIZE]
RET
@@ -95,6 +96,7 @@ _dl_tlsdesc_return:
cfi_startproc
.align 2
_dl_tlsdesc_undefweak:
+ BTI_C
str x1, [sp, #-16]!
cfi_adjust_cfa_offset (16)
DELOUSE (0)
@@ -142,6 +144,7 @@ _dl_tlsdesc_undefweak:
cfi_startproc
.align 2
_dl_tlsdesc_dynamic:
+ BTI_C
DELOUSE (0)
/* Save just enough registers to support fast path, if we fall
diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
index 94e965c096..2cbfa81434 100644
--- a/sysdeps/aarch64/dl-trampoline.S
+++ b/sysdeps/aarch64/dl-trampoline.S
@@ -35,6 +35,7 @@
cfi_startproc
.align 2
_dl_runtime_resolve:
+ BTI_C
/* AArch64 we get called with:
ip0 &PLTGOT[2]
ip1 temp(dl resolver entry point)
@@ -126,6 +127,7 @@ _dl_runtime_resolve:
cfi_startproc
.align 2
_dl_runtime_profile:
+ BTI_C
/* AArch64 we get called with:
ip0 &PLTGOT[2]
ip1 temp(dl resolver entry point)
diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
index d96cf57e2d..75393e1c18 100644
--- a/sysdeps/aarch64/start.S
+++ b/sysdeps/aarch64/start.S
@@ -46,6 +46,7 @@
.globl _start
.type _start,#function
_start:
+ BTI_C
/* Create an initial frame with 0 LR and FP */
mov x29, #0
mov x30, #0
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 604c489170..086fc84b53 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -41,12 +41,42 @@
#define ASM_SIZE_DIRECTIVE(name) .size name,.-name
+/* Branch Target Identitication support. */
+#define BTI_C hint 34
+#define BTI_J hint 36
+
+/* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code. */
+#define FEATURE_1_AND 0xc0000000
+#define FEATURE_1_BTI 1
+#define FEATURE_1_PAC 2
+
+/* Add a NT_GNU_PROPERTY_TYPE_0 note. */
+#define GNU_PROPERTY(type, value) \
+ .section .note.gnu.property, "a"; \
+ .p2align 3; \
+ .word 4; \
+ .word 16; \
+ .word 5; \
+ .asciz "GNU"; \
+ .word type; \
+ .word 4; \
+ .word value; \
+ .word 0; \
+ .text
+
+/* Add GNU property note with the supported features to all asm code
+ where sysdep.h is included. */
+#if defined HAVE_AARCH64_BTI
+GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
+#endif
+
/* Define an entry point visible from C. */
#define ENTRY(name) \
.globl C_SYMBOL_NAME(name); \
.type C_SYMBOL_NAME(name),%function; \
.align 4; \
C_LABEL(name) \
+ BTI_C; \
cfi_startproc; \
CALL_MCOUNT
@@ -56,6 +86,7 @@
.type C_SYMBOL_NAME(name),%function; \
.p2align align; \
C_LABEL(name) \
+ BTI_C; \
cfi_startproc; \
CALL_MCOUNT
@@ -68,10 +99,11 @@
.globl C_SYMBOL_NAME(name); \
.type C_SYMBOL_NAME(name),%function; \
.p2align align; \
- .rep padding; \
+ .rep padding - 1; /* -1 for bti c. */ \
nop; \
.endr; \
C_LABEL(name) \
+ BTI_C; \
cfi_startproc; \
CALL_MCOUNT
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 06/13] aarch64: Rename place holder .S files to .c
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (4 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 05/13] aarch64: Add BTI support to assembly files Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 07/13] aarch64: fix swapcontext for BTI Szabolcs Nagy
` (6 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
The compiler can add required elf markings based on CFLAGS
but the assembler cannot, so using C code for empty files
creates less of a maintenance problem.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
sysdeps/aarch64/{bsd-_setjmp.S => bsd-_setjmp.c} | 0
sysdeps/aarch64/{bsd-setjmp.S => bsd-setjmp.c} | 0
sysdeps/aarch64/{memmove.S => memmove.c} | 0
3 files changed, 0 insertions(+), 0 deletions(-)
rename sysdeps/aarch64/{bsd-_setjmp.S => bsd-_setjmp.c} (100%)
rename sysdeps/aarch64/{bsd-setjmp.S => bsd-setjmp.c} (100%)
rename sysdeps/aarch64/{memmove.S => memmove.c} (100%)
diff --git a/sysdeps/aarch64/bsd-_setjmp.S b/sysdeps/aarch64/bsd-_setjmp.c
similarity index 100%
rename from sysdeps/aarch64/bsd-_setjmp.S
rename to sysdeps/aarch64/bsd-_setjmp.c
diff --git a/sysdeps/aarch64/bsd-setjmp.S b/sysdeps/aarch64/bsd-setjmp.c
similarity index 100%
rename from sysdeps/aarch64/bsd-setjmp.S
rename to sysdeps/aarch64/bsd-setjmp.c
diff --git a/sysdeps/aarch64/memmove.S b/sysdeps/aarch64/memmove.c
similarity index 100%
rename from sysdeps/aarch64/memmove.S
rename to sysdeps/aarch64/memmove.c
--
2.17.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 07/13] aarch64: fix swapcontext for BTI
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (5 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 06/13] aarch64: Rename place holder .S files to .c Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 08/13] aarch64: fix RTLD_START " Szabolcs Nagy
` (5 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
setcontext returns to the specified context via an indirect jump,
so there should be a BTI j.
In case of getcontext (and all other returns_twice functions) the
compiler adds BTI j at the call site, but swapcontext is a normal
c call that is currently not handled specially by the compiler.
So we change swapcontext such that the saved context returns to a
local address that has BTI j and then swapcontext returns to the
caller via a normal RET. For this we save the original return
address in the slot for x1 of the context because x1 need not be
preserved by swapcontext but it is restored when the context saved
by swapcontext is resumed.
The alternative fix (which is done on x86) would make swapcontext
special in the compiler so BTI j is emitted at call sites, on
x86 there is an indirect_return attribute for this, on AArch64
we would have to use returns_twice. It was decided against because
such fix may need user code updates: the attribute has to be added
when swapcontext is called via a function pointer and it breaks
always_inline functions with swapcontext.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
sysdeps/unix/sysv/linux/aarch64/swapcontext.S | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/aarch64/swapcontext.S b/sysdeps/unix/sysv/linux/aarch64/swapcontext.S
index d30c543e6f..f8c66f0ef0 100644
--- a/sysdeps/unix/sysv/linux/aarch64/swapcontext.S
+++ b/sysdeps/unix/sysv/linux/aarch64/swapcontext.S
@@ -28,8 +28,12 @@
.text
ENTRY(__swapcontext)
DELOUSE (0)
- /* Set the value returned when swapcontext() returns in this context. */
- str xzr, [x0, oX0 + 0 * SZREG]
+ /* Set the value returned when swapcontext() returns in this context.
+ And set up x1 to become the return address of the caller, so we
+ can return there with a normal RET instead of an indirect jump. */
+ stp xzr, x30, [x0, oX0 + 0 * SZREG]
+ /* Arrange the oucp context to return to 2f. */
+ adr x30, 2f
stp x18, x19, [x0, oX0 + 18 * SZREG]
stp x20, x21, [x0, oX0 + 20 * SZREG]
@@ -97,5 +101,11 @@ ENTRY(__swapcontext)
1:
b C_SYMBOL_NAME(__syscall_error)
+2:
+ /* The oucp context is restored here via an indirect branch,
+ x1 must be restored too which has the real return address. */
+ BTI_J
+ mov x30, x1
+ RET
PSEUDO_END (__swapcontext)
weak_alias (__swapcontext, swapcontext)
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 08/13] aarch64: fix RTLD_START for BTI
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (6 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 07/13] aarch64: fix swapcontext for BTI Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 09/13] aarch64: enable BTI at runtime Szabolcs Nagy
` (4 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
Tailcalls must use x16 or x17 for the indirect branch instruction
to be compatible with code that uses BTI c at function entries.
(Other forms of indirect branches can only land on BTI j.)
Also added a BTI c at the ELF entry point of rtld, this is not
strictly necessary since the kernel does not use indirect branch
to get there, but it seems safest once building glibc itself with
BTI is supported.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
sysdeps/aarch64/dl-machine.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
index db3335e5ad..70b9ed3925 100644
--- a/sysdeps/aarch64/dl-machine.h
+++ b/sysdeps/aarch64/dl-machine.h
@@ -125,6 +125,8 @@ elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
.globl _dl_start_user \n\
.type _dl_start_user, %function \n\
_start: \n\
+ // bti c \n\
+ hint 34 \n\
mov " PTR "0, " PTR_SP " \n\
bl _dl_start \n\
// returns user entry point in x0 \n\
@@ -178,7 +180,8 @@ _dl_start_user: \n\
adrp x0, _dl_fini \n\
add " PTR "0, " PTR "0, #:lo12:_dl_fini \n\
// jump to the user_s entry point \n\
- br x21 \n\
+ mov x16, x21 \n\
+ br x16 \n\
");
#define elf_machine_type_class(type) \
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 09/13] aarch64: enable BTI at runtime
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (7 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 08/13] aarch64: fix RTLD_START " Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-25 19:53 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 10/13] aarch64: configure check for pac-ret code generation Szabolcs Nagy
` (3 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
From: Sudakshina Das <sudi.das@arm.com>
Binaries can opt-in to using BTI via an ELF object file marking.
The dynamic linker has to then mprotect the executable segments
with PROT_BTI. In case of static linked executables or in case
of the dynamic linker itself, PROT_BTI protection is done by the
operating system.
On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
the properties of a binary because PT_NOTE can be unreliable with
old linkers (old linkers just append the notes of input objects
together and add them to the output without checking them for
consistency which means multiple incompatible GNU property notes
can be present in PT_NOTE). A new _dl_process_pt_gnu_property
hook is introduced in dl-prop.h and to keep it maintainable the
rtld and dlopen code paths use the same function (if the main
map needs special treatment, that should be inferred by the hook
from the link map). Unlike the _dt_process_pt_note hook this one
is called after segments are mapped to avoid unbounded allocation
and additional read syscall. Otherwise the AArch64 logic follows
the x86 logic for handling GNU properties (but the code is not
shared because x86 needs to manage internal CET state and look
out for multiple property notes).
BTI property is handled in the loader even if glibc is not built
with BTI support, so in theory user code can be BTI protected
independently of glibc. In practice though user binaries are not
marked with the BTI property if glibc has no support because the
static linked libc objects (crt files, libc_nonshared.a) are
unmarked.
This patch relies on Linux userspace API that is scheduled to be
merged in Linux 5.8 and now it is in the for-next/bti-user branch
of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git.
Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
elf/dl-load.c | 13 ++
elf/rtld.c | 6 +
sysdeps/aarch64/Makefile | 4 +
sysdeps/aarch64/dl-bti.c | 54 +++++++
sysdeps/aarch64/dl-prop.h | 145 ++++++++++++++++++
sysdeps/aarch64/linkmap.h | 3 +
sysdeps/generic/dl-prop.h | 16 +-
sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 ++++
.../unix/sysv/linux/aarch64/cpu-features.c | 3 +
.../unix/sysv/linux/aarch64/cpu-features.h | 2 +
sysdeps/x86/dl-prop.h | 6 +
12 files changed, 279 insertions(+), 5 deletions(-)
create mode 100644 sysdeps/aarch64/dl-bti.c
create mode 100644 sysdeps/aarch64/dl-prop.h
create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 06f2ba7264..9c37ec1098 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1188,6 +1188,19 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
maplength, has_holes, loader);
if (__glibc_unlikely (errstring != NULL))
goto call_lose;
+
+ /* Process program headers again after load segments are mapped. */
+ for (ph = phdr; ph < &phdr[l->l_phnum]; ++ph)
+ switch (ph->p_type)
+ {
+ case PT_GNU_PROPERTY:
+ if (_dl_process_pt_gnu_property (l, ph))
+ {
+ errstring = N_("cannot process GNU property segment");
+ goto call_lose;
+ }
+ break;
+ }
}
if (l->l_ld == 0)
diff --git a/elf/rtld.c b/elf/rtld.c
index 5ccc3c2dbb..97a0bbf4dc 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1506,6 +1506,12 @@ of this helper program; chances are you did not intend to run this program.\n\
main_map->l_relro_size = ph->p_memsz;
break;
+ case PT_GNU_PROPERTY:
+ if (_dl_process_pt_gnu_property (main_map, ph))
+ _dl_error_printf (
+"ERROR: '%s': cannot process GNU property segment.\n", _dl_argv[0]);
+ break;
+
case PT_NOTE:
if (_rtld_process_pt_note (main_map, ph))
_dl_error_printf ("\
diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
index 9cb141004d..5ae8b082b0 100644
--- a/sysdeps/aarch64/Makefile
+++ b/sysdeps/aarch64/Makefile
@@ -1,5 +1,9 @@
long-double-fcts = yes
+ifeq ($(subdir),elf)
+sysdep-dl-routines += dl-bti
+endif
+
ifeq ($(subdir),elf)
sysdep-dl-routines += tlsdesc dl-tlsdesc
gen-as-const-headers += dl-link.sym
diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
new file mode 100644
index 0000000000..6003686601
--- /dev/null
+++ b/sysdeps/aarch64/dl-bti.c
@@ -0,0 +1,54 @@
+/* AArch64 BTI functions.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <unistd.h>
+#include <errno.h>
+#include <libintl.h>
+#include <ldsodefs.h>
+
+static int
+enable_bti (struct link_map *map, const char *program)
+{
+ const ElfW(Phdr) *phdr;
+ unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
+
+ for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
+ if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
+ {
+ ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
+ ElfW(Addr) len = phdr->p_memsz;
+ if (__mprotect ((void *) start, len, prot) < 0)
+ {
+ if (program)
+ _dl_fatal_printf ("%s: mprotect failed to turn on BTI\n",
+ map->l_name);
+ else
+ _dl_signal_error (EINVAL, map->l_name, "dlopen",
+ N_("mprotect failed to turn on BTI"));
+ }
+ }
+ return 0;
+}
+
+/* Enable BTI for L if required. */
+
+void
+_dl_bti_check (struct link_map *l, const char *program)
+{
+ if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti)
+ enable_bti (l, program);
+}
diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
new file mode 100644
index 0000000000..b6f8a88667
--- /dev/null
+++ b/sysdeps/aarch64/dl-prop.h
@@ -0,0 +1,145 @@
+/* Support for GNU properties. AArch64 version.
+ Copyright (C) 2018-2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _DL_PROP_H
+#define _DL_PROP_H
+
+#include <not-cancel.h>
+
+extern void _dl_bti_check (struct link_map *, const char *)
+ attribute_hidden;
+
+static inline void __attribute__ ((always_inline))
+_rtld_main_check (struct link_map *m, const char *program)
+{
+ _dl_bti_check (m, program);
+}
+
+static inline void __attribute__ ((always_inline))
+_dl_open_check (struct link_map *m)
+{
+ _dl_bti_check (m, NULL);
+}
+
+static inline void __attribute__ ((unused))
+_dl_process_aarch64_property (struct link_map *l,
+ const ElfW(Nhdr) *note,
+ const ElfW(Addr) size,
+ const ElfW(Addr) align)
+{
+ /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
+ 32-bit objects and to 8 bytes in 64-bit objects. Skip notes
+ with incorrect alignment. */
+ if (align != (__ELF_NATIVE_CLASS / 8))
+ return;
+
+ const ElfW(Addr) start = (ElfW(Addr)) note;
+
+ unsigned int feature_1 = 0;
+ unsigned int last_type = 0;
+
+ while ((ElfW(Addr)) (note + 1) - start < size)
+ {
+ /* Find the NT_GNU_PROPERTY_TYPE_0 note. */
+ if (note->n_namesz == 4
+ && note->n_type == NT_GNU_PROPERTY_TYPE_0
+ && memcmp (note + 1, "GNU", 4) == 0)
+ {
+ /* Check for invalid property. */
+ if (note->n_descsz < 8
+ || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
+ return;
+
+ /* Start and end of property array. */
+ unsigned char *ptr = (unsigned char *) (note + 1) + 4;
+ unsigned char *ptr_end = ptr + note->n_descsz;
+
+ do
+ {
+ unsigned int type = *(unsigned int *) ptr;
+ unsigned int datasz = *(unsigned int *) (ptr + 4);
+
+ /* Property type must be in ascending order. */
+ if (type < last_type)
+ return;
+
+ ptr += 8;
+ if ((ptr + datasz) > ptr_end)
+ return;
+
+ last_type = type;
+
+ if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
+ {
+ /* The size of GNU_PROPERTY_AARCH64_FEATURE_1_AND is 4
+ bytes. When seeing GNU_PROPERTY_AARCH64_FEATURE_1_AND,
+ we stop the search regardless if its size is correct
+ or not. There is no point to continue if this note
+ is ill-formed. */
+ if (datasz != 4)
+ return;
+
+ feature_1 = *(unsigned int *) ptr;
+ if ((feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI))
+ l->l_mach.bti = true;
+
+ /* Stop if we found the property note. */
+ return;
+ }
+ else if (type > GNU_PROPERTY_AARCH64_FEATURE_1_AND)
+ {
+ /* Stop since property type is in ascending order. */
+ return;
+ }
+
+ /* Check the next property item. */
+ ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
+ }
+ while ((ptr_end - ptr) >= 8);
+ }
+
+ note = ((const void *) note
+ + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
+ align));
+ }
+}
+
+#ifdef FILEBUF_SIZE
+static inline int __attribute__ ((always_inline))
+_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
+ int fd, struct filebuf *fbp)
+{
+ return 0;
+}
+#endif
+
+static inline int __attribute__ ((always_inline))
+_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
+{
+ return 0;
+}
+
+static inline int
+_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
+{
+ const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
+ _dl_process_aarch64_property (l, note, ph->p_memsz, ph->p_align);
+ return 0;
+}
+
+#endif /* _DL_PROP_H */
diff --git a/sysdeps/aarch64/linkmap.h b/sysdeps/aarch64/linkmap.h
index 943a9ee9e4..847a03ace2 100644
--- a/sysdeps/aarch64/linkmap.h
+++ b/sysdeps/aarch64/linkmap.h
@@ -16,8 +16,11 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
+#include <stdbool.h>
+
struct link_map_machine
{
ElfW(Addr) plt; /* Address of .plt */
void *tlsdesc_table; /* Address of TLS descriptor hash table. */
+ bool bti; /* Branch Target Identification is enabled. */
};
diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
index 6b0f2aa95a..4192049739 100644
--- a/sysdeps/generic/dl-prop.h
+++ b/sysdeps/generic/dl-prop.h
@@ -20,11 +20,11 @@
#define _DL_PROP_H
/* The following functions are used by the dynamic loader and the
- dlopen machinery to process PT_NOTE entries in the binary or
- shared object. The notes can be used to change the behaviour of
- the loader, and as such offer a flexible mechanism for hooking in
- various checks related to ABI tags or implementing "flag day" ABI
- transitions. */
+ dlopen machinery to process PT_NOTE and PT_GNU_PROPERTY entries in
+ the binary or shared object. The notes can be used to change the
+ behaviour of the loader, and as such offer a flexible mechanism
+ for hooking in various checks related to ABI tags or implementing
+ "flag day" ABI transitions. */
static inline void __attribute__ ((always_inline))
_rtld_main_check (struct link_map *m, const char *program)
@@ -51,4 +51,10 @@ _rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
return 0;
}
+static inline int __attribute__ ((always_inline))
+_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
+{
+ return 0;
+}
+
#endif /* _DL_PROP_H */
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
index 4ee14b4208..af90d8a626 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
@@ -72,3 +72,4 @@
#define HWCAP2_BF16 (1 << 14)
#define HWCAP2_DGH (1 << 15)
#define HWCAP2_RNG (1 << 16)
+#define HWCAP2_BTI (1 << 17)
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
new file mode 100644
index 0000000000..ecae046344
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
@@ -0,0 +1,31 @@
+/* Definitions for POSIX memory map interface. Linux/AArch64 version.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_MMAN_H
+# error "Never use <bits/mman.h> directly; include <sys/mman.h> instead."
+#endif
+
+/* AArch64 specific definitions, should be in sync with
+ arch/arm64/include/uapi/asm/mman.h. */
+
+#define PROT_BTI 0x10
+
+#include <bits/mman-map-flags-generic.h>
+
+/* Include generic Linux declarations. */
+#include <bits/mman-linux.h>
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
index 896c588fee..b9ab827aca 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
@@ -83,4 +83,7 @@ init_cpu_features (struct cpu_features *cpu_features)
if ((dczid & DCZID_DZP_MASK) == 0)
cpu_features->zva_size = 4 << (dczid & DCZID_BS_MASK);
+
+ /* Check if BTI is supported. */
+ cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
}
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
index 1389cea1b3..a81f186ec2 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
@@ -20,6 +20,7 @@
#define _CPU_FEATURES_AARCH64_H
#include <stdint.h>
+#include <stdbool.h>
#define MIDR_PARTNUM_SHIFT 4
#define MIDR_PARTNUM_MASK (0xfff << MIDR_PARTNUM_SHIFT)
@@ -64,6 +65,7 @@ struct cpu_features
{
uint64_t midr_el1;
unsigned zva_size;
+ bool bti;
};
#endif /* _CPU_FEATURES_AARCH64_H */
diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index 516f88ea80..8649314f9d 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -191,4 +191,10 @@ _rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
return 0;
}
+static inline int __attribute__ ((always_inline))
+_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
+{
+ return 0;
+}
+
#endif /* _DL_PROP_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 10/13] aarch64: configure check for pac-ret code generation
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (8 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 09/13] aarch64: enable BTI at runtime Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-25 21:49 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 11/13] aarch64: Add pac-ret support to assembly files Szabolcs Nagy
` (2 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
Return address signing requires unwinder support, which is
present in libgcc since >=gcc-7, however due to bugs the
support may be broken in <gcc-10 (and similarly there may
be issues in custom unwinders), so pac-ret is not always
safe to use. So in assembly code glibc should only use
pac-ret if the compiler uses it too. Unfortunately there
is no predefined feature macro for it set by the compiler
so pac-ret is inferred from the code generation.
---
config.h.in | 3 +++
sysdeps/aarch64/configure | 39 ++++++++++++++++++++++++++++++++++++
sysdeps/aarch64/configure.ac | 21 +++++++++++++++++++
3 files changed, 63 insertions(+)
diff --git a/config.h.in b/config.h.in
index 506b0c416c..f441470385 100644
--- a/config.h.in
+++ b/config.h.in
@@ -112,6 +112,9 @@
/* AArch64 BTI support enabled. */
#undef HAVE_AARCH64_BTI
+/* AArch64 PAC-RET code generation is enabled. */
+#undef HAVE_AARCH64_PAC_RET
+
/* C-SKY ABI version. */
#undef CSKYABI
diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
index 70477a7fa5..5f1cdf5d9b 100644
--- a/sysdeps/aarch64/configure
+++ b/sysdeps/aarch64/configure
@@ -214,3 +214,42 @@ if test $libc_cv_aarch64_bti = yes; then
$as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
fi
+
+# Check if glibc is built with return address signing, i.e.
+# if -mbranch-protection=pac-ret is on. We need this because
+# pac-ret relies on unwinder support so it's not safe to use
+# it in assembly code unconditionally, but there is no
+# feature test macro for it in gcc.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking if pac-ret is enabled" >&5
+$as_echo_n "checking if pac-ret is enabled... " >&6; }
+if ${libc_cv_aarch64_pac_ret+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat > conftest.c <<EOF
+int bar (void);
+int foo (void) { return bar () + 1; }
+EOF
+ libc_cv_aarch64_pac_ret=no
+ if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -S -o conftest.s conftest.c'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; } \
+ && { ac_try='grep -q -E '\''(hint( | )+25|paciasp)'\'' conftest.s'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; }
+ then
+ libc_cv_aarch64_pac_ret=yes
+ fi
+ rm -rf conftest.*
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_pac_ret" >&5
+$as_echo "$libc_cv_aarch64_pac_ret" >&6; }
+if test $libc_cv_aarch64_pac_ret = yes; then
+ $as_echo "#define HAVE_AARCH64_PAC_RET 1" >>confdefs.h
+
+fi
diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
index 798f494740..8248ecf2ed 100644
--- a/sysdeps/aarch64/configure.ac
+++ b/sysdeps/aarch64/configure.ac
@@ -39,3 +39,24 @@ EOF
if test $libc_cv_aarch64_bti = yes; then
AC_DEFINE(HAVE_AARCH64_BTI)
fi
+
+# Check if glibc is built with return address signing, i.e.
+# if -mbranch-protection=pac-ret is on. We need this because
+# pac-ret relies on unwinder support so it's not safe to use
+# it in assembly code unconditionally, but there is no
+# feature test macro for it in gcc.
+AC_CACHE_CHECK([if pac-ret is enabled], [libc_cv_aarch64_pac_ret], [dnl
+ cat > conftest.c <<EOF
+int bar (void);
+int foo (void) { return bar () + 1; }
+EOF
+ libc_cv_aarch64_pac_ret=no
+ if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS -S -o conftest.s conftest.c]) \
+ && AC_TRY_COMMAND([grep -q -E '\''(hint( | )+25|paciasp)'\'' conftest.s])
+ then
+ libc_cv_aarch64_pac_ret=yes
+ fi
+ rm -rf conftest.*])
+if test $libc_cv_aarch64_pac_ret = yes; then
+ AC_DEFINE(HAVE_AARCH64_PAC_RET)
+fi
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 11/13] aarch64: Add pac-ret support to assembly files
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (9 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 10/13] aarch64: configure check for pac-ret code generation Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-26 11:26 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 12/13] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 13/13] aarch64: fix _mcount for pac-ret Szabolcs Nagy
12 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
Use return address signing in assembly files for functions that save
LR when pac-ret is enabled in the compiler.
The GNU property note for PAC-RET is not meaningful to the dynamic
linker so it is not strictly required, but it may be used to track
the security property of binaries. (The PAC-RET property is only set
if BTI is set too because BTI implies working GNU property support.)
---
sysdeps/aarch64/crti.S | 8 ++++++++
sysdeps/aarch64/crtn.S | 6 ++++++
sysdeps/aarch64/dl-tlsdesc.S | 8 ++++++++
sysdeps/aarch64/dl-trampoline.S | 18 ++++++++++++++++++
sysdeps/aarch64/sysdep.h | 8 +++++++-
5 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
index c346bcad72..02ec7d015e 100644
--- a/sysdeps/aarch64/crti.S
+++ b/sysdeps/aarch64/crti.S
@@ -75,7 +75,11 @@ call_weak_fn:
.hidden _init
.type _init, %function
_init:
+#ifdef HAVE_AARCH64_PAC_RET
+ PACIASP
+#else
BTI_C
+#endif
stp x29, x30, [sp, -16]!
mov x29, sp
#if PREINIT_FUNCTION_WEAK
@@ -90,6 +94,10 @@ _init:
.hidden _fini
.type _fini, %function
_fini:
+#ifdef HAVE_AARCH64_PAC_RET
+ PACIASP
+#else
BTI_C
+#endif
stp x29, x30, [sp, -16]!
mov x29, sp
diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
index 0c1ef112c2..4b93b90411 100644
--- a/sysdeps/aarch64/crtn.S
+++ b/sysdeps/aarch64/crtn.S
@@ -41,8 +41,14 @@
.section .init,"ax",%progbits
ldp x29, x30, [sp], 16
+#ifdef HAVE_AARCH64_PAC_RET
+ AUTIASP
+#endif
RET
.section .fini,"ax",%progbits
ldp x29, x30, [sp], 16
+#ifdef HAVE_AARCH64_PAC_RET
+ AUTIASP
+#endif
RET
diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
index 9d96c8632a..3746dbec17 100644
--- a/sysdeps/aarch64/dl-tlsdesc.S
+++ b/sysdeps/aarch64/dl-tlsdesc.S
@@ -183,6 +183,10 @@ _dl_tlsdesc_dynamic:
callee will trash. */
/* Save the remaining registers that we must treat as caller save. */
+# ifdef HAVE_AARCH64_PAC_RET
+ PACIASP
+ cfi_window_save
+# endif
# define NSAVEXREGPAIRS 8
stp x29, x30, [sp,#-16*NSAVEXREGPAIRS]!
cfi_adjust_cfa_offset (16*NSAVEXREGPAIRS)
@@ -233,6 +237,10 @@ _dl_tlsdesc_dynamic:
cfi_adjust_cfa_offset (-16*NSAVEXREGPAIRS)
cfi_restore (x29)
cfi_restore (x30)
+# ifdef HAVE_AARCH64_PAC_RET
+ AUTIASP
+ cfi_window_save
+# endif
b 1b
cfi_endproc
.size _dl_tlsdesc_dynamic, .-_dl_tlsdesc_dynamic
diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
index 2cbfa81434..53f92d68bf 100644
--- a/sysdeps/aarch64/dl-trampoline.S
+++ b/sysdeps/aarch64/dl-trampoline.S
@@ -127,7 +127,12 @@ _dl_runtime_resolve:
cfi_startproc
.align 2
_dl_runtime_profile:
+# ifdef HAVE_AARCH64_PAC_RET
+ PACIASP
+ cfi_window_save
+# else
BTI_C
+# endif
/* AArch64 we get called with:
ip0 &PLTGOT[2]
ip1 temp(dl resolver entry point)
@@ -239,8 +244,17 @@ _dl_runtime_profile:
cfi_restore(x29)
cfi_restore(x30)
+# ifdef HAVE_AARCH64_PAC_RET
+ add sp, sp, SF_SIZE
+ cfi_adjust_cfa_offset (-SF_SIZE)
+ AUTIASP
+ cfi_window_save
+ add sp, sp, 16
+ cfi_adjust_cfa_offset (-16)
+# else
add sp, sp, SF_SIZE + 16
cfi_adjust_cfa_offset (- SF_SIZE - 16)
+# endif
/* Jump to the newly found address. */
br ip0
@@ -287,6 +301,10 @@ _dl_runtime_profile:
/* LR from within La_aarch64_reg */
ldr lr, [x29, #OFFSET_RG + DL_OFFSET_RG_LR]
cfi_restore(lr)
+# ifdef HAVE_AARCH64_PAC_RET
+ /* Note: LR restored from La_aarch64_reg has no PAC. */
+ cfi_window_save
+# endif
mov sp, x29
cfi_def_cfa_register (sp)
ldr x29, [x29, #0]
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 086fc84b53..c51572a690 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -45,6 +45,10 @@
#define BTI_C hint 34
#define BTI_J hint 36
+/* Return address signing support (pac-ret). */
+#define PACIASP hint 25
+#define AUTIASP hint 29
+
/* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code. */
#define FEATURE_1_AND 0xc0000000
#define FEATURE_1_BTI 1
@@ -66,7 +70,9 @@
/* Add GNU property note with the supported features to all asm code
where sysdep.h is included. */
-#if defined HAVE_AARCH64_BTI
+#if defined HAVE_AARCH64_BTI && defined HAVE_AARCH64_PAC_RET
+GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI|FEATURE_1_PAC)
+#elif defined HAVE_AARCH64_BTI
GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 12/13] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (10 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 11/13] aarch64: Add pac-ret support to assembly files Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-26 11:29 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 13/13] aarch64: fix _mcount for pac-ret Szabolcs Nagy
12 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
RETURN_ADDRESS is used at several places in glibc to mean a valid
code address of the call site, but with pac-ret it has a pointer
authentication code (PAC), so its definition is adjusted.
strip_pac is omitted if glibc is bulit without pac-ret, but it could
be added unconditionally (that's just unnecessary operations).
Inline asm is used instead of __builtin_aarch64_xpaclri since that
is an undocumented builtin and not available in all supported gccs.
Note: such change indicates a problem in the pac-ret design: it
can break code that uses __builtin_return_address and the breakage
is only visible at runtime on a system with pac-ret enabled. It is
not ideal that users need target specific inline asm to fix this up.
For now we can recommend disabling pac-ret where this is a problem,
but gcc might need improvements in this are to make pac-ret usable.
TODO: __builtin_return_address handling with pac-ret:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94891
---
sysdeps/aarch64/sysdep.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index c51572a690..7a70cf7a2b 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -35,6 +35,23 @@
#define PTR_SIZE (1<<PTR_LOG_SIZE)
+#ifndef __ASSEMBLER__
+/* Strip pointer authentication code from pointer p. */
+static inline void *
+strip_pac (void *p)
+{
+ register void *ra asm ("x30") = (p);
+ asm ("hint 7 // xpaclri" : "+r"(ra));
+ return ra;
+}
+
+/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
+# ifdef HAVE_AARCH64_PAC_RET
+# undef RETURN_ADDRESS
+# define RETURN_ADDRESS(n) strip_pac (__builtin_return_address (n))
+# endif
+#endif
+
#ifdef __ASSEMBLER__
/* Syntactic details of assembler. */
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 13/13] aarch64: fix _mcount for pac-ret
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
` (11 preceding siblings ...)
2020-05-15 14:40 ` [PATCH v3 12/13] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
@ 2020-05-15 14:40 ` Szabolcs Nagy
2020-05-26 11:33 ` Adhemerval Zanella via Libc-alpha
12 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-15 14:40 UTC (permalink / raw)
To: libc-alpha
gcc -pg with -mbranch-protection=pac-ret passes signed return address
to _mcount, so _mcount now has to always strip pac from the frompc
since that's from user code that may be built with pac-ret.
This is a backward incompatible _mcount abi change introduced by
return address signing support in gcc-7.
TODO: fix -pg on the gcc side?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94791
---
sysdeps/aarch64/machine-gmon.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h
index 730a23b781..328cbdda16 100644
--- a/sysdeps/aarch64/machine-gmon.h
+++ b/sysdeps/aarch64/machine-gmon.h
@@ -30,5 +30,5 @@ static inline void mcount_internal (u_long frompc, u_long selfpc)
#define MCOUNT \
void __mcount (void *frompc) \
{ \
- mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
+ mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \
}
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 02/13] elf.h: add aarch64 property definitions
2020-05-15 14:40 ` [PATCH v3 02/13] elf.h: add aarch64 property definitions Szabolcs Nagy
@ 2020-05-18 15:26 ` Florian Weimer via Libc-alpha
2020-05-21 8:57 ` Szabolcs Nagy
0 siblings, 1 reply; 31+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-18 15:26 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha
* Szabolcs Nagy:
> These property values are specified by the AArch64 ELF ABI and
> binutils can create binaries marked with them.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> elf/elf.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/elf/elf.h b/elf/elf.h
> index 5b5ce37d9e..197b557d15 100644
> --- a/elf/elf.h
> +++ b/elf/elf.h
> @@ -1319,6 +1319,12 @@ typedef struct
> /* Application-specific semantics, hi */
> #define GNU_PROPERTY_HIUSER 0xffffffff
>
> +/* AArch64 specific GNU properties. */
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_AND 0xc0000000
> +
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1U << 0)
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC (1U << 1)
> +
> /* The x86 instruction sets indicated by the corresponding bits are
> used in program. Their support in the hardware is optional. */
> #define GNU_PROPERTY_X86_ISA_1_USED 0xc0000000
I checked that this matches the binutils definition. I think this can
go in separately.
Thanks,
Florian
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 03/13] Rewrite abi-note.S in C.
2020-05-15 14:40 ` [PATCH v3 03/13] Rewrite abi-note.S in C Szabolcs Nagy
@ 2020-05-18 15:28 ` Florian Weimer via Libc-alpha
2020-05-20 10:27 ` Szabolcs Nagy
0 siblings, 1 reply; 31+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-18 15:28 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha
* Szabolcs Nagy:
> +/* Note: Custom type is used as ElfW(Nhdr) is wrong on 64 bit targets. */
Are you sure about that?
typedef struct
{
Elf32_Word n_namesz; /* Length of the note's name. */
Elf32_Word n_descsz; /* Length of the note's descriptor. */
Elf32_Word n_type; /* Type of the note. */
} Elf32_Nhdr;
typedef struct
{
Elf64_Word n_namesz; /* Length of the note's name. */
Elf64_Word n_descsz; /* Length of the note's descriptor. */
Elf64_Word n_type; /* Type of the note. */
} Elf64_Nhdr;
The types are the same:
typedef uint32_t Elf32_Word;
typedef uint32_t Elf64_Word;
I admit this is super-confusing.
Thanks,
Florian
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 03/13] Rewrite abi-note.S in C.
2020-05-18 15:28 ` Florian Weimer via Libc-alpha
@ 2020-05-20 10:27 ` Szabolcs Nagy
2020-05-20 10:34 ` Florian Weimer via Libc-alpha
0 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-20 10:27 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
The 05/18/2020 17:28, Florian Weimer wrote:
> * Szabolcs Nagy:
>
> > +/* Note: Custom type is used as ElfW(Nhdr) is wrong on 64 bit targets. */
>
> Are you sure about that?
>
> typedef struct
> {
> Elf32_Word n_namesz; /* Length of the note's name. */
> Elf32_Word n_descsz; /* Length of the note's descriptor. */
> Elf32_Word n_type; /* Type of the note. */
> } Elf32_Nhdr;
>
> typedef struct
> {
> Elf64_Word n_namesz; /* Length of the note's name. */
> Elf64_Word n_descsz; /* Length of the note's descriptor. */
> Elf64_Word n_type; /* Type of the note. */
> } Elf64_Nhdr;
>
> The types are the same:
>
> typedef uint32_t Elf32_Word;
> typedef uint32_t Elf64_Word;
>
> I admit this is super-confusing.
aah i missed this, then the struct can be
#include <elf.h>
__attribute__ ((used, aligned (4), section (".note.ABI-tag")))
static const struct
{
ElfW(Nhdr) nhdr;
char name[4];
int32_t desc[4];
} __abi_tag = {
{ .n_namesz = 4, .n_descsz = 16, .n_type = 1 },
"GNU",
{ __ABI_TAG_OS, __ABI_TAG_VERSION }
};
i think the aligned attribute is still needed in case on
some target uint32_t has smaller alignemnt than 4.
i can change this if it is considered to be better
or just remove the incorrect comment.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 03/13] Rewrite abi-note.S in C.
2020-05-20 10:27 ` Szabolcs Nagy
@ 2020-05-20 10:34 ` Florian Weimer via Libc-alpha
2020-05-21 8:54 ` [PATCH v4] " Szabolcs Nagy
0 siblings, 1 reply; 31+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-20 10:34 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha
* Szabolcs Nagy:
> The 05/18/2020 17:28, Florian Weimer wrote:
>> * Szabolcs Nagy:
>>
>> > +/* Note: Custom type is used as ElfW(Nhdr) is wrong on 64 bit targets. */
>>
>> Are you sure about that?
>>
>> typedef struct
>> {
>> Elf32_Word n_namesz; /* Length of the note's name. */
>> Elf32_Word n_descsz; /* Length of the note's descriptor. */
>> Elf32_Word n_type; /* Type of the note. */
>> } Elf32_Nhdr;
>>
>> typedef struct
>> {
>> Elf64_Word n_namesz; /* Length of the note's name. */
>> Elf64_Word n_descsz; /* Length of the note's descriptor. */
>> Elf64_Word n_type; /* Type of the note. */
>> } Elf64_Nhdr;
>>
>> The types are the same:
>>
>> typedef uint32_t Elf32_Word;
>> typedef uint32_t Elf64_Word;
>>
>> I admit this is super-confusing.
>
> aah i missed this, then the struct can be
>
> #include <elf.h>
>
> __attribute__ ((used, aligned (4), section (".note.ABI-tag")))
> static const struct
> {
> ElfW(Nhdr) nhdr;
> char name[4];
> int32_t desc[4];
> } __abi_tag = {
> { .n_namesz = 4, .n_descsz = 16, .n_type = 1 },
> "GNU",
> { __ABI_TAG_OS, __ABI_TAG_VERSION }
> };
>
> i think the aligned attribute is still needed in case on
> some target uint32_t has smaller alignemnt than 4.
Agreed, I think it's needed for m68k.
> i can change this if it is considered to be better
> or just remove the incorrect comment.
I slightly prefer using the type from <elf.h>.
Thanks,
Florian
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v4] Rewrite abi-note.S in C.
2020-05-20 10:34 ` Florian Weimer via Libc-alpha
@ 2020-05-21 8:54 ` Szabolcs Nagy
0 siblings, 0 replies; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-21 8:54 UTC (permalink / raw)
To: libc-alpha, Florian Weimer
Using C code allows the compiler to add target specific object file
markings based on CFLAGS.
The arm specific abi-note.S is removed and similar object file fix
up will be avoided on AArch64 with standard branch-prtection.
---
csu/{abi-note.S => abi-note.c} | 23 +++++++++++++----------
sysdeps/arm/abi-note.S | 8 --------
2 files changed, 13 insertions(+), 18 deletions(-)
rename csu/{abi-note.S => abi-note.c} (90%)
delete mode 100644 sysdeps/arm/abi-note.S
diff --git a/csu/abi-note.S b/csu/abi-note.c
similarity index 90%
rename from csu/abi-note.S
rename to csu/abi-note.c
index 2b4b5f8824..db195c7ab7 100644
--- a/csu/abi-note.S
+++ b/csu/abi-note.c
@@ -53,6 +53,8 @@ offset length contents
identify the earliest release of that OS that supports this ABI.
See abi-tags (top level) for details. */
+#include <link.h>
+#include <stdint.h>
#include <config.h>
#include <abi-tag.h> /* OS-specific ABI tag value */
@@ -60,13 +62,14 @@ offset length contents
name begins with `.note' and creates a PT_NOTE program header entry
pointing at it. */
- .section ".note.ABI-tag", "a"
- .p2align 2
- .long 1f - 0f /* name length */
- .long 3f - 2f /* data length */
- .long 1 /* note type */
-0: .asciz "GNU" /* vendor name */
-1: .p2align 2
-2: .long __ABI_TAG_OS /* note data: the ABI tag */
- .long __ABI_TAG_VERSION
-3: .p2align 2 /* pad out section */
+__attribute__ ((used, aligned (4), section (".note.ABI-tag")))
+static const struct
+{
+ ElfW(Nhdr) nhdr;
+ char name[4];
+ int32_t desc[4];
+} __abi_tag = {
+ { .n_namesz = 4, .n_descsz = 16, .n_type = 1 },
+ "GNU",
+ { __ABI_TAG_OS, __ABI_TAG_VERSION }
+};
diff --git a/sysdeps/arm/abi-note.S b/sysdeps/arm/abi-note.S
deleted file mode 100644
index 07bd4c4619..0000000000
--- a/sysdeps/arm/abi-note.S
+++ /dev/null
@@ -1,8 +0,0 @@
-/* Tag_ABI_align8_preserved: This code preserves 8-byte
- alignment in any callee. */
- .eabi_attribute 25, 1
-/* Tag_ABI_align8_needed: This code may require 8-byte alignment from
- the caller. */
- .eabi_attribute 24, 1
-
-#include <csu/abi-note.S>
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 02/13] elf.h: add aarch64 property definitions
2020-05-18 15:26 ` Florian Weimer via Libc-alpha
@ 2020-05-21 8:57 ` Szabolcs Nagy
0 siblings, 0 replies; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-21 8:57 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
The 05/18/2020 17:26, Florian Weimer wrote:
> * Szabolcs Nagy:
> > +/* AArch64 specific GNU properties. */
> > +#define GNU_PROPERTY_AARCH64_FEATURE_1_AND 0xc0000000
> > +
> > +#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1U << 0)
> > +#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC (1U << 1)
>
> I checked that this matches the binutils definition. I think this can
> go in separately.
now committed this and the PT_GNU_PROPERTY patch.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 04/13] aarch64: configure test for BTI support
2020-05-15 14:40 ` [PATCH v3 04/13] aarch64: configure test for BTI support Szabolcs Nagy
@ 2020-05-25 18:41 ` Adhemerval Zanella via Libc-alpha
2020-05-25 18:48 ` Adhemerval Zanella via Libc-alpha
0 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-25 18:41 UTC (permalink / raw)
To: libc-alpha
On 15/05/2020 11:40, Szabolcs Nagy wrote:
> Check BTI support in the compiler and linker. The check also
> requires READELF that understands the BTI GNU property note.
> It is expected to succeed with gcc >=gcc-9 configured with
> --enable-standard-branch-protection and binutils >=binutils-2.33.
From the configure check below I understand that it still possible to
build a glibc with BTI support if the required flags are add on
CC or CFLAGS. Maybe make it explicit in the commit message?
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> config.h.in | 3 +++
> sysdeps/aarch64/configure | 42 ++++++++++++++++++++++++++++++++++++
> sysdeps/aarch64/configure.ac | 19 ++++++++++++++++
> 3 files changed, 64 insertions(+)
>
> diff --git a/config.h.in b/config.h.in
> index dea43df438..506b0c416c 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -109,6 +109,9 @@
> /* AArch64 big endian ABI */
> #undef HAVE_AARCH64_BE
>
> +/* AArch64 BTI support enabled. */
> +#undef HAVE_AARCH64_BTI
> +
> /* C-SKY ABI version. */
> #undef CSKYABI
>
Ok.
> diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> index 5bd355a691..70477a7fa5 100644
> --- a/sysdeps/aarch64/configure
> +++ b/sysdeps/aarch64/configure
> @@ -172,3 +172,45 @@ else
> config_vars="$config_vars
> default-abi = lp64"
> fi
> +
> +# Only consider BTI supported if -mbranch-protection=bti is
> +# on by default in the compiler and the linker produces
> +# binaries with GNU property notes in PT_GNU_PROPERTY segment.
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for BTI support" >&5
> +$as_echo_n "checking for BTI support... " >&6; }
> +if ${libc_cv_aarch64_bti+:} false; then :
> + $as_echo_n "(cached) " >&6
> +else
> + cat > conftest.c <<EOF
> +void foo (void) { }
> +EOF
> + libc_cv_aarch64_bti=no
> + if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; } \
> + && { ac_try='$READELF -lW conftest.so | grep -q GNU_PROPERTY'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; } \
> + && { ac_try='$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; }
> + then
> + libc_cv_aarch64_bti=yes
> + fi
> + rm -rf conftest.*
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
> +$as_echo "$libc_cv_aarch64_bti" >&6; }
> +if test $libc_cv_aarch64_bti = yes; then
> + $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
> +
> +fi
> diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> index 7851dd4dac..798f494740 100644
> --- a/sysdeps/aarch64/configure.ac
> +++ b/sysdeps/aarch64/configure.ac
> @@ -20,3 +20,22 @@ if test $libc_cv_aarch64_be = yes; then
> else
> LIBC_CONFIG_VAR([default-abi], [lp64])
> fi
> +
> +# Only consider BTI supported if -mbranch-protection=bti is
> +# on by default in the compiler and the linker produces
> +# binaries with GNU property notes in PT_GNU_PROPERTY segment.
Maybe add that user might still set it on CC or CFLAGS?
> +AC_CACHE_CHECK([for BTI support], [libc_cv_aarch64_bti], [dnl
> + cat > conftest.c <<EOF
> +void foo (void) { }
> +EOF
> + libc_cv_aarch64_bti=no
> + if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c]) \
> + && AC_TRY_COMMAND([$READELF -lW conftest.so | grep -q GNU_PROPERTY]) \
> + && AC_TRY_COMMAND([$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"])
> + then
> + libc_cv_aarch64_bti=yes
> + fi
> + rm -rf conftest.*])
> +if test $libc_cv_aarch64_bti = yes; then
> + AC_DEFINE(HAVE_AARCH64_BTI)
> +fi
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 04/13] aarch64: configure test for BTI support
2020-05-25 18:41 ` Adhemerval Zanella via Libc-alpha
@ 2020-05-25 18:48 ` Adhemerval Zanella via Libc-alpha
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-25 18:48 UTC (permalink / raw)
To: libc-alpha
On 25/05/2020 15:41, Adhemerval Zanella wrote:
>
>
> On 15/05/2020 11:40, Szabolcs Nagy wrote:
>> Check BTI support in the compiler and linker. The check also
>> requires READELF that understands the BTI GNU property note.
>> It is expected to succeed with gcc >=gcc-9 configured with
>> --enable-standard-branch-protection and binutils >=binutils-2.33.
>
> From the configure check below I understand that it still possible to
> build a glibc with BTI support if the required flags are add on
> CC or CFLAGS. Maybe make it explicit in the commit message?
>
> LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
>> ---
>> config.h.in | 3 +++
>> sysdeps/aarch64/configure | 42 ++++++++++++++++++++++++++++++++++++
>> sysdeps/aarch64/configure.ac | 19 ++++++++++++++++
>> 3 files changed, 64 insertions(+)
>>
>> diff --git a/config.h.in b/config.h.in
>> index dea43df438..506b0c416c 100644
>> --- a/config.h.in
>> +++ b/config.h.in
>> @@ -109,6 +109,9 @@
>> /* AArch64 big endian ABI */
>> #undef HAVE_AARCH64_BE
>>
>> +/* AArch64 BTI support enabled. */
>> +#undef HAVE_AARCH64_BTI
>> +
>> /* C-SKY ABI version. */
>> #undef CSKYABI
>>
>
> Ok.
In fact I think you can define as:
#define HAVE_AARCH64_BTI 0
That AC_DEFINE will correctly set to 1. It allows to check for the
macro value instead of existence.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 05/13] aarch64: Add BTI support to assembly files
2020-05-15 14:40 ` [PATCH v3 05/13] aarch64: Add BTI support to assembly files Szabolcs Nagy
@ 2020-05-25 18:49 ` Adhemerval Zanella via Libc-alpha
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-25 18:49 UTC (permalink / raw)
To: libc-alpha
On 15/05/2020 11:40, Szabolcs Nagy wrote:
> From: Sudakshina Das <sudi.das@arm.com>
>
> To enable building glibc with branch protection, assembly code
> needs BTI landing pads and ELF object file markings in the form
> of a GNU property note.
>
> The landing pads are unconditionally added to all functions that
> may be indirectly called. When the code segment is not mapped
> with PROT_BTI these instructions are nops. They are kept in the
> code when BTI is not supported so that the layout of performance
> critical code is unchanged across configurations.
>
> The GNU property notes are only added when there is support for
> BTI in the toolchain, because old binutils does not handle the
> notes right. (Does not know how to merge them nor to put them in
> PT_GNU_PROPERTY segment instead of PT_NOTE, and some versions
> of binutils emit warnings about the unknown GNU property. In
> such cases the produced libc binaries would not have valid
> ELF marking so BTI would not be enabled.)
>
> Note: functions using ENTRY or ENTRY_ALIGN now start with an
> additional BTI c, so alignment of the following code changes,
> but ENTRY_ALIGN_AND_PAD was fixed so there is no change to the
> existing code layout. Some string functions may need to be
> tuned for optimal performance after this commit.
>
> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
LGTM with a small change with depends on previous commit change
(with I also added a comment).
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/aarch64/crti.S | 2 ++
> sysdeps/aarch64/crtn.S | 2 ++
> sysdeps/aarch64/dl-tlsdesc.S | 3 +++
> sysdeps/aarch64/dl-trampoline.S | 2 ++
> sysdeps/aarch64/start.S | 1 +
> sysdeps/aarch64/sysdep.h | 34 ++++++++++++++++++++++++++++++++-
> 6 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
> index 1728eac37a..c346bcad72 100644
> --- a/sysdeps/aarch64/crti.S
> +++ b/sysdeps/aarch64/crti.S
> @@ -75,6 +75,7 @@ call_weak_fn:
> .hidden _init
> .type _init, %function
> _init:
> + BTI_C
> stp x29, x30, [sp, -16]!
> mov x29, sp
> #if PREINIT_FUNCTION_WEAK
> @@ -89,5 +90,6 @@ _init:
> .hidden _fini
> .type _fini, %function
> _fini:
> + BTI_C
> stp x29, x30, [sp, -16]!
> mov x29, sp
Ok.
> diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
> index c3e97cc449..0c1ef112c2 100644
> --- a/sysdeps/aarch64/crtn.S
> +++ b/sysdeps/aarch64/crtn.S
> @@ -37,6 +37,8 @@
> /* crtn.S puts function epilogues in the .init and .fini sections
> corresponding to the prologues in crti.S. */
>
> +#include <sysdep.h>
> +
> .section .init,"ax",%progbits
> ldp x29, x30, [sp], 16
> RET
Ok.
> diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
> index 557ad1d505..9d96c8632a 100644
> --- a/sysdeps/aarch64/dl-tlsdesc.S
> +++ b/sysdeps/aarch64/dl-tlsdesc.S
> @@ -74,6 +74,7 @@
> cfi_startproc
> .align 2
> _dl_tlsdesc_return:
> + BTI_C
> DELOUSE (0)
> ldr PTR_REG (0), [x0, #PTR_SIZE]
> RET
> @@ -95,6 +96,7 @@ _dl_tlsdesc_return:
> cfi_startproc
> .align 2
> _dl_tlsdesc_undefweak:
> + BTI_C
> str x1, [sp, #-16]!
> cfi_adjust_cfa_offset (16)
> DELOUSE (0)
> @@ -142,6 +144,7 @@ _dl_tlsdesc_undefweak:
> cfi_startproc
> .align 2
> _dl_tlsdesc_dynamic:
> + BTI_C
> DELOUSE (0)
>
> /* Save just enough registers to support fast path, if we fall
Ok.
> diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
> index 94e965c096..2cbfa81434 100644
> --- a/sysdeps/aarch64/dl-trampoline.S
> +++ b/sysdeps/aarch64/dl-trampoline.S
> @@ -35,6 +35,7 @@
> cfi_startproc
> .align 2
> _dl_runtime_resolve:
> + BTI_C
> /* AArch64 we get called with:
> ip0 &PLTGOT[2]
> ip1 temp(dl resolver entry point)
> @@ -126,6 +127,7 @@ _dl_runtime_resolve:
> cfi_startproc
> .align 2
> _dl_runtime_profile:
> + BTI_C
> /* AArch64 we get called with:
> ip0 &PLTGOT[2]
> ip1 temp(dl resolver entry point)
OK.
> diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
> index d96cf57e2d..75393e1c18 100644
> --- a/sysdeps/aarch64/start.S
> +++ b/sysdeps/aarch64/start.S
> @@ -46,6 +46,7 @@
> .globl _start
> .type _start,#function
> _start:
> + BTI_C
> /* Create an initial frame with 0 LR and FP */
> mov x29, #0
> mov x30, #0
OK.
> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 604c489170..086fc84b53 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -41,12 +41,42 @@
>
> #define ASM_SIZE_DIRECTIVE(name) .size name,.-name
>
> +/* Branch Target Identitication support. */
> +#define BTI_C hint 34
> +#define BTI_J hint 36
> +
> +/* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code. */
> +#define FEATURE_1_AND 0xc0000000
> +#define FEATURE_1_BTI 1
> +#define FEATURE_1_PAC 2
> +
> +/* Add a NT_GNU_PROPERTY_TYPE_0 note. */
> +#define GNU_PROPERTY(type, value) \
> + .section .note.gnu.property, "a"; \
> + .p2align 3; \
> + .word 4; \
> + .word 16; \
> + .word 5; \
> + .asciz "GNU"; \
> + .word type; \
> + .word 4; \
> + .word value; \
> + .word 0; \
> + .text
> +
> +/* Add GNU property note with the supported features to all asm code
> + where sysdep.h is included. */
> +#if defined HAVE_AARCH64_BTI
> +GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
> +#endif
By defining the default value as 0 I think you can check with
#if HAVE_AARCH64_BTI
> +
> /* Define an entry point visible from C. */
> #define ENTRY(name) \
> .globl C_SYMBOL_NAME(name); \
> .type C_SYMBOL_NAME(name),%function; \
> .align 4; \
> C_LABEL(name) \
> + BTI_C; \
> cfi_startproc; \
> CALL_MCOUNT
>
> @@ -56,6 +86,7 @@
> .type C_SYMBOL_NAME(name),%function; \
> .p2align align; \
> C_LABEL(name) \
> + BTI_C; \
> cfi_startproc; \
> CALL_MCOUNT
>
> @@ -68,10 +99,11 @@
> .globl C_SYMBOL_NAME(name); \
> .type C_SYMBOL_NAME(name),%function; \
> .p2align align; \
> - .rep padding; \
> + .rep padding - 1; /* -1 for bti c. */ \
> nop; \
> .endr; \
> C_LABEL(name) \
> + BTI_C; \
> cfi_startproc; \
> CALL_MCOUNT
>
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 09/13] aarch64: enable BTI at runtime
2020-05-15 14:40 ` [PATCH v3 09/13] aarch64: enable BTI at runtime Szabolcs Nagy
@ 2020-05-25 19:53 ` Adhemerval Zanella via Libc-alpha
2020-05-26 11:20 ` Szabolcs Nagy
0 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-25 19:53 UTC (permalink / raw)
To: libc-alpha, Szabolcs Nagy
On 15/05/2020 11:40, Szabolcs Nagy wrote:
> From: Sudakshina Das <sudi.das@arm.com>
>
> Binaries can opt-in to using BTI via an ELF object file marking.
> The dynamic linker has to then mprotect the executable segments
> with PROT_BTI. In case of static linked executables or in case
> of the dynamic linker itself, PROT_BTI protection is done by the
> operating system.
>
> On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
> the properties of a binary because PT_NOTE can be unreliable with
> old linkers (old linkers just append the notes of input objects
> together and add them to the output without checking them for
> consistency which means multiple incompatible GNU property notes
> can be present in PT_NOTE). A new _dl_process_pt_gnu_property
> hook is introduced in dl-prop.h and to keep it maintainable the
> rtld and dlopen code paths use the same function (if the main
> map needs special treatment, that should be inferred by the hook
> from the link map). Unlike the _dt_process_pt_note hook this one
> is called after segments are mapped to avoid unbounded allocation
> and additional read syscall. Otherwise the AArch64 logic follows
> the x86 logic for handling GNU properties (but the code is not
> shared because x86 needs to manage internal CET state and look
> out for multiple property notes).
>
> BTI property is handled in the loader even if glibc is not built
> with BTI support, so in theory user code can be BTI protected
> independently of glibc. In practice though user binaries are not
> marked with the BTI property if glibc has no support because the
> static linked libc objects (crt files, libc_nonshared.a) are
> unmarked.
>
> This patch relies on Linux userspace API that is scheduled to be
> merged in Linux 5.8 and now it is in the for-next/bti-user branch
> of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git.
>
> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
LGTM with a just a nit below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> elf/dl-load.c | 13 ++
> elf/rtld.c | 6 +
> sysdeps/aarch64/Makefile | 4 +
> sysdeps/aarch64/dl-bti.c | 54 +++++++
> sysdeps/aarch64/dl-prop.h | 145 ++++++++++++++++++
> sysdeps/aarch64/linkmap.h | 3 +
> sysdeps/generic/dl-prop.h | 16 +-
> sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
> sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 ++++
> .../unix/sysv/linux/aarch64/cpu-features.c | 3 +
> .../unix/sysv/linux/aarch64/cpu-features.h | 2 +
> sysdeps/x86/dl-prop.h | 6 +
> 12 files changed, 279 insertions(+), 5 deletions(-)
> create mode 100644 sysdeps/aarch64/dl-bti.c
> create mode 100644 sysdeps/aarch64/dl-prop.h
> create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 06f2ba7264..9c37ec1098 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1188,6 +1188,19 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> maplength, has_holes, loader);
> if (__glibc_unlikely (errstring != NULL))
> goto call_lose;
> +
> + /* Process program headers again after load segments are mapped. */
Maybe add a brief explanation of why it is done after load segment mapping?
> + for (ph = phdr; ph < &phdr[l->l_phnum]; ++ph)
> + switch (ph->p_type)
> + {
> + case PT_GNU_PROPERTY:
> + if (_dl_process_pt_gnu_property (l, ph))
> + {
> + errstring = N_("cannot process GNU property segment");
> + goto call_lose;
> + }
> + break;
> + }
> }
>
> if (l->l_ld == 0)
Ok.
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 5ccc3c2dbb..97a0bbf4dc 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1506,6 +1506,12 @@ of this helper program; chances are you did not intend to run this program.\n\
> main_map->l_relro_size = ph->p_memsz;
> break;
>
> + case PT_GNU_PROPERTY:
> + if (_dl_process_pt_gnu_property (main_map, ph))
> + _dl_error_printf (
> +"ERROR: '%s': cannot process GNU property segment.\n", _dl_argv[0]);
> + break;
> +
> case PT_NOTE:
> if (_rtld_process_pt_note (main_map, ph))
> _dl_error_printf ("\
Ok.
> diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> index 9cb141004d..5ae8b082b0 100644
> --- a/sysdeps/aarch64/Makefile
> +++ b/sysdeps/aarch64/Makefile
> @@ -1,5 +1,9 @@
> long-double-fcts = yes
>
> +ifeq ($(subdir),elf)
> +sysdep-dl-routines += dl-bti
> +endif
> +
> ifeq ($(subdir),elf)
> sysdep-dl-routines += tlsdesc dl-tlsdesc
> gen-as-const-headers += dl-link.sym
Ok.
> diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
> new file mode 100644
> index 0000000000..6003686601
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-bti.c
> @@ -0,0 +1,54 @@
> +/* AArch64 BTI functions.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <unistd.h>
> +#include <errno.h>
> +#include <libintl.h>
> +#include <ldsodefs.h>
> +
> +static int
> +enable_bti (struct link_map *map, const char *program)
> +{
> + const ElfW(Phdr) *phdr;
> + unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
> +
> + for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
> + if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
> + {
> + ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
> + ElfW(Addr) len = phdr->p_memsz;
> + if (__mprotect ((void *) start, len, prot) < 0)
> + {
> + if (program)
> + _dl_fatal_printf ("%s: mprotect failed to turn on BTI\n",
> + map->l_name);
> + else
> + _dl_signal_error (EINVAL, map->l_name, "dlopen",
> + N_("mprotect failed to turn on BTI"));
Is EINVAL the only possible error here (EACCES or ENOMEM might be still
possible)?
> + }
> + }
> + return 0;
> +}
> +
> +/* Enable BTI for L if required. */
> +
> +void
> +_dl_bti_check (struct link_map *l, const char *program)
> +{
> + if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti)
> + enable_bti (l, program);
> +}
Ok.
> diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
> new file mode 100644
> index 0000000000..b6f8a88667
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-prop.h
> @@ -0,0 +1,145 @@
> +/* Support for GNU properties. AArch64 version.
> + Copyright (C) 2018-2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#ifndef _DL_PROP_H
> +#define _DL_PROP_H
> +
> +#include <not-cancel.h>
> +
> +extern void _dl_bti_check (struct link_map *, const char *)
> + attribute_hidden;
> +
> +static inline void __attribute__ ((always_inline))
> +_rtld_main_check (struct link_map *m, const char *program)
> +{
> + _dl_bti_check (m, program);
> +}
> +
> +static inline void __attribute__ ((always_inline))
> +_dl_open_check (struct link_map *m)
> +{
> + _dl_bti_check (m, NULL);
> +}
> +
> +static inline void __attribute__ ((unused))
> +_dl_process_aarch64_property (struct link_map *l,
> + const ElfW(Nhdr) *note,
> + const ElfW(Addr) size,
> + const ElfW(Addr) align)
> +{
> + /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
> + 32-bit objects and to 8 bytes in 64-bit objects. Skip notes
> + with incorrect alignment. */
> + if (align != (__ELF_NATIVE_CLASS / 8))
> + return;
> +
> + const ElfW(Addr) start = (ElfW(Addr)) note;
> +
> + unsigned int feature_1 = 0;
> + unsigned int last_type = 0;
> +
> + while ((ElfW(Addr)) (note + 1) - start < size)
> + {
> + /* Find the NT_GNU_PROPERTY_TYPE_0 note. */
> + if (note->n_namesz == 4
> + && note->n_type == NT_GNU_PROPERTY_TYPE_0
> + && memcmp (note + 1, "GNU", 4) == 0)
> + {
> + /* Check for invalid property. */
> + if (note->n_descsz < 8
> + || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
> + return;
> +
> + /* Start and end of property array. */
> + unsigned char *ptr = (unsigned char *) (note + 1) + 4;
> + unsigned char *ptr_end = ptr + note->n_descsz;
> +
> + do
> + {
> + unsigned int type = *(unsigned int *) ptr;
> + unsigned int datasz = *(unsigned int *) (ptr + 4);
> +
> + /* Property type must be in ascending order. */
> + if (type < last_type)
> + return;
> +
> + ptr += 8;
> + if ((ptr + datasz) > ptr_end)
> + return;
> +
> + last_type = type;
> +
> + if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
> + {
> + /* The size of GNU_PROPERTY_AARCH64_FEATURE_1_AND is 4
> + bytes. When seeing GNU_PROPERTY_AARCH64_FEATURE_1_AND,
> + we stop the search regardless if its size is correct
> + or not. There is no point to continue if this note
> + is ill-formed. */
> + if (datasz != 4)
> + return;
> +
> + feature_1 = *(unsigned int *) ptr;
> + if ((feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI))
> + l->l_mach.bti = true;
> +
> + /* Stop if we found the property note. */
> + return;
> + }
> + else if (type > GNU_PROPERTY_AARCH64_FEATURE_1_AND)
> + {
> + /* Stop since property type is in ascending order. */
> + return;
> + }
> +
> + /* Check the next property item. */
> + ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> + }
> + while ((ptr_end - ptr) >= 8);
> + }
> +
> + note = ((const void *) note
> + + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
> + align));
> + }
> +}
> +
> +#ifdef FILEBUF_SIZE
> +static inline int __attribute__ ((always_inline))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
> + int fd, struct filebuf *fbp)
> +{
> + return 0;
> +}
> +#endif
> +
> +static inline int __attribute__ ((always_inline))
> +_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> + return 0;
> +}
> +
> +static inline int
> +_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> + const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
> + _dl_process_aarch64_property (l, note, ph->p_memsz, ph->p_align);
> + return 0;
> +}
> +
> +#endif /* _DL_PROP_H */
Ok.
> diff --git a/sysdeps/aarch64/linkmap.h b/sysdeps/aarch64/linkmap.h
> index 943a9ee9e4..847a03ace2 100644
> --- a/sysdeps/aarch64/linkmap.h
> +++ b/sysdeps/aarch64/linkmap.h
> @@ -16,8 +16,11 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> +#include <stdbool.h>
> +
> struct link_map_machine
> {
> ElfW(Addr) plt; /* Address of .plt */
> void *tlsdesc_table; /* Address of TLS descriptor hash table. */
> + bool bti; /* Branch Target Identification is enabled. */
> };
Ok.
> diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
> index 6b0f2aa95a..4192049739 100644
> --- a/sysdeps/generic/dl-prop.h
> +++ b/sysdeps/generic/dl-prop.h
> @@ -20,11 +20,11 @@
> #define _DL_PROP_H
>
> /* The following functions are used by the dynamic loader and the
> - dlopen machinery to process PT_NOTE entries in the binary or
> - shared object. The notes can be used to change the behaviour of
> - the loader, and as such offer a flexible mechanism for hooking in
> - various checks related to ABI tags or implementing "flag day" ABI
> - transitions. */
> + dlopen machinery to process PT_NOTE and PT_GNU_PROPERTY entries in
> + the binary or shared object. The notes can be used to change the
> + behaviour of the loader, and as such offer a flexible mechanism
> + for hooking in various checks related to ABI tags or implementing
> + "flag day" ABI transitions. */
>
> static inline void __attribute__ ((always_inline))
> _rtld_main_check (struct link_map *m, const char *program)
> @@ -51,4 +51,10 @@ _rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> return 0;
> }
>
> +static inline int __attribute__ ((always_inline))
> +_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> + return 0;
> +}
> +
> #endif /* _DL_PROP_H */
Ok.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> index 4ee14b4208..af90d8a626 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> @@ -72,3 +72,4 @@
> #define HWCAP2_BF16 (1 << 14)
> #define HWCAP2_DGH (1 << 15)
> #define HWCAP2_RNG (1 << 16)
> +#define HWCAP2_BTI (1 << 17)
Ok.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> new file mode 100644
> index 0000000000..ecae046344
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> @@ -0,0 +1,31 @@
> +/* Definitions for POSIX memory map interface. Linux/AArch64 version.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_MMAN_H
> +# error "Never use <bits/mman.h> directly; include <sys/mman.h> instead."
> +#endif
> +
> +/* AArch64 specific definitions, should be in sync with
> + arch/arm64/include/uapi/asm/mman.h. */
> +
> +#define PROT_BTI 0x10
> +
> +#include <bits/mman-map-flags-generic.h>
> +
> +/* Include generic Linux declarations. */
> +#include <bits/mman-linux.h>
Ok (although I am still not sure if this should be inside a __USE_MISC).
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> index 896c588fee..b9ab827aca 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -83,4 +83,7 @@ init_cpu_features (struct cpu_features *cpu_features)
>
> if ((dczid & DCZID_DZP_MASK) == 0)
> cpu_features->zva_size = 4 << (dczid & DCZID_BS_MASK);
> +
> + /* Check if BTI is supported. */
> + cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
> }
Ok.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> index 1389cea1b3..a81f186ec2 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> @@ -20,6 +20,7 @@
> #define _CPU_FEATURES_AARCH64_H
>
> #include <stdint.h>
> +#include <stdbool.h>
>
> #define MIDR_PARTNUM_SHIFT 4
> #define MIDR_PARTNUM_MASK (0xfff << MIDR_PARTNUM_SHIFT)
> @@ -64,6 +65,7 @@ struct cpu_features
> {
> uint64_t midr_el1;
> unsigned zva_size;
> + bool bti;
> };
>
> #endif /* _CPU_FEATURES_AARCH64_H */
Ok.
> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index 516f88ea80..8649314f9d 100644
> --- a/sysdeps/x86/dl-prop.h
> +++ b/sysdeps/x86/dl-prop.h
> @@ -191,4 +191,10 @@ _rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> return 0;
> }
>
> +static inline int __attribute__ ((always_inline))
> +_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> + return 0;
> +}
> +
> #endif /* _DL_PROP_H */
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 10/13] aarch64: configure check for pac-ret code generation
2020-05-15 14:40 ` [PATCH v3 10/13] aarch64: configure check for pac-ret code generation Szabolcs Nagy
@ 2020-05-25 21:49 ` Adhemerval Zanella via Libc-alpha
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-25 21:49 UTC (permalink / raw)
To: libc-alpha
On 15/05/2020 11:40, Szabolcs Nagy wrote:
> Return address signing requires unwinder support, which is
> present in libgcc since >=gcc-7, however due to bugs the
> support may be broken in <gcc-10 (and similarly there may
> be issues in custom unwinders), so pac-ret is not always
> safe to use. So in assembly code glibc should only use
> pac-ret if the compiler uses it too. Unfortunately there
> is no predefined feature macro for it set by the compiler
> so pac-ret is inferred from the code generation.
OK with this fixes below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> config.h.in | 3 +++
> sysdeps/aarch64/configure | 39 ++++++++++++++++++++++++++++++++++++
> sysdeps/aarch64/configure.ac | 21 +++++++++++++++++++
> 3 files changed, 63 insertions(+)
>
> diff --git a/config.h.in b/config.h.in
> index 506b0c416c..f441470385 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -112,6 +112,9 @@
> /* AArch64 BTI support enabled. */
> #undef HAVE_AARCH64_BTI
>
> +/* AArch64 PAC-RET code generation is enabled. */
> +#undef HAVE_AARCH64_PAC_RET
> +
> /* C-SKY ABI version. */
> #undef CSKYABI
>
Use
#define HAVE_AARCH64_PAC_RET 0
instead so we can check the define value instead of the its existence.
> diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> index 70477a7fa5..5f1cdf5d9b 100644
> --- a/sysdeps/aarch64/configure
> +++ b/sysdeps/aarch64/configure
> @@ -214,3 +214,42 @@ if test $libc_cv_aarch64_bti = yes; then
> $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
>
> fi
> +
> +# Check if glibc is built with return address signing, i.e.
> +# if -mbranch-protection=pac-ret is on. We need this because
> +# pac-ret relies on unwinder support so it's not safe to use
> +# it in assembly code unconditionally, but there is no
> +# feature test macro for it in gcc.
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking if pac-ret is enabled" >&5
> +$as_echo_n "checking if pac-ret is enabled... " >&6; }
> +if ${libc_cv_aarch64_pac_ret+:} false; then :
> + $as_echo_n "(cached) " >&6
> +else
> + cat > conftest.c <<EOF
> +int bar (void);
> +int foo (void) { return bar () + 1; }
> +EOF
> + libc_cv_aarch64_pac_ret=no
> + if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -S -o conftest.s conftest.c'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; } \
> + && { ac_try='grep -q -E '\''(hint( | )+25|paciasp)'\'' conftest.s'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; }
> + then
> + libc_cv_aarch64_pac_ret=yes
> + fi
> + rm -rf conftest.*
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_pac_ret" >&5
> +$as_echo "$libc_cv_aarch64_pac_ret" >&6; }
> +if test $libc_cv_aarch64_pac_ret = yes; then
> + $as_echo "#define HAVE_AARCH64_PAC_RET 1" >>confdefs.h
> +
> +fi
> diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> index 798f494740..8248ecf2ed 100644
> --- a/sysdeps/aarch64/configure.ac
> +++ b/sysdeps/aarch64/configure.ac
> @@ -39,3 +39,24 @@ EOF
> if test $libc_cv_aarch64_bti = yes; then
> AC_DEFINE(HAVE_AARCH64_BTI)
> fi
> +
> +# Check if glibc is built with return address signing, i.e.
> +# if -mbranch-protection=pac-ret is on. We need this because
Two spaces after period.
> +# pac-ret relies on unwinder support so it's not safe to use
> +# it in assembly code unconditionally, but there is no
> +# feature test macro for it in gcc.
> +AC_CACHE_CHECK([if pac-ret is enabled], [libc_cv_aarch64_pac_ret], [dnl
> + cat > conftest.c <<EOF
> +int bar (void);
> +int foo (void) { return bar () + 1; }
> +EOF
> + libc_cv_aarch64_pac_ret=no
> + if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS -S -o conftest.s conftest.c]) \
> + && AC_TRY_COMMAND([grep -q -E '\''(hint( | )+25|paciasp)'\'' conftest.s])
I am not sure this expression is doing what it is intended to do,
I think you are matching the gcc output because it also add an
extra comment with 'paciasp'.
I think what you need here is "(hint[[:space:]]+25|paciasp)".
> + then
> + libc_cv_aarch64_pac_ret=yes
> + fi
> + rm -rf conftest.*])
> +if test $libc_cv_aarch64_pac_ret = yes; then
> + AC_DEFINE(HAVE_AARCH64_PAC_RET)
> +fi
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 09/13] aarch64: enable BTI at runtime
2020-05-25 19:53 ` Adhemerval Zanella via Libc-alpha
@ 2020-05-26 11:20 ` Szabolcs Nagy
0 siblings, 0 replies; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-26 11:20 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
The 05/25/2020 16:53, Adhemerval Zanella wrote:
> On 15/05/2020 11:40, Szabolcs Nagy wrote:
> > From: Sudakshina Das <sudi.das@arm.com>
> >
> > Binaries can opt-in to using BTI via an ELF object file marking.
> > The dynamic linker has to then mprotect the executable segments
> > with PROT_BTI. In case of static linked executables or in case
> > of the dynamic linker itself, PROT_BTI protection is done by the
> > operating system.
> >
> > On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
> > the properties of a binary because PT_NOTE can be unreliable with
> > old linkers (old linkers just append the notes of input objects
> > together and add them to the output without checking them for
> > consistency which means multiple incompatible GNU property notes
> > can be present in PT_NOTE). A new _dl_process_pt_gnu_property
> > hook is introduced in dl-prop.h and to keep it maintainable the
> > rtld and dlopen code paths use the same function (if the main
> > map needs special treatment, that should be inferred by the hook
> > from the link map). Unlike the _dt_process_pt_note hook this one
> > is called after segments are mapped to avoid unbounded allocation
> > and additional read syscall. Otherwise the AArch64 logic follows
> > the x86 logic for handling GNU properties (but the code is not
> > shared because x86 needs to manage internal CET state and look
> > out for multiple property notes).
> >
> > BTI property is handled in the loader even if glibc is not built
> > with BTI support, so in theory user code can be BTI protected
> > independently of glibc. In practice though user binaries are not
> > marked with the BTI property if glibc has no support because the
> > static linked libc objects (crt files, libc_nonshared.a) are
> > unmarked.
> >
> > This patch relies on Linux userspace API that is scheduled to be
> > merged in Linux 5.8 and now it is in the for-next/bti-user branch
> > of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git.
> >
> > Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>
> LGTM with a just a nit below.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
thanks for the review.
> > @@ -1188,6 +1188,19 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> > maplength, has_holes, loader);
> > if (__glibc_unlikely (errstring != NULL))
> > goto call_lose;
> > +
> > + /* Process program headers again after load segments are mapped. */
>
> Maybe add a brief explanation of why it is done after load segment mapping?
>
> > + for (ph = phdr; ph < &phdr[l->l_phnum]; ++ph)
> > + switch (ph->p_type)
> > + {
> > + case PT_GNU_PROPERTY:
> > + if (_dl_process_pt_gnu_property (l, ph))
> > + {
> > + errstring = N_("cannot process GNU property segment");
> > + goto call_lose;
> > + }
> > + break;
> > + }
btw i think the _dl_process_pt_note callback should
be done here too and x86 fixed up accordingly so it
does not need unbounded allocation + pread_nocancel
to process the notes.
> > +static int
> > +enable_bti (struct link_map *map, const char *program)
> > +{
> > + const ElfW(Phdr) *phdr;
> > + unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
> > +
> > + for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
> > + if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
> > + {
> > + ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
> > + ElfW(Addr) len = phdr->p_memsz;
> > + if (__mprotect ((void *) start, len, prot) < 0)
> > + {
> > + if (program)
> > + _dl_fatal_printf ("%s: mprotect failed to turn on BTI\n",
> > + map->l_name);
> > + else
> > + _dl_signal_error (EINVAL, map->l_name, "dlopen",
> > + N_("mprotect failed to turn on BTI"));
>
> Is EINVAL the only possible error here (EACCES or ENOMEM might be still
> possible)?
no, i think passing errno makes more sense here.
i'll fix it.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 11/13] aarch64: Add pac-ret support to assembly files
2020-05-15 14:40 ` [PATCH v3 11/13] aarch64: Add pac-ret support to assembly files Szabolcs Nagy
@ 2020-05-26 11:26 ` Adhemerval Zanella via Libc-alpha
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-26 11:26 UTC (permalink / raw)
To: libc-alpha
On 15/05/2020 11:40, Szabolcs Nagy wrote:
> Use return address signing in assembly files for functions that save
> LR when pac-ret is enabled in the compiler.
>
> The GNU property note for PAC-RET is not meaningful to the dynamic
> linker so it is not strictly required, but it may be used to track
> the security property of binaries. (The PAC-RET property is only set
> if BTI is set too because BTI implies working GNU property support.)
With the suggestion from previous HAVE_AARCH64_PAC_RET patch,
these tests could be used as #if HAVE_AARCH64_PAC_RET.
Besides that, LGTM.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/aarch64/crti.S | 8 ++++++++
> sysdeps/aarch64/crtn.S | 6 ++++++
> sysdeps/aarch64/dl-tlsdesc.S | 8 ++++++++
> sysdeps/aarch64/dl-trampoline.S | 18 ++++++++++++++++++
> sysdeps/aarch64/sysdep.h | 8 +++++++-
> 5 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
> index c346bcad72..02ec7d015e 100644
> --- a/sysdeps/aarch64/crti.S
> +++ b/sysdeps/aarch64/crti.S
> @@ -75,7 +75,11 @@ call_weak_fn:
> .hidden _init
> .type _init, %function
> _init:
> +#ifdef HAVE_AARCH64_PAC_RET
> + PACIASP
> +#else
> BTI_C
> +#endif
> stp x29, x30, [sp, -16]!
> mov x29, sp
> #if PREINIT_FUNCTION_WEAK
> @@ -90,6 +94,10 @@ _init:
> .hidden _fini
> .type _fini, %function
> _fini:
> +#ifdef HAVE_AARCH64_PAC_RET
> + PACIASP
> +#else
> BTI_C
> +#endif
> stp x29, x30, [sp, -16]!
> mov x29, sp
Ok.
> diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
> index 0c1ef112c2..4b93b90411 100644
> --- a/sysdeps/aarch64/crtn.S
> +++ b/sysdeps/aarch64/crtn.S
> @@ -41,8 +41,14 @@
>
> .section .init,"ax",%progbits
> ldp x29, x30, [sp], 16
> +#ifdef HAVE_AARCH64_PAC_RET
> + AUTIASP
> +#endif
> RET
>
> .section .fini,"ax",%progbits
> ldp x29, x30, [sp], 16
> +#ifdef HAVE_AARCH64_PAC_RET
> + AUTIASP
> +#endif
> RET
Ok.
> diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
> index 9d96c8632a..3746dbec17 100644
> --- a/sysdeps/aarch64/dl-tlsdesc.S
> +++ b/sysdeps/aarch64/dl-tlsdesc.S
> @@ -183,6 +183,10 @@ _dl_tlsdesc_dynamic:
> callee will trash. */
>
> /* Save the remaining registers that we must treat as caller save. */
> +# ifdef HAVE_AARCH64_PAC_RET
> + PACIASP
> + cfi_window_save
> +# endif
> # define NSAVEXREGPAIRS 8
> stp x29, x30, [sp,#-16*NSAVEXREGPAIRS]!
> cfi_adjust_cfa_offset (16*NSAVEXREGPAIRS)
> @@ -233,6 +237,10 @@ _dl_tlsdesc_dynamic:
> cfi_adjust_cfa_offset (-16*NSAVEXREGPAIRS)
> cfi_restore (x29)
> cfi_restore (x30)
> +# ifdef HAVE_AARCH64_PAC_RET
> + AUTIASP
> + cfi_window_save
> +# endif
> b 1b
> cfi_endproc
> .size _dl_tlsdesc_dynamic, .-_dl_tlsdesc_dynamic
Ok.
> diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
> index 2cbfa81434..53f92d68bf 100644
> --- a/sysdeps/aarch64/dl-trampoline.S
> +++ b/sysdeps/aarch64/dl-trampoline.S
> @@ -127,7 +127,12 @@ _dl_runtime_resolve:
> cfi_startproc
> .align 2
> _dl_runtime_profile:
> +# ifdef HAVE_AARCH64_PAC_RET
> + PACIASP
> + cfi_window_save
> +# else
> BTI_C
> +# endif
> /* AArch64 we get called with:
> ip0 &PLTGOT[2]
> ip1 temp(dl resolver entry point)
> @@ -239,8 +244,17 @@ _dl_runtime_profile:
> cfi_restore(x29)
> cfi_restore(x30)
>
> +# ifdef HAVE_AARCH64_PAC_RET
> + add sp, sp, SF_SIZE
> + cfi_adjust_cfa_offset (-SF_SIZE)
> + AUTIASP
> + cfi_window_save
> + add sp, sp, 16
> + cfi_adjust_cfa_offset (-16)
> +# else
> add sp, sp, SF_SIZE + 16
> cfi_adjust_cfa_offset (- SF_SIZE - 16)
> +# endif
>
> /* Jump to the newly found address. */
> br ip0
> @@ -287,6 +301,10 @@ _dl_runtime_profile:
> /* LR from within La_aarch64_reg */
> ldr lr, [x29, #OFFSET_RG + DL_OFFSET_RG_LR]
> cfi_restore(lr)
> +# ifdef HAVE_AARCH64_PAC_RET
> + /* Note: LR restored from La_aarch64_reg has no PAC. */
> + cfi_window_save
> +# endif
> mov sp, x29
> cfi_def_cfa_register (sp)
> ldr x29, [x29, #0]
Ok.
> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 086fc84b53..c51572a690 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -45,6 +45,10 @@
> #define BTI_C hint 34
> #define BTI_J hint 36
>
> +/* Return address signing support (pac-ret). */
> +#define PACIASP hint 25
> +#define AUTIASP hint 29
> +
> /* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code. */
> #define FEATURE_1_AND 0xc0000000
> #define FEATURE_1_BTI 1
> @@ -66,7 +70,9 @@
>
> /* Add GNU property note with the supported features to all asm code
> where sysdep.h is included. */
> -#if defined HAVE_AARCH64_BTI
> +#if defined HAVE_AARCH64_BTI && defined HAVE_AARCH64_PAC_RET
> +GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI|FEATURE_1_PAC)
> +#elif defined HAVE_AARCH64_BTI
> GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
> #endif
>
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 12/13] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-05-15 14:40 ` [PATCH v3 12/13] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
@ 2020-05-26 11:29 ` Adhemerval Zanella via Libc-alpha
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-26 11:29 UTC (permalink / raw)
To: libc-alpha
On 15/05/2020 11:40, Szabolcs Nagy wrote:
> RETURN_ADDRESS is used at several places in glibc to mean a valid
> code address of the call site, but with pac-ret it has a pointer
> authentication code (PAC), so its definition is adjusted.
>
> strip_pac is omitted if glibc is bulit without pac-ret, but it could
s/bulit/built
> be added unconditionally (that's just unnecessary operations).
> Inline asm is used instead of __builtin_aarch64_xpaclri since that
> is an undocumented builtin and not available in all supported gccs.
>
> Note: such change indicates a problem in the pac-ret design: it
> can break code that uses __builtin_return_address and the breakage
> is only visible at runtime on a system with pac-ret enabled. It is
> not ideal that users need target specific inline asm to fix this up.
> For now we can recommend disabling pac-ret where this is a problem,
> but gcc might need improvements in this are to make pac-ret usable.
>
> TODO: __builtin_return_address handling with pac-ret:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94891
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/aarch64/sysdep.h | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index c51572a690..7a70cf7a2b 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -35,6 +35,23 @@
>
> #define PTR_SIZE (1<<PTR_LOG_SIZE)
>
> +#ifndef __ASSEMBLER__
> +/* Strip pointer authentication code from pointer p. */
> +static inline void *
> +strip_pac (void *p)
> +{
> + register void *ra asm ("x30") = (p);
> + asm ("hint 7 // xpaclri" : "+r"(ra));
> + return ra;
Indentation seems off here (tab instead of double space).
> +}
> +
> +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
> +# ifdef HAVE_AARCH64_PAC_RET
> +# undef RETURN_ADDRESS
> +# define RETURN_ADDRESS(n) strip_pac (__builtin_return_address (n))
> +# endif
> +#endif
> +
> #ifdef __ASSEMBLER__
>
> /* Syntactic details of assembler. */
>
Ok.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 13/13] aarch64: fix _mcount for pac-ret
2020-05-15 14:40 ` [PATCH v3 13/13] aarch64: fix _mcount for pac-ret Szabolcs Nagy
@ 2020-05-26 11:33 ` Adhemerval Zanella via Libc-alpha
2020-05-26 18:38 ` Szabolcs Nagy
0 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-26 11:33 UTC (permalink / raw)
To: libc-alpha
On 15/05/2020 11:40, Szabolcs Nagy wrote:
> gcc -pg with -mbranch-protection=pac-ret passes signed return address
> to _mcount, so _mcount now has to always strip pac from the frompc
> since that's from user code that may be built with pac-ret.
>
> This is a backward incompatible _mcount abi change introduced by
> return address signing support in gcc-7.
Which change are you referring about specifically?
Besides it, LGTM.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> TODO: fix -pg on the gcc side?
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94791
> ---
> sysdeps/aarch64/machine-gmon.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h
> index 730a23b781..328cbdda16 100644
> --- a/sysdeps/aarch64/machine-gmon.h
> +++ b/sysdeps/aarch64/machine-gmon.h
> @@ -30,5 +30,5 @@ static inline void mcount_internal (u_long frompc, u_long selfpc)
> #define MCOUNT \
> void __mcount (void *frompc) \
> { \
> - mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
> + mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \
> }
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 13/13] aarch64: fix _mcount for pac-ret
2020-05-26 11:33 ` Adhemerval Zanella via Libc-alpha
@ 2020-05-26 18:38 ` Szabolcs Nagy
2020-05-26 19:13 ` Adhemerval Zanella via Libc-alpha
0 siblings, 1 reply; 31+ messages in thread
From: Szabolcs Nagy @ 2020-05-26 18:38 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
The 05/26/2020 08:33, Adhemerval Zanella via Libc-alpha wrote:
>
>
> On 15/05/2020 11:40, Szabolcs Nagy wrote:
> > gcc -pg with -mbranch-protection=pac-ret passes signed return address
> > to _mcount, so _mcount now has to always strip pac from the frompc
> > since that's from user code that may be built with pac-ret.
> >
> > This is a backward incompatible _mcount abi change introduced by
> > return address signing support in gcc-7.
>
> Which change are you referring about specifically?
gcc-7 introduced -msigned-return-address (which was
later deprecated by -mbranch-protection=pac-ret) and
the code generation with -pg is not compatible with
existing _mcount runtime implementations (because
now _mcount needs unconditional xpac)
i'm still trying to see if this _mcount change is
needed or we can change gcc to do the right thing
(and not pass signed address to _mcount), but i
need internal consensus about such abi break first.
>
> Besides it, LGTM.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> >
> > TODO: fix -pg on the gcc side?
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94791
> > ---
> > sysdeps/aarch64/machine-gmon.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h
> > index 730a23b781..328cbdda16 100644
> > --- a/sysdeps/aarch64/machine-gmon.h
> > +++ b/sysdeps/aarch64/machine-gmon.h
> > @@ -30,5 +30,5 @@ static inline void mcount_internal (u_long frompc, u_long selfpc)
> > #define MCOUNT \
> > void __mcount (void *frompc) \
> > { \
> > - mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
> > + mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \
> > }
> >
--
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 13/13] aarch64: fix _mcount for pac-ret
2020-05-26 18:38 ` Szabolcs Nagy
@ 2020-05-26 19:13 ` Adhemerval Zanella via Libc-alpha
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-26 19:13 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha
On 26/05/2020 15:38, Szabolcs Nagy wrote:
> The 05/26/2020 08:33, Adhemerval Zanella via Libc-alpha wrote:
>>
>>
>> On 15/05/2020 11:40, Szabolcs Nagy wrote:
>>> gcc -pg with -mbranch-protection=pac-ret passes signed return address
>>> to _mcount, so _mcount now has to always strip pac from the frompc
>>> since that's from user code that may be built with pac-ret.
>>>
>>> This is a backward incompatible _mcount abi change introduced by
>>> return address signing support in gcc-7.
>>
>> Which change are you referring about specifically?
>
> gcc-7 introduced -msigned-return-address (which was
> later deprecated by -mbranch-protection=pac-ret) and
> the code generation with -pg is not compatible with
> existing _mcount runtime implementations (because
> now _mcount needs unconditional xpac)
Thanks, could you add this explanation on commit message as
well?
>
> i'm still trying to see if this _mcount change is
> needed or we can change gcc to do the right thing
> (and not pass signed address to _mcount), but i
> need internal consensus about such abi break first.
>
>>
>> Besides it, LGTM.
>>
>> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>
>>>
>>> TODO: fix -pg on the gcc side?
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94791
>>> ---
>>> sysdeps/aarch64/machine-gmon.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h
>>> index 730a23b781..328cbdda16 100644
>>> --- a/sysdeps/aarch64/machine-gmon.h
>>> +++ b/sysdeps/aarch64/machine-gmon.h
>>> @@ -30,5 +30,5 @@ static inline void mcount_internal (u_long frompc, u_long selfpc)
>>> #define MCOUNT \
>>> void __mcount (void *frompc) \
>>> { \
>>> - mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
>>> + mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \
>>> }
>>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2020-05-26 19:13 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15 14:40 [PATCH v3 00/13] aarch64: branch protection support Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 01/13] elf.h: Add PT_GNU_PROPERTY Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 02/13] elf.h: add aarch64 property definitions Szabolcs Nagy
2020-05-18 15:26 ` Florian Weimer via Libc-alpha
2020-05-21 8:57 ` Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 03/13] Rewrite abi-note.S in C Szabolcs Nagy
2020-05-18 15:28 ` Florian Weimer via Libc-alpha
2020-05-20 10:27 ` Szabolcs Nagy
2020-05-20 10:34 ` Florian Weimer via Libc-alpha
2020-05-21 8:54 ` [PATCH v4] " Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 04/13] aarch64: configure test for BTI support Szabolcs Nagy
2020-05-25 18:41 ` Adhemerval Zanella via Libc-alpha
2020-05-25 18:48 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 05/13] aarch64: Add BTI support to assembly files Szabolcs Nagy
2020-05-25 18:49 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 06/13] aarch64: Rename place holder .S files to .c Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 07/13] aarch64: fix swapcontext for BTI Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 08/13] aarch64: fix RTLD_START " Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 09/13] aarch64: enable BTI at runtime Szabolcs Nagy
2020-05-25 19:53 ` Adhemerval Zanella via Libc-alpha
2020-05-26 11:20 ` Szabolcs Nagy
2020-05-15 14:40 ` [PATCH v3 10/13] aarch64: configure check for pac-ret code generation Szabolcs Nagy
2020-05-25 21:49 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 11/13] aarch64: Add pac-ret support to assembly files Szabolcs Nagy
2020-05-26 11:26 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 12/13] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
2020-05-26 11:29 ` Adhemerval Zanella via Libc-alpha
2020-05-15 14:40 ` [PATCH v3 13/13] aarch64: fix _mcount for pac-ret Szabolcs Nagy
2020-05-26 11:33 ` Adhemerval Zanella via Libc-alpha
2020-05-26 18:38 ` Szabolcs Nagy
2020-05-26 19:13 ` Adhemerval Zanella via Libc-alpha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).