* [PATCH 00/12] aarch64: branch protection support
@ 2020-04-30 17:34 Szabolcs Nagy
2020-04-30 17:37 ` [PATCH 01/12] elf.h: Add PT_GNU_PROPERTY Szabolcs Nagy
` (12 more replies)
0 siblings, 13 replies; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:34 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
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, but, just like BTI,
it can be used in libc binaries.
The patch series is not finalized:
- PAC-RET may need to be configure checked and disabled if user
did not explicitly configured glibc with standard branch
protection, because it can have compatibility problems:
requires recent libgcc for working unwinding.
- The GNU property ELF marking can trigger ugly linker warnings
before binutils-2.33 so probably BTI should not be added
unconditionally either.
- Changed the logic of how NOTEs are processed (which may
affect x86 too) because I only wanted to handle PT_GNU_PROPERTY
not PT_NOTE on aarch64. (Otherwise note handling is similar
to the x86 code.)
- Some changes may be better handled by target hooks
(e.g. moved abi-note.S to C and copied the syscall template
just to add the BTI property NOTE)
- The -pg profiling abi with PAC-RET is not finalized: _mcount
currently may get a signed return address as argument so either
it has to remove it or gcc -pg has to be fixed not to pass
such argument to _mcount. (glibc gmon tests currently fail)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94791
- Redefined RETURN_ADDRESS for aarch64, this may change depending
on the ruling about
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94891
- I was considering separating out the bits that are necessary for
just enabling BTI to work in user binaries from changes that are
needed for building glibc itself with BTI, but decided against
it as it needs more work, cannot work with static linking and
unlikely to be very useful.
Ran cross tests in qemu using the linux bti patches from
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
i did some changes to the posted patches and rerunning the tests
now, the previous results:
FAIL: gmon/tst-gmon-gprof
FAIL: gmon/tst-gmon-pie-gprof
FAIL: gmon/tst-gmon-static-gprof
_mcount ABI with pac-ret
FAIL: misc/tst-atomic
FAIL: nptl/tst-cancel7
FAIL: nptl/tst-cancelx7
not reproducible issues
FAIL: elf/tst-ldconfig-ld_so_conf-update
not sure, likely something in my cross test setup makes
/etc/ld.so.cache not being reread after a change (nfs?).
FAIL: elf/tst-audit14
FAIL: elf/tst-audit15
FAIL: elf/tst-audit16
missing /dev/stdout (even if i have /dev/stdout
these fail because reading /proc/self/fd/1 fails
in my cross test setup)
FAIL: io/ftwtest
FAIL: libio/tst-wfile-sync
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 issues
Sudakshina Das (3):
aarch64: Add BTI landing pads to assembly code
aarch64: support BTI enabled binaries
aarch64: Configure option to build glibc with branch protection
Szabolcs Nagy (9):
elf.h: Add PT_GNU_PROPERTY
elf.h: add aarch64 property definitions
aarch64: Rename place holder .S files to .c
aarch64: fix swapcontext for BTI
aarch64: fix RTLD_START for BTI
aarch64: fix syscalls for BTI
Rewrite abi-note.S in C.
aarch64: Add pac-ret support to asm files
aarch64: redefine RETURN_ADDRESS to strip PAC
configure | 14 +-
configure.ac | 6 +
csu/{abi-note.S => abi-note.c} | 24 +--
elf/dl-load.c | 2 +
elf/elf.h | 7 +
elf/rtld.c | 2 +
sysdeps/aarch64/Makefile | 8 +
sysdeps/aarch64/__longjmp.S | 1 +
.../aarch64/{bsd-_setjmp.S => bsd-_setjmp.c} | 0
.../aarch64/{bsd-setjmp.S => bsd-setjmp.c} | 0
sysdeps/aarch64/configure | 31 ++++
sysdeps/aarch64/configure.ac | 19 ++
sysdeps/aarch64/crti.S | 12 ++
sysdeps/aarch64/crtn.S | 10 ++
sysdeps/aarch64/dl-bti.c | 54 ++++++
sysdeps/aarch64/dl-machine.h | 5 +-
sysdeps/aarch64/dl-prop.h | 170 ++++++++++++++++++
sysdeps/aarch64/dl-tlsdesc.S | 13 ++
sysdeps/aarch64/dl-trampoline.S | 19 +-
sysdeps/aarch64/linkmap.h | 1 +
sysdeps/aarch64/memchr.S | 1 +
sysdeps/aarch64/memcmp.S | 1 +
sysdeps/aarch64/memcpy.S | 1 +
sysdeps/aarch64/{memmove.S => memmove.c} | 0
sysdeps/aarch64/memrchr.S | 1 +
sysdeps/aarch64/memset.S | 1 +
sysdeps/aarch64/multiarch/memchr_nosimd.S | 1 +
sysdeps/aarch64/multiarch/memcpy_falkor.S | 1 +
sysdeps/aarch64/multiarch/memcpy_thunderx.S | 1 +
sysdeps/aarch64/multiarch/memcpy_thunderx2.S | 1 +
sysdeps/aarch64/multiarch/memmove_falkor.S | 1 +
sysdeps/aarch64/multiarch/memset_base64.S | 1 +
sysdeps/aarch64/multiarch/memset_kunpeng.S | 1 +
sysdeps/aarch64/multiarch/strlen_asimd.S | 1 +
sysdeps/aarch64/rawmemchr.S | 1 +
sysdeps/aarch64/setjmp.S | 1 +
sysdeps/aarch64/start.S | 2 +
sysdeps/aarch64/strchr.S | 1 +
sysdeps/aarch64/strchrnul.S | 1 +
sysdeps/aarch64/strcmp.S | 1 +
sysdeps/aarch64/strcpy.S | 1 +
sysdeps/aarch64/strlen.S | 1 +
sysdeps/aarch64/strncmp.S | 1 +
sysdeps/aarch64/strnlen.S | 1 +
sysdeps/aarch64/strrchr.S | 1 +
sysdeps/aarch64/sysdep.h | 53 +++++-
sysdeps/unix/sysv/linux/aarch64/__read_tp.S | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 ++++
sysdeps/unix/sysv/linux/aarch64/clone.S | 1 +
.../unix/sysv/linux/aarch64/cpu-features.c | 3 +
.../unix/sysv/linux/aarch64/cpu-features.h | 1 +
sysdeps/unix/sysv/linux/aarch64/getcontext.S | 1 +
sysdeps/unix/sysv/linux/aarch64/ioctl.S | 1 +
.../unix/sysv/linux/aarch64/libc-__read_tp.S | 1 +
sysdeps/unix/sysv/linux/aarch64/setcontext.S | 1 +
sysdeps/unix/sysv/linux/aarch64/swapcontext.S | 15 +-
.../sysv/linux/aarch64/syscall-template.S | 20 +++
sysdeps/unix/sysv/linux/aarch64/syscall.S | 1 +
sysdeps/unix/sysv/linux/aarch64/umount2.S | 25 +++
sysdeps/unix/sysv/linux/aarch64/vfork.S | 1 +
61 files changed, 564 insertions(+), 16 deletions(-)
rename csu/{abi-note.S => abi-note.c} (85%)
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%)
create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
create mode 100644 sysdeps/unix/sysv/linux/aarch64/syscall-template.S
create mode 100644 sysdeps/unix/sysv/linux/aarch64/umount2.S
--
2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 01/12] elf.h: Add PT_GNU_PROPERTY
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
@ 2020-04-30 17:37 ` Szabolcs Nagy
2020-05-07 14:49 ` Adhemerval Zanella
2020-04-30 17:37 ` [PATCH 02/12] elf.h: add aarch64 property definitions Szabolcs Nagy
` (11 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:37 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
This program header type is already used in binaries on x86 and
aarch64 targets.
---
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 [flat|nested] 48+ messages in thread
* [PATCH 02/12] elf.h: add aarch64 property definitions
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
2020-04-30 17:37 ` [PATCH 01/12] elf.h: Add PT_GNU_PROPERTY Szabolcs Nagy
@ 2020-04-30 17:37 ` Szabolcs Nagy
2020-05-07 14:50 ` Adhemerval Zanella
2020-04-30 17:39 ` [PATCH 03/12] aarch64: Add BTI landing pads to assembly code Szabolcs Nagy
` (10 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:37 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
These property values are specified by the AArch64 ELF ABI and
binutils can create binaries marked with them.
---
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 [flat|nested] 48+ messages in thread
* [PATCH 03/12] aarch64: Add BTI landing pads to assembly code
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
2020-04-30 17:37 ` [PATCH 01/12] elf.h: Add PT_GNU_PROPERTY Szabolcs Nagy
2020-04-30 17:37 ` [PATCH 02/12] elf.h: add aarch64 property definitions Szabolcs Nagy
@ 2020-04-30 17:39 ` Szabolcs Nagy
2020-05-07 16:55 ` Adhemerval Zanella
2020-04-30 17:40 ` [PATCH 04/12] aarch64: Rename place holder .S files to .c Szabolcs Nagy
` (9 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:39 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0003-aarch64-Add-BTI-landing-pads-to-assembly-code.patch --]
[-- Type: text/x-diff, Size: 17761 bytes --]
From 550fe66ed93e13c0f063955e81bfcb8db386413c Mon Sep 17 00:00:00 2001
From: Sudakshina Das <sudi.das@arm.com>
Date: Tue, 17 Mar 2020 15:44:18 +0000
Subject: [PATCH 03/12] aarch64: Add BTI landing pads to assembly code
Adding the landing pads and the ELF markings are required if
glibc is built with branch protection. For the handful of asm
files this is done unconditionally, this simplifies maintenance
and avoids complications where code layout is carefully aligned
such that conditionally turning BTI off may cause performance
regression (e.g. string functions).
Note: old binutils ld just merges notes of input objects into
the output, so if any input has a note claiming BTI support,
then the output will have such note too which is undesirable.
So after this commit libc shared objects and binaries linked
with libc crt code may contain incorrect notes. For this reason
checking properties in PT_NOTE is not reliable, new linkers
create PT_GNU_PROPERTY which is always reliable when present
so on AArch64 only that should be checked for properties.
Note: functions using ENTRY or ENTRY_ALIGN now have an
additional BTI c after the function label so alignment of
the code changes, but ENTRY_ALIGN_AND_PAD was fixed so there
is no change to the existing code layout.
Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
sysdeps/aarch64/__longjmp.S | 1 +
sysdeps/aarch64/crti.S | 4 +++
sysdeps/aarch64/crtn.S | 4 +++
sysdeps/aarch64/dl-tlsdesc.S | 5 ++++
sysdeps/aarch64/dl-trampoline.S | 4 +++
sysdeps/aarch64/memchr.S | 1 +
sysdeps/aarch64/memcmp.S | 1 +
sysdeps/aarch64/memcpy.S | 1 +
sysdeps/aarch64/memrchr.S | 1 +
sysdeps/aarch64/memset.S | 1 +
sysdeps/aarch64/multiarch/memchr_nosimd.S | 1 +
sysdeps/aarch64/multiarch/memcpy_falkor.S | 1 +
sysdeps/aarch64/multiarch/memcpy_thunderx.S | 1 +
sysdeps/aarch64/multiarch/memcpy_thunderx2.S | 1 +
sysdeps/aarch64/multiarch/memmove_falkor.S | 1 +
sysdeps/aarch64/multiarch/memset_base64.S | 1 +
sysdeps/aarch64/multiarch/memset_kunpeng.S | 1 +
sysdeps/aarch64/multiarch/strlen_asimd.S | 1 +
sysdeps/aarch64/rawmemchr.S | 1 +
sysdeps/aarch64/setjmp.S | 1 +
sysdeps/aarch64/start.S | 2 ++
sysdeps/aarch64/strchr.S | 1 +
sysdeps/aarch64/strchrnul.S | 1 +
sysdeps/aarch64/strcmp.S | 1 +
sysdeps/aarch64/strcpy.S | 1 +
sysdeps/aarch64/strlen.S | 1 +
sysdeps/aarch64/strncmp.S | 1 +
sysdeps/aarch64/strnlen.S | 1 +
sysdeps/aarch64/strrchr.S | 1 +
sysdeps/aarch64/sysdep.h | 27 ++++++++++++++++++-
sysdeps/unix/sysv/linux/aarch64/__read_tp.S | 1 +
sysdeps/unix/sysv/linux/aarch64/clone.S | 1 +
sysdeps/unix/sysv/linux/aarch64/getcontext.S | 1 +
sysdeps/unix/sysv/linux/aarch64/ioctl.S | 1 +
.../unix/sysv/linux/aarch64/libc-__read_tp.S | 1 +
sysdeps/unix/sysv/linux/aarch64/setcontext.S | 1 +
sysdeps/unix/sysv/linux/aarch64/syscall.S | 1 +
sysdeps/unix/sysv/linux/aarch64/vfork.S | 1 +
38 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/__longjmp.S b/sysdeps/aarch64/__longjmp.S
index f9060776b4..362171cdb8 100644
--- a/sysdeps/aarch64/__longjmp.S
+++ b/sysdeps/aarch64/__longjmp.S
@@ -116,3 +116,4 @@ ENTRY (__longjmp)
/* Use br instead of ret because ret is guaranteed to mispredict */
br x30
END (__longjmp)
+END_FILE
diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
index 1728eac37a..89a9e25f5b 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,8 @@ _init:
.hidden _fini
.type _fini, %function
_fini:
+ BTI_C
stp x29, x30, [sp, -16]!
mov x29, sp
+
+END_FILE
diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
index c3e97cc449..94a6f970ef 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
@@ -44,3 +46,5 @@
.section .fini,"ax",%progbits
ldp x29, x30, [sp], 16
RET
+
+END_FILE
diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
index 557ad1d505..d55e0443aa 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
@@ -235,3 +238,5 @@ _dl_tlsdesc_dynamic:
.size _dl_tlsdesc_dynamic, .-_dl_tlsdesc_dynamic
# undef NSAVEXREGPAIRS
#endif
+
+END_FILE
diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
index 94e965c096..fba5689d09 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)
@@ -298,3 +300,5 @@ _dl_runtime_profile:
.size _dl_runtime_profile, .-_dl_runtime_profile
#endif
.previous
+
+END_FILE
diff --git a/sysdeps/aarch64/memchr.S b/sysdeps/aarch64/memchr.S
index 85c65cbfca..c67a31223f 100644
--- a/sysdeps/aarch64/memchr.S
+++ b/sysdeps/aarch64/memchr.S
@@ -159,3 +159,4 @@ L(zero_length):
END (MEMCHR)
weak_alias (MEMCHR, memchr)
libc_hidden_builtin_def (memchr)
+END_FILE
diff --git a/sysdeps/aarch64/memcmp.S b/sysdeps/aarch64/memcmp.S
index 827f54f99e..c6e07f9287 100644
--- a/sysdeps/aarch64/memcmp.S
+++ b/sysdeps/aarch64/memcmp.S
@@ -178,3 +178,4 @@ END (memcmp)
#undef bcmp
weak_alias (memcmp, bcmp)
libc_hidden_builtin_def (memcmp)
+END_FILE
diff --git a/sysdeps/aarch64/memcpy.S b/sysdeps/aarch64/memcpy.S
index e0b4c4502f..543d9417f3 100644
--- a/sysdeps/aarch64/memcpy.S
+++ b/sysdeps/aarch64/memcpy.S
@@ -282,3 +282,4 @@ L(copy64_from_start):
END (MEMMOVE)
libc_hidden_builtin_def (MEMMOVE)
+END_FILE
diff --git a/sysdeps/aarch64/memrchr.S b/sysdeps/aarch64/memrchr.S
index ace5a94e8f..f35a68d14d 100644
--- a/sysdeps/aarch64/memrchr.S
+++ b/sysdeps/aarch64/memrchr.S
@@ -163,3 +163,4 @@ L(zero_length):
END (__memrchr)
weak_alias (__memrchr, memrchr)
libc_hidden_builtin_def (memrchr)
+END_FILE
diff --git a/sysdeps/aarch64/memset.S b/sysdeps/aarch64/memset.S
index ac577f1660..7cdae20563 100644
--- a/sysdeps/aarch64/memset.S
+++ b/sysdeps/aarch64/memset.S
@@ -189,3 +189,4 @@ L(zva_other):
END (MEMSET)
libc_hidden_builtin_def (MEMSET)
+END_FILE
diff --git a/sysdeps/aarch64/multiarch/memchr_nosimd.S b/sysdeps/aarch64/multiarch/memchr_nosimd.S
index 41ce10eb32..6d7d38d5bb 100644
--- a/sysdeps/aarch64/multiarch/memchr_nosimd.S
+++ b/sysdeps/aarch64/multiarch/memchr_nosimd.S
@@ -221,3 +221,4 @@ L(none_chr):
END (MEMCHR)
libc_hidden_builtin_def (MEMCHR)
+END_FILE
diff --git a/sysdeps/aarch64/multiarch/memcpy_falkor.S b/sysdeps/aarch64/multiarch/memcpy_falkor.S
index 35a1fae1b9..999aa48b16 100644
--- a/sysdeps/aarch64/multiarch/memcpy_falkor.S
+++ b/sysdeps/aarch64/multiarch/memcpy_falkor.S
@@ -188,4 +188,5 @@ L(last64):
END (__memcpy_falkor)
libc_hidden_builtin_def (__memcpy_falkor)
+END_FILE
#endif
diff --git a/sysdeps/aarch64/multiarch/memcpy_thunderx.S b/sysdeps/aarch64/multiarch/memcpy_thunderx.S
index e9407571b5..e6e36a6633 100644
--- a/sysdeps/aarch64/multiarch/memcpy_thunderx.S
+++ b/sysdeps/aarch64/multiarch/memcpy_thunderx.S
@@ -318,5 +318,6 @@ L(move_long):
END (MEMCPY)
libc_hidden_builtin_def (MEMCPY)
+END_FILE
#endif
diff --git a/sysdeps/aarch64/multiarch/memcpy_thunderx2.S b/sysdeps/aarch64/multiarch/memcpy_thunderx2.S
index 68e99455c8..fde4c7198c 100644
--- a/sysdeps/aarch64/multiarch/memcpy_thunderx2.S
+++ b/sysdeps/aarch64/multiarch/memcpy_thunderx2.S
@@ -474,4 +474,5 @@ L(ext_table):
.word L(ext_size_15) -.
libc_hidden_builtin_def (MEMCPY)
+END_FILE
#endif
diff --git a/sysdeps/aarch64/multiarch/memmove_falkor.S b/sysdeps/aarch64/multiarch/memmove_falkor.S
index 35fc1fdd41..d8cc992d27 100644
--- a/sysdeps/aarch64/multiarch/memmove_falkor.S
+++ b/sysdeps/aarch64/multiarch/memmove_falkor.S
@@ -223,3 +223,4 @@ L(move_long):
END (__memmove_falkor)
libc_hidden_builtin_def (__memmove_falkor)
+END_FILE
diff --git a/sysdeps/aarch64/multiarch/memset_base64.S b/sysdeps/aarch64/multiarch/memset_base64.S
index 8f85cd1caf..ee0b832ef9 100644
--- a/sysdeps/aarch64/multiarch/memset_base64.S
+++ b/sysdeps/aarch64/multiarch/memset_base64.S
@@ -184,3 +184,4 @@ L(zva_64):
END (MEMSET)
libc_hidden_builtin_def (MEMSET)
+END_FILE
diff --git a/sysdeps/aarch64/multiarch/memset_kunpeng.S b/sysdeps/aarch64/multiarch/memset_kunpeng.S
index 8e051d4fd1..aa7ab62fdd 100644
--- a/sysdeps/aarch64/multiarch/memset_kunpeng.S
+++ b/sysdeps/aarch64/multiarch/memset_kunpeng.S
@@ -111,3 +111,4 @@ L(set_long):
END (MEMSET)
libc_hidden_builtin_def (MEMSET)
#endif
+END_FILE
diff --git a/sysdeps/aarch64/multiarch/strlen_asimd.S b/sysdeps/aarch64/multiarch/strlen_asimd.S
index 236a2c96a6..c28aa0ca8d 100644
--- a/sysdeps/aarch64/multiarch/strlen_asimd.S
+++ b/sysdeps/aarch64/multiarch/strlen_asimd.S
@@ -176,3 +176,4 @@ L(page_cross):
END (__strlen_asimd)
weak_alias (__strlen_asimd, strlen_asimd)
libc_hidden_builtin_def (strlen_asimd)
+END_FILE
diff --git a/sysdeps/aarch64/rawmemchr.S b/sysdeps/aarch64/rawmemchr.S
index 5c7a664fb4..4ad614a169 100644
--- a/sysdeps/aarch64/rawmemchr.S
+++ b/sysdeps/aarch64/rawmemchr.S
@@ -40,3 +40,4 @@ L(do_strlen):
END (__rawmemchr)
weak_alias (__rawmemchr, rawmemchr)
libc_hidden_builtin_def (__rawmemchr)
+END_FILE
diff --git a/sysdeps/aarch64/setjmp.S b/sysdeps/aarch64/setjmp.S
index 28fdd3f46a..9a0201ef70 100644
--- a/sysdeps/aarch64/setjmp.S
+++ b/sysdeps/aarch64/setjmp.S
@@ -73,3 +73,4 @@ ENTRY (__sigsetjmp)
#endif
END (__sigsetjmp)
hidden_def (__sigsetjmp)
+END_FILE
diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
index d96cf57e2d..e6c0393c20 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
@@ -110,3 +111,4 @@ __data_start:
.long 0
.weak data_start
data_start = __data_start
+END_FILE
diff --git a/sysdeps/aarch64/strchr.S b/sysdeps/aarch64/strchr.S
index 4a75e73945..e1f98aa42c 100644
--- a/sysdeps/aarch64/strchr.S
+++ b/sysdeps/aarch64/strchr.S
@@ -137,3 +137,4 @@ L(tail):
END (strchr)
libc_hidden_builtin_def (strchr)
weak_alias (strchr, index)
+END_FILE
diff --git a/sysdeps/aarch64/strchrnul.S b/sysdeps/aarch64/strchrnul.S
index a65be6cba8..a9ccc54205 100644
--- a/sysdeps/aarch64/strchrnul.S
+++ b/sysdeps/aarch64/strchrnul.S
@@ -129,3 +129,4 @@ L(tail):
END(__strchrnul)
weak_alias (__strchrnul, strchrnul)
+END_FILE
diff --git a/sysdeps/aarch64/strcmp.S b/sysdeps/aarch64/strcmp.S
index d044c29e9b..a7bee697c0 100644
--- a/sysdeps/aarch64/strcmp.S
+++ b/sysdeps/aarch64/strcmp.S
@@ -182,3 +182,4 @@ L(done):
RET
END(strcmp)
libc_hidden_builtin_def (strcmp)
+END_FILE
diff --git a/sysdeps/aarch64/strcpy.S b/sysdeps/aarch64/strcpy.S
index 548130e413..631da4a358 100644
--- a/sysdeps/aarch64/strcpy.S
+++ b/sysdeps/aarch64/strcpy.S
@@ -321,3 +321,4 @@ libc_hidden_builtin_def (stpcpy)
#else
libc_hidden_builtin_def (strcpy)
#endif
+END_FILE
diff --git a/sysdeps/aarch64/strlen.S b/sysdeps/aarch64/strlen.S
index e01fab7c2a..3af25de4b8 100644
--- a/sysdeps/aarch64/strlen.S
+++ b/sysdeps/aarch64/strlen.S
@@ -222,3 +222,4 @@ L(page_cross):
END (STRLEN)
weak_alias (STRLEN, strlen)
libc_hidden_builtin_def (strlen)
+END_FILE
diff --git a/sysdeps/aarch64/strncmp.S b/sysdeps/aarch64/strncmp.S
index c5141fab8a..d289d4a3ba 100644
--- a/sysdeps/aarch64/strncmp.S
+++ b/sysdeps/aarch64/strncmp.S
@@ -270,3 +270,4 @@ L(ret0):
END (strncmp)
libc_hidden_builtin_def (strncmp)
+END_FILE
diff --git a/sysdeps/aarch64/strnlen.S b/sysdeps/aarch64/strnlen.S
index 5981247dd9..964536dba9 100644
--- a/sysdeps/aarch64/strnlen.S
+++ b/sysdeps/aarch64/strnlen.S
@@ -213,3 +213,4 @@ END (__strnlen)
libc_hidden_def (__strnlen)
weak_alias (__strnlen, strnlen)
libc_hidden_def (strnlen)
+END_FILE
diff --git a/sysdeps/aarch64/strrchr.S b/sysdeps/aarch64/strrchr.S
index 94da08d351..9f6d956f21 100644
--- a/sysdeps/aarch64/strrchr.S
+++ b/sysdeps/aarch64/strrchr.S
@@ -164,3 +164,4 @@ L(null_search):
END(strrchr)
weak_alias (strrchr, rindex)
libc_hidden_builtin_def (strrchr)
+END_FILE
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 604c489170..07dc7858a5 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -41,12 +41,35 @@
#define ASM_SIZE_DIRECTIVE(name) .size name,.-name
+/* Branch Target Identitication support. */
+#define BTI_C hint 34
+#define BTI_J hint 36
+
+#define FEATURE_1_BTI 1
+#define FEATURE_1_PAC 2
+
+/* Add a GNU_PROPERTY_AARCH64_FEATURE_1_AND note. */
+#define GNU_PROPERTY(features) \
+ .section .note.gnu.property, "a"; \
+ .p2align 3; \
+ .word 4; \
+ .word 16; \
+ .word 5; \
+ .asciz "GNU"; \
+ .word 0xc0000000; \
+ .word 4; \
+ .word features; \
+ .word 0;
+
+#define END_FILE GNU_PROPERTY(FEATURE_1_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 +79,7 @@
.type C_SYMBOL_NAME(name),%function; \
.p2align align; \
C_LABEL(name) \
+ BTI_C; \
cfi_startproc; \
CALL_MCOUNT
@@ -68,10 +92,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
diff --git a/sysdeps/unix/sysv/linux/aarch64/__read_tp.S b/sysdeps/unix/sysv/linux/aarch64/__read_tp.S
index 12e1131fe7..7825aa5e1b 100644
--- a/sysdeps/unix/sysv/linux/aarch64/__read_tp.S
+++ b/sysdeps/unix/sysv/linux/aarch64/__read_tp.S
@@ -23,3 +23,4 @@ ENTRY (__read_tp)
mrs x0, tpidr_el0
RET
END (__read_tp)
+END_FILE
diff --git a/sysdeps/unix/sysv/linux/aarch64/clone.S b/sysdeps/unix/sysv/linux/aarch64/clone.S
index 2b14106fd1..15b5a7b217 100644
--- a/sysdeps/unix/sysv/linux/aarch64/clone.S
+++ b/sysdeps/unix/sysv/linux/aarch64/clone.S
@@ -85,3 +85,4 @@ thread_start:
libc_hidden_def (__clone)
weak_alias (__clone, clone)
+END_FILE
diff --git a/sysdeps/unix/sysv/linux/aarch64/getcontext.S b/sysdeps/unix/sysv/linux/aarch64/getcontext.S
index 8571556189..95f61d5f28 100644
--- a/sysdeps/unix/sysv/linux/aarch64/getcontext.S
+++ b/sysdeps/unix/sysv/linux/aarch64/getcontext.S
@@ -107,3 +107,4 @@ ENTRY(__getcontext)
PSEUDO_END (__getcontext)
weak_alias (__getcontext, getcontext)
+END_FILE
diff --git a/sysdeps/unix/sysv/linux/aarch64/ioctl.S b/sysdeps/unix/sysv/linux/aarch64/ioctl.S
index ed36e309c4..19abf43726 100644
--- a/sysdeps/unix/sysv/linux/aarch64/ioctl.S
+++ b/sysdeps/unix/sysv/linux/aarch64/ioctl.S
@@ -30,3 +30,4 @@ PSEUDO_END (__ioctl)
libc_hidden_def (__ioctl)
weak_alias (__ioctl, ioctl)
+END_FILE
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-__read_tp.S b/sysdeps/unix/sysv/linux/aarch64/libc-__read_tp.S
index 6de259ed83..df5eec8599 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc-__read_tp.S
+++ b/sysdeps/unix/sysv/linux/aarch64/libc-__read_tp.S
@@ -17,3 +17,4 @@
<https://www.gnu.org/licenses/>. */
#include <__read_tp.S>
+END_FILE
diff --git a/sysdeps/unix/sysv/linux/aarch64/setcontext.S b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
index 61fb813db3..d9d6c0f364 100644
--- a/sysdeps/unix/sysv/linux/aarch64/setcontext.S
+++ b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
@@ -128,3 +128,4 @@ ENTRY (__startcontext)
cbnz x0, __setcontext
1: b HIDDEN_JUMPTARGET (exit)
END (__startcontext)
+END_FILE
diff --git a/sysdeps/unix/sysv/linux/aarch64/syscall.S b/sysdeps/unix/sysv/linux/aarch64/syscall.S
index 993e307fcf..10e59ece58 100644
--- a/sysdeps/unix/sysv/linux/aarch64/syscall.S
+++ b/sysdeps/unix/sysv/linux/aarch64/syscall.S
@@ -42,3 +42,4 @@ ENTRY (syscall)
1:
b SYSCALL_ERROR
PSEUDO_END (syscall)
+END_FILE
diff --git a/sysdeps/unix/sysv/linux/aarch64/vfork.S b/sysdeps/unix/sysv/linux/aarch64/vfork.S
index 9c8ea48c5a..0306003a22 100644
--- a/sysdeps/unix/sysv/linux/aarch64/vfork.S
+++ b/sysdeps/unix/sysv/linux/aarch64/vfork.S
@@ -40,3 +40,4 @@ libc_hidden_def (__vfork)
weak_alias (__vfork, vfork)
strong_alias (__vfork, __libc_vfork)
+END_FILE
--
2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 04/12] aarch64: Rename place holder .S files to .c
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (2 preceding siblings ...)
2020-04-30 17:39 ` [PATCH 03/12] aarch64: Add BTI landing pads to assembly code Szabolcs Nagy
@ 2020-04-30 17:40 ` Szabolcs Nagy
2020-05-07 18:29 ` Adhemerval Zanella
2020-04-30 17:41 ` [PATCH 05/12] aarch64: fix swapcontext for BTI Szabolcs Nagy
` (8 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:40 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0004-aarch64-Rename-place-holder-.S-files-to-.c.patch --]
[-- Type: text/x-diff, Size: 1304 bytes --]
From 2084e33755ee4d00b3ecf956b1705d8eea46ebc7 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Wed, 1 Apr 2020 09:52:37 +0100
Subject: [PATCH 04/12] aarch64: Rename place holder .S files to .c
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.
---
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] 48+ messages in thread
* [PATCH 05/12] aarch64: fix swapcontext for BTI
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (3 preceding siblings ...)
2020-04-30 17:40 ` [PATCH 04/12] aarch64: Rename place holder .S files to .c Szabolcs Nagy
@ 2020-04-30 17:41 ` Szabolcs Nagy
2020-05-07 18:42 ` Adhemerval Zanella
2020-04-30 17:42 ` [PATCH 06/12] aarch64: fix RTLD_START " Szabolcs Nagy
` (7 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:41 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0005-aarch64-fix-swapcontext-for-BTI.patch --]
[-- Type: text/x-diff, Size: 2566 bytes --]
From c156ff9d5e332ad17d7fa74b777e8f5466aff0a4 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Wed, 1 Apr 2020 10:31:41 +0100
Subject: [PATCH 05/12] aarch64: fix swapcontext for BTI
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.
---
sysdeps/unix/sysv/linux/aarch64/swapcontext.S | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/aarch64/swapcontext.S b/sysdeps/unix/sysv/linux/aarch64/swapcontext.S
index d30c543e6f..b60cf04c92 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,12 @@ 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)
+END_FILE
--
2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 06/12] aarch64: fix RTLD_START for BTI
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (4 preceding siblings ...)
2020-04-30 17:41 ` [PATCH 05/12] aarch64: fix swapcontext for BTI Szabolcs Nagy
@ 2020-04-30 17:42 ` Szabolcs Nagy
2020-05-07 18:49 ` Adhemerval Zanella
2020-04-30 17:42 ` [PATCH 07/12] aarch64: fix syscalls " Szabolcs Nagy
` (6 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:42 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0006-aarch64-fix-RTLD_START-for-BTI.patch --]
[-- Type: text/x-diff, Size: 1518 bytes --]
From 1e8662264c07e69d807761882e8d77f0916ae562 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Tue, 31 Mar 2020 17:32:14 +0100
Subject: [PATCH 06/12] aarch64: fix RTLD_START for BTI
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.
---
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 [flat|nested] 48+ messages in thread
* [PATCH 07/12] aarch64: fix syscalls for BTI
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (5 preceding siblings ...)
2020-04-30 17:42 ` [PATCH 06/12] aarch64: fix RTLD_START " Szabolcs Nagy
@ 2020-04-30 17:42 ` Szabolcs Nagy
2020-05-07 19:40 ` Adhemerval Zanella
2020-04-30 17:43 ` [PATCH 08/12] Rewrite abi-note.S in C Szabolcs Nagy
` (5 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:42 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0007-aarch64-fix-syscalls-for-BTI.patch --]
[-- Type: text/x-diff, Size: 3049 bytes --]
From 10741a1943239f0b22c8dd3f7df93aa338ec003f Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Wed, 1 Apr 2020 15:27:12 +0100
Subject: [PATCH 07/12] aarch64: fix syscalls for BTI
Syscall asm code needs an ELF property marking for BTI
when glibc is built with BTI support so we add AArch64
variants of syscall-template.S and umount2.S.
---
.../sysv/linux/aarch64/syscall-template.S | 20 +++++++++++++++
sysdeps/unix/sysv/linux/aarch64/umount2.S | 25 +++++++++++++++++++
2 files changed, 45 insertions(+)
create mode 100644 sysdeps/unix/sysv/linux/aarch64/syscall-template.S
create mode 100644 sysdeps/unix/sysv/linux/aarch64/umount2.S
diff --git a/sysdeps/unix/sysv/linux/aarch64/syscall-template.S b/sysdeps/unix/sysv/linux/aarch64/syscall-template.S
new file mode 100644
index 0000000000..50db585289
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/syscall-template.S
@@ -0,0 +1,20 @@
+/* Assembly code template for system call stubs.
+ 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/>. */
+
+#include <sysdeps/unix/syscall-template.S>
+END_FILE
diff --git a/sysdeps/unix/sysv/linux/aarch64/umount2.S b/sysdeps/unix/sysv/linux/aarch64/umount2.S
new file mode 100644
index 0000000000..4fe26c35e2
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/umount2.S
@@ -0,0 +1,25 @@
+/* umount system call with two parameters.
+ 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/>. */
+
+#include <sysdep.h>
+PSEUDO (__umount2, umount2, 2)
+ ret
+PSEUDO_END(__umount2)
+weak_alias (__umount2, umount2)
+END_FILE
--
2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 08/12] Rewrite abi-note.S in C.
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (6 preceding siblings ...)
2020-04-30 17:42 ` [PATCH 07/12] aarch64: fix syscalls " Szabolcs Nagy
@ 2020-04-30 17:43 ` Szabolcs Nagy
2020-04-30 20:07 ` Zack Weinberg
2020-04-30 17:44 ` [PATCH 09/12] aarch64: support BTI enabled binaries Szabolcs Nagy
` (4 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:43 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0008-Rewrite-abi-note.S-in-C.patch --]
[-- Type: text/x-diff, Size: 1863 bytes --]
From 0647d5658df82f89d8bf73dec65ef4aa6a4b77a9 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Wed, 1 Apr 2020 16:02:03 +0100
Subject: [PATCH 08/12] Rewrite abi-note.S in C.
Using C code with __asm() allows the compiler to add target
specific object file markings based on CFLAGS.
This is e.g. needed for building glibc with branch-protection
on AArch64.
---
csu/{abi-note.S => abi-note.c} | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
rename csu/{abi-note.S => abi-note.c} (85%)
diff --git a/csu/abi-note.S b/csu/abi-note.c
similarity index 85%
rename from csu/abi-note.S
rename to csu/abi-note.c
index 2b4b5f8824..5d86b452d9 100644
--- a/csu/abi-note.S
+++ b/csu/abi-note.c
@@ -56,17 +56,21 @@ offset length contents
#include <config.h>
#include <abi-tag.h> /* OS-specific ABI tag value */
+#define STRINGIFY(...) STRINGIFY_1 (__VA_ARGS__)
+#define STRINGIFY_1(...) #__VA_ARGS__
+
/* The linker (GNU ld 2.8 and later) recognizes an allocated section whose
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 */
+__asm (
+ " .section \".note.ABI-tag\", \"a\"\n"
+ " .p2align 2\n"
+ " .long 1f - 0f\n" /* name length */
+ " .long 3f - 2f\n" /* data length */
+ " .long 1\n" /* note type */
+ "0: .asciz \"GNU\"\n" /* vendor name */
+ "1: .p2align 2\n"
+ "2: .long " STRINGIFY (__ABI_TAG_OS) "\n" /* note data: the ABI tag */
+ " .long " STRINGIFY (__ABI_TAG_VERSION) "\n"
+ "3: .p2align 2"); /* pad out section */
--
2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 09/12] aarch64: support BTI enabled binaries
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (7 preceding siblings ...)
2020-04-30 17:43 ` [PATCH 08/12] Rewrite abi-note.S in C Szabolcs Nagy
@ 2020-04-30 17:44 ` Szabolcs Nagy
2020-05-07 21:07 ` Adhemerval Zanella
2020-04-30 17:44 ` [PATCH 10/12] aarch64: Add pac-ret support to asm files Szabolcs Nagy
` (3 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:44 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0009-aarch64-support-BTI-enabled-binaries.patch --]
[-- Type: text/x-diff, Size: 13035 bytes --]
From 45c6bce5a691ecec9bba52785bd1f3a4cbc76fd4 Mon Sep 17 00:00:00 2001
From: Sudakshina Das <sudi.das@arm.com>
Date: Tue, 17 Mar 2020 15:54:12 +0000
Subject: [PATCH 09/12] aarch64: support BTI enabled binaries
Binaries can opt-in to using BTI via an ELF property 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.
Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
elf/dl-load.c | 2 +
elf/rtld.c | 2 +
sysdeps/aarch64/Makefile | 4 +
sysdeps/aarch64/dl-bti.c | 54 ++++++
sysdeps/aarch64/dl-prop.h | 170 ++++++++++++++++++
sysdeps/aarch64/linkmap.h | 1 +
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 | 1 +
10 files changed, 269 insertions(+)
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 a6b80f9395..0930250619 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1145,6 +1145,8 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
l->l_relro_size = ph->p_memsz;
break;
+ case PT_GNU_PROPERTY:
+ /* Fall through. PT_GNU_PROPERTY holds property notes. */
case PT_NOTE:
if (_dl_process_pt_note (l, ph, fd, fbp))
{
diff --git a/elf/rtld.c b/elf/rtld.c
index b2ea21c98b..88b8e74de0 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1505,6 +1505,8 @@ 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:
+ /* Fall through. PT_GNU_PROPERTY holds property notes. */
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..9ce697527d
--- /dev/null
+++ b/sysdeps/aarch64/dl-bti.c
@@ -0,0 +1,54 @@
+/* AArch64 BTI initializers function.
+ 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_guarded)
+ enable_bti (l, program);
+}
diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
new file mode 100644
index 0000000000..6662e4ab14
--- /dev/null
+++ b/sysdeps/aarch64/dl-prop.h
@@ -0,0 +1,170 @@
+/* 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, 0);
+}
+
+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 aliged 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_guarded = 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);
+ }
+
+ /* NB: Note sections like .note.ABI-tag and .note.gnu.build-id are
+ aligned to 4 bytes in 64-bit ELF objects. */
+ note = ((const void *) note
+ + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
+ align));
+ }
+}
+
+#ifdef FILEBUF_SIZE
+static inline int __attribute__ ((unused))
+_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
+ int fd, struct filebuf *fbp)
+{
+ if (ph->p_type != PT_GNU_PROPERTY)
+ return 0;
+
+ const ElfW(Nhdr) *note;
+ ElfW(Nhdr) *note_malloced = NULL;
+ ElfW(Addr) size = ph->p_filesz;
+
+ if (ph->p_offset + size <= (size_t) fbp->len)
+ note = (const void *) (fbp->buf + ph->p_offset);
+ else
+ {
+ if (size < __MAX_ALLOCA_CUTOFF)
+ note = alloca (size);
+ else
+ note = note_malloced = malloc (size);
+ if (note == NULL)
+ return -1;
+ if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
+ {
+ if (note_malloced)
+ free (note_malloced);
+ return -1;
+ }
+ }
+ _dl_process_aarch64_property (l, note, ph->p_filesz, ph->p_align);
+ if (note_malloced)
+ free (note_malloced);
+ return 0;
+}
+#endif
+
+static inline int __attribute__ ((unused))
+_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
+{
+ if (ph->p_type != PT_GNU_PROPERTY)
+ return 0;
+ 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..cc196512d7 100644
--- a/sysdeps/aarch64/linkmap.h
+++ b/sysdeps/aarch64/linkmap.h
@@ -20,4 +20,5 @@ struct link_map_machine
{
ElfW(Addr) plt; /* Address of .plt */
void *tlsdesc_table; /* Address of TLS descriptor hash table. */
+ int bti_guarded; /* Branch Target Identification mechanism enabled. */
};
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..c2385fb498 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 enabled. */
+ 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..88983eb723 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
@@ -64,6 +64,7 @@ struct cpu_features
{
uint64_t midr_el1;
unsigned zva_size;
+ int bti;
};
#endif /* _CPU_FEATURES_AARCH64_H */
--
2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 10/12] aarch64: Add pac-ret support to asm files
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (8 preceding siblings ...)
2020-04-30 17:44 ` [PATCH 09/12] aarch64: support BTI enabled binaries Szabolcs Nagy
@ 2020-04-30 17:44 ` Szabolcs Nagy
2020-05-08 16:59 ` Adhemerval Zanella
2020-04-30 17:45 ` [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
` (2 subsequent siblings)
12 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:44 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0010-aarch64-Add-pac-ret-support-to-asm-files.patch --]
[-- Type: text/x-diff, Size: 4253 bytes --]
From de8968ed58686c26391de8343184a1283bb5e305 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Wed, 29 Apr 2020 11:49:20 +0100
Subject: [PATCH 10/12] aarch64: Add pac-ret support to asm files
This patch unconditionally enables pac-ret in asm files.
TODO: This will need configure checks, cannot be done
unconditionally because we cannot guarantee pac-ret
compatibility (e.g. libgcc unwinder had no support for
it before gcc-7 and newer libgcc had bugs that could
cause unwind crash when pac-ret and non-pac-ret stack
frames are mixed)
---
sysdeps/aarch64/crti.S | 8 ++++++++
sysdeps/aarch64/crtn.S | 6 ++++++
sysdeps/aarch64/dl-tlsdesc.S | 8 ++++++++
sysdeps/aarch64/dl-trampoline.S | 15 ++++++++++++++-
sysdeps/aarch64/sysdep.h | 18 +++++++++++++++++-
5 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
index 89a9e25f5b..36f58c9a01 100644
--- a/sysdeps/aarch64/crti.S
+++ b/sysdeps/aarch64/crti.S
@@ -75,7 +75,11 @@ call_weak_fn:
.hidden _init
.type _init, %function
_init:
+#if ENABLE_PAC_RET
+ PACIASP
+#else
BTI_C
+#endif
stp x29, x30, [sp, -16]!
mov x29, sp
#if PREINIT_FUNCTION_WEAK
@@ -90,7 +94,11 @@ _init:
.hidden _fini
.type _fini, %function
_fini:
+#if ENABLE_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 94a6f970ef..e1cb74a572 100644
--- a/sysdeps/aarch64/crtn.S
+++ b/sysdeps/aarch64/crtn.S
@@ -41,10 +41,16 @@
.section .init,"ax",%progbits
ldp x29, x30, [sp], 16
+#if ENABLE_PAC_RET
+ AUTIASP
+#endif
RET
.section .fini,"ax",%progbits
ldp x29, x30, [sp], 16
+#if ENABLE_PAC_RET
+ AUTIASP
+#endif
RET
END_FILE
diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
index d55e0443aa..25628d942f 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. */
+# if ENABLE_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)
+#if ENABLE_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 fba5689d09..c0c4c23128 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:
+# if ENABLE_PAC_RET
+ PACIASP
+ cfi_window_save
+# else
BTI_C
+# endif
/* AArch64 we get called with:
ip0 &PLTGOT[2]
ip1 temp(dl resolver entry point)
@@ -291,9 +296,17 @@ _dl_runtime_profile:
cfi_def_cfa_register (sp)
ldr x29, [x29, #0]
cfi_restore(x29)
+# if ENABLE_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
br lr
cfi_endproc
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 07dc7858a5..63a04a70cd 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -45,6 +45,18 @@
#define BTI_C hint 34
#define BTI_J hint 36
+/* Return address signing support (pac-ret). */
+#define ENABLE_PAC_RET 1
+#if ENABLE_PAC_RET
+# define PACIASP hint 25
+# define AUTIASP hint 29
+# define PACIASP_AND_BTI_C PACIASP
+#else
+# define PACIASP
+# define AUTIASP
+# define PACIASP_AND_BTI_C BTI_C
+#endif
+
#define FEATURE_1_BTI 1
#define FEATURE_1_PAC 2
@@ -61,7 +73,11 @@
.word features; \
.word 0;
-#define END_FILE GNU_PROPERTY(FEATURE_1_BTI)
+#if ENABLE_PAC_RET
+# define END_FILE GNU_PROPERTY(FEATURE_1_BTI|FEATURE_1_PAC)
+#else
+# define END_FILE GNU_PROPERTY(FEATURE_1_BTI)
+#endif
/* Define an entry point visible from C. */
#define ENTRY(name) \
--
2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (9 preceding siblings ...)
2020-04-30 17:44 ` [PATCH 10/12] aarch64: Add pac-ret support to asm files Szabolcs Nagy
@ 2020-04-30 17:45 ` Szabolcs Nagy
2020-05-08 17:44 ` Adhemerval Zanella
2020-05-11 19:22 ` Florian Weimer
2020-04-30 17:45 ` [PATCH 12/12] aarch64: Configure option to build glibc with branch protection Szabolcs Nagy
2020-05-04 11:27 ` [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
12 siblings, 2 replies; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:45 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0011-aarch64-redefine-RETURN_ADDRESS-to-strip-PAC.patch --]
[-- Type: text/x-diff, Size: 1393 bytes --]
From 2223e5ed1d78634ef59f0a7efbd3a9885a8da53f Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Wed, 15 Apr 2020 17:40:45 +0100
Subject: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
RETURN_ADDRESS is used at several places in glibc to mean a valid
code address of the call site, but with pac-ret that includes a
pointer authentication code, so the definition is adjusted.
XPAC is added unconditionally for now, but it's only needed if
glibc is compiled with -mbranch-protection=pac-ret. Inline asm
is used instead of __builtin_aarch64_xpaclri since that's an
undocumented builtin and not available in all supported gccs.
---
sysdeps/aarch64/sysdep.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 63a04a70cd..87f19b9bef 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -35,6 +35,16 @@
#define PTR_SIZE (1<<PTR_LOG_SIZE)
+/* Strip pointer authentication code from pointer p. */
+#define XPAC(p) ({ \
+ register void *__ra asm ("x30") = (p); \
+ asm ("hint 7 // xpaclri" : "+r"(__ra)); \
+ __ra;})
+
+/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
+#undef RETURN_ADDRESS
+#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
+
#ifdef __ASSEMBLER__
/* Syntactic details of assembler. */
--
2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 12/12] aarch64: Configure option to build glibc with branch protection
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (10 preceding siblings ...)
2020-04-30 17:45 ` [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
@ 2020-04-30 17:45 ` Szabolcs Nagy
2020-04-30 19:02 ` Joseph Myers
2020-05-08 17:53 ` Adhemerval Zanella
2020-05-04 11:27 ` [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
12 siblings, 2 replies; 48+ messages in thread
From: Szabolcs Nagy @ 2020-04-30 17:45 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
[-- Attachment #1: 0012-aarch64-Configure-option-to-build-glibc-with-branch-.patch --]
[-- Type: text/x-diff, Size: 6645 bytes --]
From ec96daabd8fad129ea3660d479fa63941712c410 Mon Sep 17 00:00:00 2001
From: Sudakshina Das <sudi.das@arm.com>
Date: Thu, 26 Mar 2020 11:49:48 +0000
Subject: [PATCH 12/12] aarch64: Configure option to build glibc with branch
protection
If gcc is configured with --enable-standard-branch-protection then
the built glibc should have branch protection suppport too, which
includes bti and pac-ret. The new configure option is only for
additional configure checks, it does not try to add new CFLAGS
(i.e. -mbranch-protection=standard ), it expects gcc to default to
using branch protection, since likely the static linked compiler
libraries are not compatible otherwise.
The -z force-bti linker flag is also passed with branch protection,
but this is not very useful: by default the BTI property marking
is set on the linker output if all linker inputs have it and it is
silently missing otherwise, -z force-bti at least warns if an input
is missing the property, but that's not a fatal error.
(Using --fatal-warnings ld flag does not work in the test system.)
Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
configure | 14 +++++++++++++-
configure.ac | 6 ++++++
sysdeps/aarch64/Makefile | 4 ++++
sysdeps/aarch64/configure | 31 +++++++++++++++++++++++++++++++
sysdeps/aarch64/configure.ac | 19 +++++++++++++++++++
5 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 8df47d61f8..fff5734f6d 100755
--- a/configure
+++ b/configure
@@ -794,6 +794,7 @@ enable_pt_chown
enable_tunables
enable_mathvec
enable_cet
+enable_standard_branch_protection
with_cpu
'
ac_precious_vars='build_alias
@@ -1471,6 +1472,9 @@ Optional Features:
depends on architecture]
--enable-cet enable Intel Control-flow Enforcement Technology
(CET), x86 only
+ --enable-standard-branch-protection
+ enable AArch64 Branch Target Identification and
+ Return Address Signing, AArch64 only
Optional Packages:
--with-PACKAGE[=ARG] use PACKAGE [ARG=yes]
@@ -3785,7 +3789,7 @@ main ()
{
#ifndef __CET__
-#error no CET compiler support
+# error no CET compiler support
#endif
;
return 0;
@@ -3806,6 +3810,14 @@ else
fi
+# Check whether --enable-standard-branch-protection was given.
+if test "${enable_standard_branch_protection+set}" = set; then :
+ enableval=$enable_standard_branch_protection; libc_cv_branch_protection=$enableval
+else
+ libc_cv_branch_protection=no
+fi
+
+
# We keep the original values in `$config_*' and never modify them, so we
# can write them unchanged into config.make. Everything else uses
# $machine, $vendor, and $os, and changes them whenever convenient.
diff --git a/configure.ac b/configure.ac
index 5f229679a9..e08b0f3766 100644
--- a/configure.ac
+++ b/configure.ac
@@ -486,6 +486,12 @@ AC_ARG_ENABLE([cet],
[enable_cet=$enableval],
[enable_cet=$libc_cv_compiler_default_cet])
+AC_ARG_ENABLE([standard-branch-protection],
+ AC_HELP_STRING([--enable-standard-branch-protection],
+ [enable AArch64 Branch Target Identification and Return Address Signing, AArch64 only]),
+ [libc_cv_branch_protection=$enableval],
+ [libc_cv_branch_protection=no])
+
# We keep the original values in `$config_*' and never modify them, so we
# can write them unchanged into config.make. Everything else uses
# $machine, $vendor, and $os, and changes them whenever convenient.
diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
index 5ae8b082b0..313c371e72 100644
--- a/sysdeps/aarch64/Makefile
+++ b/sysdeps/aarch64/Makefile
@@ -1,5 +1,9 @@
long-double-fcts = yes
+ifeq (yes,$(enable-branch-protection))
+sysdep-LDFLAGS += -Wl,-z,force-bti
+endif
+
ifeq ($(subdir),elf)
sysdep-dl-routines += dl-bti
endif
diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
index 5bd355a691..83a6c8c852 100644
--- a/sysdeps/aarch64/configure
+++ b/sysdeps/aarch64/configure
@@ -172,3 +172,34 @@ else
config_vars="$config_vars
default-abi = lp64"
fi
+
+if test "$libc_cv_branch_protection" = yes; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for branch protection support" >&5
+$as_echo_n "checking for branch protection support... " >&6; }
+if ${libc_cv_branch_protection_support+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat > conftest.c <<EOF
+void foo (void) { }
+EOF
+ libc_cv_branch_protection_support=no
+ if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -fPIC -shared -Wl,-z,force-bti,--fatal-warnings -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; }; } \
+ && LC_ALL=C $READELF -Wn conftest.so | \
+ grep -q 'NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI'; then
+ libc_cv_branch_protection_support=yes
+ fi
+ rm -rf conftest.*
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_branch_protection_support" >&5
+$as_echo "$libc_cv_branch_protection_support" >&6; }
+ if test $libc_cv_branch_protection_support = no; then
+ as_fn_error $? "branch-protection is enabled, but the toolchain does not support it." "$LINENO" 5
+ fi
+fi
+config_vars="$config_vars
+enable-branch-protection = $libc_cv_branch_protection"
diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
index 7851dd4dac..d16ba3710a 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
+
+if test "$libc_cv_branch_protection" = yes; then
+ AC_CACHE_CHECK([for branch protection support],
+ [libc_cv_branch_protection_support],
+ [cat > conftest.c <<EOF
+void foo (void) { }
+EOF
+ libc_cv_branch_protection_support=no
+ if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -fPIC -shared -Wl,-z,force-bti,--fatal-warnings -o conftest.so conftest.c]) \
+ && LC_ALL=C $READELF -Wn conftest.so | \
+ grep -q 'NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI'; then
+ libc_cv_branch_protection_support=yes
+ fi
+ rm -rf conftest.*])
+ if test $libc_cv_branch_protection_support = no; then
+ AC_MSG_ERROR([branch-protection is enabled, but the toolchain does not support it.])
+ fi
+fi
+LIBC_CONFIG_VAR([enable-branch-protection], [$libc_cv_branch_protection])
--
2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 12/12] aarch64: Configure option to build glibc with branch protection
2020-04-30 17:45 ` [PATCH 12/12] aarch64: Configure option to build glibc with branch protection Szabolcs Nagy
@ 2020-04-30 19:02 ` Joseph Myers
2020-05-08 17:53 ` Adhemerval Zanella
1 sibling, 0 replies; 48+ messages in thread
From: Joseph Myers @ 2020-04-30 19:02 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, Sudakshina Das
New configure options should be documented in install.texi, and the
INSTALL file regenerated.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 08/12] Rewrite abi-note.S in C.
2020-04-30 17:43 ` [PATCH 08/12] Rewrite abi-note.S in C Szabolcs Nagy
@ 2020-04-30 20:07 ` Zack Weinberg
2020-05-01 9:23 ` Szabolcs Nagy
0 siblings, 1 reply; 48+ messages in thread
From: Zack Weinberg @ 2020-04-30 20:07 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: GNU C Library, Sudakshina Das
On Thu, Apr 30, 2020 at 1:43 PM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> Using C code with __asm() allows the compiler to add target
> specific object file markings based on CFLAGS.
> This is e.g. needed for building glibc with branch-protection
> on AArch64.
Hmm. If we're going to do this, should we maybe go further and use
actual C? This snippet produces a matching .note.ABI-tag section for
me:
#include <stdint.h>
#include <config.h>
#include <endian.h>
#include <abi-tag.h>
const int32_t
__attribute__((section(".note.ABI-tag")))
__abi_tag[] = {
4, /* name length: 4 */
16, /* data length: 4 32-bit numbers */
1,
#if BYTE_ORDER == BIG_ENDIAN
0x474E5500, /* "GNU\0" */
#else
0x00554E47, /* same, little-endian */
#endif
__ABI_TAG_OS,
__ABI_TAG_VERSION
};
I don't see a problem with hardwiring a name length of 4, and code
elsewhere appears to assume that the payload will always be 4 32-bit
integers, so maybe it's OK to hardwire the 16 too. If it's not OK,
perhaps we could figure out a way to count the commas in
__ABI_TAG_VERSION in the preprocessor and do a little math.
The other potential problem I can think of is that there's a symbol
pointing into the .note.ABI-tag section now; I don't know if that
might break anything.
zw
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 08/12] Rewrite abi-note.S in C.
2020-04-30 20:07 ` Zack Weinberg
@ 2020-05-01 9:23 ` Szabolcs Nagy
2020-05-01 14:07 ` Zack Weinberg
0 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-01 9:23 UTC (permalink / raw)
To: Zack Weinberg; +Cc: GNU C Library, Sudakshina Das
The 04/30/2020 16:07, Zack Weinberg wrote:
> On Thu, Apr 30, 2020 at 1:43 PM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > Using C code with __asm() allows the compiler to add target
> > specific object file markings based on CFLAGS.
> > This is e.g. needed for building glibc with branch-protection
> > on AArch64.
>
> Hmm. If we're going to do this, should we maybe go further and use
> actual C? This snippet produces a matching .note.ABI-tag section for
> me:
yes this makes sense.
> #include <stdint.h>
> #include <config.h>
> #include <endian.h>
> #include <abi-tag.h>
> const int32_t
> __attribute__((section(".note.ABI-tag")))
> __abi_tag[] = {
> 4, /* name length: 4 */
> 16, /* data length: 4 32-bit numbers */
> 1,
> #if BYTE_ORDER == BIG_ENDIAN
> 0x474E5500, /* "GNU\0" */
> #else
> 0x00554E47, /* same, little-endian */
> #endif
> __ABI_TAG_OS,
> __ABI_TAG_VERSION
> };
>
> I don't see a problem with hardwiring a name length of 4, and code
> elsewhere appears to assume that the payload will always be 4 32-bit
> integers, so maybe it's OK to hardwire the 16 too. If it's not OK,
> perhaps we could figure out a way to count the commas in
> __ABI_TAG_VERSION in the preprocessor and do a little math.
>
> The other potential problem I can think of is that there's a symbol
> pointing into the .note.ABI-tag section now; I don't know if that
> might break anything.
what about
/* 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, /* name length: 4 */
16, /* data length: 4 32-bit numbers */
1,
"GNU",
{ __ABI_TAG_OS, __ABI_TAG_VERSION }
};
this fixes the alignment, makes the symbol local, does not need
endian.h and uses identifiers according to the gabi specification.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 08/12] Rewrite abi-note.S in C.
2020-05-01 9:23 ` Szabolcs Nagy
@ 2020-05-01 14:07 ` Zack Weinberg
0 siblings, 0 replies; 48+ messages in thread
From: Zack Weinberg @ 2020-05-01 14:07 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: GNU C Library, Sudakshina Das
On Fri, May 1, 2020 at 5:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> what about
>
> /* 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, /* name length: 4 */
> 16, /* data length: 4 32-bit numbers */
> 1,
> "GNU",
> { __ABI_TAG_OS, __ABI_TAG_VERSION }
> };
>
> this fixes the alignment, makes the symbol local, does not need
> endian.h and uses identifiers according to the gabi specification.
Looks good to me. Fixing the array length means the compiler should
complain if __ABI_TAG_VERSION ever expands to the wrong number of
initializers.
zw
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 00/12] aarch64: branch protection support
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
` (11 preceding siblings ...)
2020-04-30 17:45 ` [PATCH 12/12] aarch64: Configure option to build glibc with branch protection Szabolcs Nagy
@ 2020-05-04 11:27 ` Szabolcs Nagy
12 siblings, 0 replies; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-04 11:27 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
The 04/30/2020 18:34, Szabolcs Nagy wrote:
> FAIL: misc/tst-atomic
on a closer look this seems to be a glibc test bug:
529 mem = 14;
530 expected = 14;
531 if (!atomic_compare_exchange_weak_relaxed (&mem, &expected, 25)
532 || mem != 25 || expected != 14)
533 {
534 puts ("atomic_compare_exchange_weak_relaxed test 1 failed");
535 ret = 1;
536 }
compare exchange may fail intermittently e.g.
because of scheduling events, so the caller
should retry. i opened
https://sourceware.org/bugzilla/show_bug.cgi?id=25919
> FAIL: nptl/tst-cancel7
> FAIL: nptl/tst-cancelx7
> not reproducible issues
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 01/12] elf.h: Add PT_GNU_PROPERTY
2020-04-30 17:37 ` [PATCH 01/12] elf.h: Add PT_GNU_PROPERTY Szabolcs Nagy
@ 2020-05-07 14:49 ` Adhemerval Zanella
0 siblings, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 14:49 UTC (permalink / raw)
To: libc-alpha
On 30/04/2020 14:37, Szabolcs Nagy wrote:
> This program header type is already used in binaries on x86 and
> aarch64 targets.
LGTM, its value matches binutils (0a59decbb81676ac30deede1bb6b6e241cd75502).
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 */
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 02/12] elf.h: add aarch64 property definitions
2020-04-30 17:37 ` [PATCH 02/12] elf.h: add aarch64 property definitions Szabolcs Nagy
@ 2020-05-07 14:50 ` Adhemerval Zanella
0 siblings, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 14:50 UTC (permalink / raw)
To: libc-alpha
On 30/04/2020 14:37, Szabolcs Nagy wrote:
> These property values are specified by the AArch64 ELF ABI and
> binutils can create binaries marked with them.
LGTM, they match binutils (cd702818c6c).
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
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 03/12] aarch64: Add BTI landing pads to assembly code
2020-04-30 17:39 ` [PATCH 03/12] aarch64: Add BTI landing pads to assembly code Szabolcs Nagy
@ 2020-05-07 16:55 ` Adhemerval Zanella
2020-05-11 11:38 ` Szabolcs Nagy
0 siblings, 1 reply; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 16:55 UTC (permalink / raw)
To: libc-alpha
On 30/04/2020 14:39, Szabolcs Nagy wrote:
> From 550fe66ed93e13c0f063955e81bfcb8db386413c Mon Sep 17 00:00:00 2001
> From: Sudakshina Das <sudi.das@arm.com>
> Date: Tue, 17 Mar 2020 15:44:18 +0000
> Subject: [PATCH 03/12] aarch64: Add BTI landing pads to assembly code
>
> Adding the landing pads and the ELF markings are required if
> glibc is built with branch protection. For the handful of asm
> files this is done unconditionally, this simplifies maintenance
> and avoids complications where code layout is carefully aligned
> such that conditionally turning BTI off may cause performance
> regression (e.g. string functions).
I am seeing the warning the cover letter warned on old binutils:
unsupported GNU_PROPERTY_TYPE (5) type: 0xc0000000
In this case, how exactly binutils handles it? Does it still emit
the expected correct notes, and if so, is the expected layout?
In any case I think these warning might be misleading and I think
it would be better to conditionalize the ELF landing pad marks
with binutils version (HAVE_XXX plus a configure check).
>
> Note: old binutils ld just merges notes of input objects into
> the output, so if any input has a note claiming BTI support,
> then the output will have such note too which is undesirable.
> So after this commit libc shared objects and binaries linked
> with libc crt code may contain incorrect notes. For this reason
> checking properties in PT_NOTE is not reliable, new linkers
> create PT_GNU_PROPERTY which is always reliable when present
> so on AArch64 only that should be checked for properties.
Also on cover letter it states that PT_NOTES handling changes
is not yet done. Would it trigger any possible runtime failure
in possible binutils / glibc version combination?
>
> Note: functions using ENTRY or ENTRY_ALIGN now have an
> additional BTI c after the function label so alignment of
> the code changes, but ENTRY_ALIGN_AND_PAD was fixed so there
> is no change to the existing code layout.
>
> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> ---
> sysdeps/aarch64/__longjmp.S | 1 +
> sysdeps/aarch64/crti.S | 4 +++
> sysdeps/aarch64/crtn.S | 4 +++
> sysdeps/aarch64/dl-tlsdesc.S | 5 ++++
> sysdeps/aarch64/dl-trampoline.S | 4 +++
> sysdeps/aarch64/memchr.S | 1 +
> sysdeps/aarch64/memcmp.S | 1 +
> sysdeps/aarch64/memcpy.S | 1 +
> sysdeps/aarch64/memrchr.S | 1 +
> sysdeps/aarch64/memset.S | 1 +
> sysdeps/aarch64/multiarch/memchr_nosimd.S | 1 +
> sysdeps/aarch64/multiarch/memcpy_falkor.S | 1 +
> sysdeps/aarch64/multiarch/memcpy_thunderx.S | 1 +
> sysdeps/aarch64/multiarch/memcpy_thunderx2.S | 1 +
> sysdeps/aarch64/multiarch/memmove_falkor.S | 1 +
> sysdeps/aarch64/multiarch/memset_base64.S | 1 +
> sysdeps/aarch64/multiarch/memset_kunpeng.S | 1 +
> sysdeps/aarch64/multiarch/strlen_asimd.S | 1 +
> sysdeps/aarch64/rawmemchr.S | 1 +
> sysdeps/aarch64/setjmp.S | 1 +
> sysdeps/aarch64/start.S | 2 ++
> sysdeps/aarch64/strchr.S | 1 +
> sysdeps/aarch64/strchrnul.S | 1 +
> sysdeps/aarch64/strcmp.S | 1 +
> sysdeps/aarch64/strcpy.S | 1 +
> sysdeps/aarch64/strlen.S | 1 +
> sysdeps/aarch64/strncmp.S | 1 +
> sysdeps/aarch64/strnlen.S | 1 +
> sysdeps/aarch64/strrchr.S | 1 +
> sysdeps/aarch64/sysdep.h | 27 ++++++++++++++++++-
> sysdeps/unix/sysv/linux/aarch64/__read_tp.S | 1 +
> sysdeps/unix/sysv/linux/aarch64/clone.S | 1 +
> sysdeps/unix/sysv/linux/aarch64/getcontext.S | 1 +
> sysdeps/unix/sysv/linux/aarch64/ioctl.S | 1 +
> .../unix/sysv/linux/aarch64/libc-__read_tp.S | 1 +
> sysdeps/unix/sysv/linux/aarch64/setcontext.S | 1 +
> sysdeps/unix/sysv/linux/aarch64/syscall.S | 1 +
> sysdeps/unix/sysv/linux/aarch64/vfork.S | 1 +
> 38 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/aarch64/__longjmp.S b/sysdeps/aarch64/__longjmp.S
> index f9060776b4..362171cdb8 100644
> --- a/sysdeps/aarch64/__longjmp.S
> +++ b/sysdeps/aarch64/__longjmp.S
> @@ -116,3 +116,4 @@ ENTRY (__longjmp)
> /* Use br instead of ret because ret is guaranteed to mispredict */
> br x30
> END (__longjmp)
> +END_FILE
> diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
> index 1728eac37a..89a9e25f5b 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,8 @@ _init:
> .hidden _fini
> .type _fini, %function
> _fini:
> + BTI_C
> stp x29, x30, [sp, -16]!
> mov x29, sp
> +
> +END_FILE
> diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
> index c3e97cc449..94a6f970ef 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
> @@ -44,3 +46,5 @@
> .section .fini,"ax",%progbits
> ldp x29, x30, [sp], 16
> RET
> +
> +END_FILE
> diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
> index 557ad1d505..d55e0443aa 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
> @@ -235,3 +238,5 @@ _dl_tlsdesc_dynamic:
> .size _dl_tlsdesc_dynamic, .-_dl_tlsdesc_dynamic
> # undef NSAVEXREGPAIRS
> #endif
> +
> +END_FILE
> diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
> index 94e965c096..fba5689d09 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)
> @@ -298,3 +300,5 @@ _dl_runtime_profile:
> .size _dl_runtime_profile, .-_dl_runtime_profile
> #endif
> .previous
> +
> +END_FILE
> diff --git a/sysdeps/aarch64/memchr.S b/sysdeps/aarch64/memchr.S
> index 85c65cbfca..c67a31223f 100644
> --- a/sysdeps/aarch64/memchr.S
> +++ b/sysdeps/aarch64/memchr.S
> @@ -159,3 +159,4 @@ L(zero_length):
> END (MEMCHR)
> weak_alias (MEMCHR, memchr)
> libc_hidden_builtin_def (memchr)
> +END_FILE
> diff --git a/sysdeps/aarch64/memcmp.S b/sysdeps/aarch64/memcmp.S
> index 827f54f99e..c6e07f9287 100644
> --- a/sysdeps/aarch64/memcmp.S
> +++ b/sysdeps/aarch64/memcmp.S
> @@ -178,3 +178,4 @@ END (memcmp)
> #undef bcmp
> weak_alias (memcmp, bcmp)
> libc_hidden_builtin_def (memcmp)
> +END_FILE
> diff --git a/sysdeps/aarch64/memcpy.S b/sysdeps/aarch64/memcpy.S
> index e0b4c4502f..543d9417f3 100644
> --- a/sysdeps/aarch64/memcpy.S
> +++ b/sysdeps/aarch64/memcpy.S
> @@ -282,3 +282,4 @@ L(copy64_from_start):
>
> END (MEMMOVE)
> libc_hidden_builtin_def (MEMMOVE)
> +END_FILE
> diff --git a/sysdeps/aarch64/memrchr.S b/sysdeps/aarch64/memrchr.S
> index ace5a94e8f..f35a68d14d 100644
> --- a/sysdeps/aarch64/memrchr.S
> +++ b/sysdeps/aarch64/memrchr.S
> @@ -163,3 +163,4 @@ L(zero_length):
> END (__memrchr)
> weak_alias (__memrchr, memrchr)
> libc_hidden_builtin_def (memrchr)
> +END_FILE
> diff --git a/sysdeps/aarch64/memset.S b/sysdeps/aarch64/memset.S
> index ac577f1660..7cdae20563 100644
> --- a/sysdeps/aarch64/memset.S
> +++ b/sysdeps/aarch64/memset.S
> @@ -189,3 +189,4 @@ L(zva_other):
>
> END (MEMSET)
> libc_hidden_builtin_def (MEMSET)
> +END_FILE
> diff --git a/sysdeps/aarch64/multiarch/memchr_nosimd.S b/sysdeps/aarch64/multiarch/memchr_nosimd.S
> index 41ce10eb32..6d7d38d5bb 100644
> --- a/sysdeps/aarch64/multiarch/memchr_nosimd.S
> +++ b/sysdeps/aarch64/multiarch/memchr_nosimd.S
> @@ -221,3 +221,4 @@ L(none_chr):
>
> END (MEMCHR)
> libc_hidden_builtin_def (MEMCHR)
> +END_FILE
> diff --git a/sysdeps/aarch64/multiarch/memcpy_falkor.S b/sysdeps/aarch64/multiarch/memcpy_falkor.S
> index 35a1fae1b9..999aa48b16 100644
> --- a/sysdeps/aarch64/multiarch/memcpy_falkor.S
> +++ b/sysdeps/aarch64/multiarch/memcpy_falkor.S
> @@ -188,4 +188,5 @@ L(last64):
>
> END (__memcpy_falkor)
> libc_hidden_builtin_def (__memcpy_falkor)
> +END_FILE
> #endif
> diff --git a/sysdeps/aarch64/multiarch/memcpy_thunderx.S b/sysdeps/aarch64/multiarch/memcpy_thunderx.S
> index e9407571b5..e6e36a6633 100644
> --- a/sysdeps/aarch64/multiarch/memcpy_thunderx.S
> +++ b/sysdeps/aarch64/multiarch/memcpy_thunderx.S
> @@ -318,5 +318,6 @@ L(move_long):
>
> END (MEMCPY)
> libc_hidden_builtin_def (MEMCPY)
> +END_FILE
>
> #endif
> diff --git a/sysdeps/aarch64/multiarch/memcpy_thunderx2.S b/sysdeps/aarch64/multiarch/memcpy_thunderx2.S
> index 68e99455c8..fde4c7198c 100644
> --- a/sysdeps/aarch64/multiarch/memcpy_thunderx2.S
> +++ b/sysdeps/aarch64/multiarch/memcpy_thunderx2.S
> @@ -474,4 +474,5 @@ L(ext_table):
> .word L(ext_size_15) -.
>
> libc_hidden_builtin_def (MEMCPY)
> +END_FILE
> #endif
> diff --git a/sysdeps/aarch64/multiarch/memmove_falkor.S b/sysdeps/aarch64/multiarch/memmove_falkor.S
> index 35fc1fdd41..d8cc992d27 100644
> --- a/sysdeps/aarch64/multiarch/memmove_falkor.S
> +++ b/sysdeps/aarch64/multiarch/memmove_falkor.S
> @@ -223,3 +223,4 @@ L(move_long):
>
> END (__memmove_falkor)
> libc_hidden_builtin_def (__memmove_falkor)
> +END_FILE
> diff --git a/sysdeps/aarch64/multiarch/memset_base64.S b/sysdeps/aarch64/multiarch/memset_base64.S
> index 8f85cd1caf..ee0b832ef9 100644
> --- a/sysdeps/aarch64/multiarch/memset_base64.S
> +++ b/sysdeps/aarch64/multiarch/memset_base64.S
> @@ -184,3 +184,4 @@ L(zva_64):
>
> END (MEMSET)
> libc_hidden_builtin_def (MEMSET)
> +END_FILE
> diff --git a/sysdeps/aarch64/multiarch/memset_kunpeng.S b/sysdeps/aarch64/multiarch/memset_kunpeng.S
> index 8e051d4fd1..aa7ab62fdd 100644
> --- a/sysdeps/aarch64/multiarch/memset_kunpeng.S
> +++ b/sysdeps/aarch64/multiarch/memset_kunpeng.S
> @@ -111,3 +111,4 @@ L(set_long):
> END (MEMSET)
> libc_hidden_builtin_def (MEMSET)
> #endif
> +END_FILE
> diff --git a/sysdeps/aarch64/multiarch/strlen_asimd.S b/sysdeps/aarch64/multiarch/strlen_asimd.S
> index 236a2c96a6..c28aa0ca8d 100644
> --- a/sysdeps/aarch64/multiarch/strlen_asimd.S
> +++ b/sysdeps/aarch64/multiarch/strlen_asimd.S
> @@ -176,3 +176,4 @@ L(page_cross):
> END (__strlen_asimd)
> weak_alias (__strlen_asimd, strlen_asimd)
> libc_hidden_builtin_def (strlen_asimd)
> +END_FILE
> diff --git a/sysdeps/aarch64/rawmemchr.S b/sysdeps/aarch64/rawmemchr.S
> index 5c7a664fb4..4ad614a169 100644
> --- a/sysdeps/aarch64/rawmemchr.S
> +++ b/sysdeps/aarch64/rawmemchr.S
> @@ -40,3 +40,4 @@ L(do_strlen):
> END (__rawmemchr)
> weak_alias (__rawmemchr, rawmemchr)
> libc_hidden_builtin_def (__rawmemchr)
> +END_FILE
> diff --git a/sysdeps/aarch64/setjmp.S b/sysdeps/aarch64/setjmp.S
> index 28fdd3f46a..9a0201ef70 100644
> --- a/sysdeps/aarch64/setjmp.S
> +++ b/sysdeps/aarch64/setjmp.S
> @@ -73,3 +73,4 @@ ENTRY (__sigsetjmp)
> #endif
> END (__sigsetjmp)
> hidden_def (__sigsetjmp)
> +END_FILE
> diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
> index d96cf57e2d..e6c0393c20 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
> @@ -110,3 +111,4 @@ __data_start:
> .long 0
> .weak data_start
> data_start = __data_start
> +END_FILE
> diff --git a/sysdeps/aarch64/strchr.S b/sysdeps/aarch64/strchr.S
> index 4a75e73945..e1f98aa42c 100644
> --- a/sysdeps/aarch64/strchr.S
> +++ b/sysdeps/aarch64/strchr.S
> @@ -137,3 +137,4 @@ L(tail):
> END (strchr)
> libc_hidden_builtin_def (strchr)
> weak_alias (strchr, index)
> +END_FILE
> diff --git a/sysdeps/aarch64/strchrnul.S b/sysdeps/aarch64/strchrnul.S
> index a65be6cba8..a9ccc54205 100644
> --- a/sysdeps/aarch64/strchrnul.S
> +++ b/sysdeps/aarch64/strchrnul.S
> @@ -129,3 +129,4 @@ L(tail):
>
> END(__strchrnul)
> weak_alias (__strchrnul, strchrnul)
> +END_FILE
> diff --git a/sysdeps/aarch64/strcmp.S b/sysdeps/aarch64/strcmp.S
> index d044c29e9b..a7bee697c0 100644
> --- a/sysdeps/aarch64/strcmp.S
> +++ b/sysdeps/aarch64/strcmp.S
> @@ -182,3 +182,4 @@ L(done):
> RET
> END(strcmp)
> libc_hidden_builtin_def (strcmp)
> +END_FILE
> diff --git a/sysdeps/aarch64/strcpy.S b/sysdeps/aarch64/strcpy.S
> index 548130e413..631da4a358 100644
> --- a/sysdeps/aarch64/strcpy.S
> +++ b/sysdeps/aarch64/strcpy.S
> @@ -321,3 +321,4 @@ libc_hidden_builtin_def (stpcpy)
> #else
> libc_hidden_builtin_def (strcpy)
> #endif
> +END_FILE
> diff --git a/sysdeps/aarch64/strlen.S b/sysdeps/aarch64/strlen.S
> index e01fab7c2a..3af25de4b8 100644
> --- a/sysdeps/aarch64/strlen.S
> +++ b/sysdeps/aarch64/strlen.S
> @@ -222,3 +222,4 @@ L(page_cross):
> END (STRLEN)
> weak_alias (STRLEN, strlen)
> libc_hidden_builtin_def (strlen)
> +END_FILE
> diff --git a/sysdeps/aarch64/strncmp.S b/sysdeps/aarch64/strncmp.S
> index c5141fab8a..d289d4a3ba 100644
> --- a/sysdeps/aarch64/strncmp.S
> +++ b/sysdeps/aarch64/strncmp.S
> @@ -270,3 +270,4 @@ L(ret0):
>
> END (strncmp)
> libc_hidden_builtin_def (strncmp)
> +END_FILE
> diff --git a/sysdeps/aarch64/strnlen.S b/sysdeps/aarch64/strnlen.S
> index 5981247dd9..964536dba9 100644
> --- a/sysdeps/aarch64/strnlen.S
> +++ b/sysdeps/aarch64/strnlen.S
> @@ -213,3 +213,4 @@ END (__strnlen)
> libc_hidden_def (__strnlen)
> weak_alias (__strnlen, strnlen)
> libc_hidden_def (strnlen)
> +END_FILE
> diff --git a/sysdeps/aarch64/strrchr.S b/sysdeps/aarch64/strrchr.S
> index 94da08d351..9f6d956f21 100644
> --- a/sysdeps/aarch64/strrchr.S
> +++ b/sysdeps/aarch64/strrchr.S
> @@ -164,3 +164,4 @@ L(null_search):
> END(strrchr)
> weak_alias (strrchr, rindex)
> libc_hidden_builtin_def (strrchr)
> +END_FILE
> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 604c489170..07dc7858a5 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -41,12 +41,35 @@
>
> #define ASM_SIZE_DIRECTIVE(name) .size name,.-name
>
> +/* Branch Target Identitication support. */
> +#define BTI_C hint 34
> +#define BTI_J hint 36
> +
> +#define FEATURE_1_BTI 1
> +#define FEATURE_1_PAC 2
There are GNU_PROPERTY_AARCH64_FEATURE_1_BTI and GNU_PROPERTY_AARCH64_FEATURE_1_PAC,
why not use them?
> +
> +/* Add a GNU_PROPERTY_AARCH64_FEATURE_1_AND note. */
> +#define GNU_PROPERTY(features) \
> + .section .note.gnu.property, "a"; \
> + .p2align 3; \
> + .word 4; \
> + .word 16; \
> + .word 5; \
> + .asciz "GNU"; \
> + .word 0xc0000000; \
> + .word 4; \
> + .word features; \
> + .word 0;
> +
> +#define END_FILE GNU_PROPERTY(FEATURE_1_BTI)
END_FILE name does not really give much information. Would be better
to use ADD_GNU_PROPERTIES or something?
> +
> /* 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 +79,7 @@
> .type C_SYMBOL_NAME(name),%function; \
> .p2align align; \
> C_LABEL(name) \
> + BTI_C; \
> cfi_startproc; \
> CALL_MCOUNT
>
> @@ -68,10 +92,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
>
> diff --git a/sysdeps/unix/sysv/linux/aarch64/__read_tp.S b/sysdeps/unix/sysv/linux/aarch64/__read_tp.S
> index 12e1131fe7..7825aa5e1b 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/__read_tp.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/__read_tp.S
> @@ -23,3 +23,4 @@ ENTRY (__read_tp)
> mrs x0, tpidr_el0
> RET
> END (__read_tp)
> +END_FILE
> diff --git a/sysdeps/unix/sysv/linux/aarch64/clone.S b/sysdeps/unix/sysv/linux/aarch64/clone.S
> index 2b14106fd1..15b5a7b217 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/clone.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/clone.S
> @@ -85,3 +85,4 @@ thread_start:
>
> libc_hidden_def (__clone)
> weak_alias (__clone, clone)
> +END_FILE
> diff --git a/sysdeps/unix/sysv/linux/aarch64/getcontext.S b/sysdeps/unix/sysv/linux/aarch64/getcontext.S
> index 8571556189..95f61d5f28 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/getcontext.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/getcontext.S
> @@ -107,3 +107,4 @@ ENTRY(__getcontext)
>
> PSEUDO_END (__getcontext)
> weak_alias (__getcontext, getcontext)
> +END_FILE
> diff --git a/sysdeps/unix/sysv/linux/aarch64/ioctl.S b/sysdeps/unix/sysv/linux/aarch64/ioctl.S
> index ed36e309c4..19abf43726 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/ioctl.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/ioctl.S
> @@ -30,3 +30,4 @@ PSEUDO_END (__ioctl)
>
> libc_hidden_def (__ioctl)
> weak_alias (__ioctl, ioctl)
> +END_FILE
> diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-__read_tp.S b/sysdeps/unix/sysv/linux/aarch64/libc-__read_tp.S
> index 6de259ed83..df5eec8599 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/libc-__read_tp.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/libc-__read_tp.S
> @@ -17,3 +17,4 @@
> <https://www.gnu.org/licenses/>. */
>
> #include <__read_tp.S>
> +END_FILE
> diff --git a/sysdeps/unix/sysv/linux/aarch64/setcontext.S b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
> index 61fb813db3..d9d6c0f364 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/setcontext.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
> @@ -128,3 +128,4 @@ ENTRY (__startcontext)
> cbnz x0, __setcontext
> 1: b HIDDEN_JUMPTARGET (exit)
> END (__startcontext)
> +END_FILE
> diff --git a/sysdeps/unix/sysv/linux/aarch64/syscall.S b/sysdeps/unix/sysv/linux/aarch64/syscall.S
> index 993e307fcf..10e59ece58 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/syscall.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/syscall.S
> @@ -42,3 +42,4 @@ ENTRY (syscall)
> 1:
> b SYSCALL_ERROR
> PSEUDO_END (syscall)
> +END_FILE
> diff --git a/sysdeps/unix/sysv/linux/aarch64/vfork.S b/sysdeps/unix/sysv/linux/aarch64/vfork.S
> index 9c8ea48c5a..0306003a22 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/vfork.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/vfork.S
> @@ -40,3 +40,4 @@ libc_hidden_def (__vfork)
>
> weak_alias (__vfork, vfork)
> strong_alias (__vfork, __libc_vfork)
> +END_FILE
> --
> 2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 04/12] aarch64: Rename place holder .S files to .c
2020-04-30 17:40 ` [PATCH 04/12] aarch64: Rename place holder .S files to .c Szabolcs Nagy
@ 2020-05-07 18:29 ` Adhemerval Zanella
0 siblings, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 18:29 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: Sudakshina Das
On 30/04/2020 14:40, Szabolcs Nagy wrote:
> From 2084e33755ee4d00b3ecf956b1705d8eea46ebc7 Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Date: Wed, 1 Apr 2020 09:52:37 +0100
> Subject: [PATCH 04/12] aarch64: Rename place holder .S files to .c
>
> 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.
LGTM, thanks.
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] 48+ messages in thread
* Re: [PATCH 05/12] aarch64: fix swapcontext for BTI
2020-04-30 17:41 ` [PATCH 05/12] aarch64: fix swapcontext for BTI Szabolcs Nagy
@ 2020-05-07 18:42 ` Adhemerval Zanella
0 siblings, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 18:42 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: Sudakshina Das
On 30/04/2020 14:41, Szabolcs Nagy wrote:
> From c156ff9d5e332ad17d7fa74b777e8f5466aff0a4 Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Date: Wed, 1 Apr 2020 10:31:41 +0100
> Subject: [PATCH 05/12] aarch64: fix swapcontext for BTI
>
> 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.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/unix/sysv/linux/aarch64/swapcontext.S | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/aarch64/swapcontext.S b/sysdeps/unix/sysv/linux/aarch64/swapcontext.S
> index d30c543e6f..b60cf04c92 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,12 @@ 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)
> +END_FILE
> --
> 2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 06/12] aarch64: fix RTLD_START for BTI
2020-04-30 17:42 ` [PATCH 06/12] aarch64: fix RTLD_START " Szabolcs Nagy
@ 2020-05-07 18:49 ` Adhemerval Zanella
2020-05-07 19:24 ` Szabolcs Nagy
0 siblings, 1 reply; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 18:49 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: Sudakshina Das
On 30/04/2020 14:42, Szabolcs Nagy wrote:
> From 1e8662264c07e69d807761882e8d77f0916ae562 Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Date: Tue, 31 Mar 2020 17:32:14 +0100
> Subject: [PATCH 06/12] aarch64: fix RTLD_START for BTI
>
> 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.
LGTM, thanks.
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\
This is the BTI_C defined at sysdeps/aarch64/sysdep.h, why can't you use
it here?
> 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 [flat|nested] 48+ messages in thread
* Re: [PATCH 06/12] aarch64: fix RTLD_START for BTI
2020-05-07 18:49 ` Adhemerval Zanella
@ 2020-05-07 19:24 ` Szabolcs Nagy
2020-05-07 19:55 ` Adhemerval Zanella
0 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-07 19:24 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Sudakshina Das
The 05/07/2020 15:49, Adhemerval Zanella wrote:
> On 30/04/2020 14:42, Szabolcs Nagy wrote:
> > From 1e8662264c07e69d807761882e8d77f0916ae562 Mon Sep 17 00:00:00 2001
> > From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> > Date: Tue, 31 Mar 2020 17:32:14 +0100
> > Subject: [PATCH 06/12] aarch64: fix RTLD_START for BTI
> >
> > 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.
>
> LGTM, thanks.
>
> 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\
>
> This is the BTI_C defined at sysdeps/aarch64/sysdep.h, why can't you use
> it here?
BTI_C is only defined for asm files, but this is inline asm in c.
thanks for the reviews so far, i'm working on a v2 of the patchset
(changing configury bits and how all this is enabled) will post it
when the tests finish running.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 07/12] aarch64: fix syscalls for BTI
2020-04-30 17:42 ` [PATCH 07/12] aarch64: fix syscalls " Szabolcs Nagy
@ 2020-05-07 19:40 ` Adhemerval Zanella
2020-05-11 11:46 ` Szabolcs Nagy
0 siblings, 1 reply; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 19:40 UTC (permalink / raw)
To: libc-alpha
On 30/04/2020 14:42, Szabolcs Nagy wrote:
> Syscall asm code needs an ELF property marking for BTI
> when glibc is built with BTI support so we add AArch64
> variants of syscall-template.S and umount2.S.
> ---
> .../sysv/linux/aarch64/syscall-template.S | 20 +++++++++++++++
> sysdeps/unix/sysv/linux/aarch64/umount2.S | 25 +++++++++++++++++++
> 2 files changed, 45 insertions(+)
> create mode 100644 sysdeps/unix/sysv/linux/aarch64/syscall-template.S
> create mode 100644 sysdeps/unix/sysv/linux/aarch64/umount2.S
>
> diff --git a/sysdeps/unix/sysv/linux/aarch64/syscall-template.S b/sysdeps/unix/sysv/linux/aarch64/syscall-template.S
> new file mode 100644
> index 0000000000..50db585289
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/syscall-template.S
> @@ -0,0 +1,20 @@
> +/* Assembly code template for system call stubs.
> + 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/>. */
> +
> +#include <sysdeps/unix/syscall-template.S>
> +END_FILE
Although I am not very found of adding another arch-specific file,
I don't see better straightforward solution.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/umount2.S b/sysdeps/unix/sysv/linux/aarch64/umount2.S
> new file mode 100644
> index 0000000000..4fe26c35e2
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/umount2.S
> @@ -0,0 +1,25 @@
> +/* umount system call with two parameters.
> + 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/>. */
> +
> +#include <sysdep.h>
> +PSEUDO (__umount2, umount2, 2)
> + ret
> +PSEUDO_END(__umount2)
> +weak_alias (__umount2, umount2)
> +END_FILE
I think it is better to adapt the linux umount2.S to a C file instead of
adding another arch-specific implementation. Only alpha and ia64 do
not support __NR_umount2 (exported as __NR_umount), but recent kernel
fixes (74cd2184833f for ia64, 12b57c5c70f39 for alpha) add the alias.
So we can use __NR_umount without the need to check its definition:
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 0326f92c40..62ac921e8f 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -54,7 +54,7 @@ CFLAGS-malloc.c += -DMORECORE_CLEARS=2
endif
ifeq ($(subdir),misc)
-sysdep_routines += adjtimex clone umount umount2 readahead sysctl \
+sysdep_routines += adjtimex clone umount readahead sysctl \
setfsuid setfsgid epoll_pwait signalfd \
eventfd eventfd_read eventfd_write prlimit \
personality epoll_wait tee vmsplice splice \
diff --git a/sysdeps/unix/sysv/linux/ia64/syscalls.list b/sysdeps/unix/sysv/linux/ia64/syscalls.list
index 56f4138c43..bd39441d3c 100644
--- a/sysdeps/unix/sysv/linux/ia64/syscalls.list
+++ b/sysdeps/unix/sysv/linux/ia64/syscalls.list
@@ -1,7 +1,5 @@
# File name Caller Syscall name # args Strong name Weak names
-umount2 - umount 2 __umount2 umount2
-
getpriority - getpriority i:ii __getpriority getpriority
# proper socket implementations:
diff --git a/sysdeps/unix/sysv/linux/umount.c b/sysdeps/unix/sysv/linux/umount.c
index ab6a20f596..cdc8759118 100644
--- a/sysdeps/unix/sysv/linux/umount.c
+++ b/sysdeps/unix/sysv/linux/umount.c
@@ -16,12 +16,17 @@
License along with the GNU C Library. If not, see
<https://www.gnu.org/licenses/>. */
-/* Since the generic Linux syscall ABI doesn't have an oldumount system call,
- do what the kernel does down here. */
+#include <sys/mount.h>
+#include <sysdep.h>
-extern long int __umount2 (const char *name, int flags);
+int
+__umount2 (const char *special_file, int flags)
+{
+ return INLINE_SYSCALL_CALL (umount2, special_file, flags);
+}
+weak_alias (__umount2, umount2)
-long int
+int
__umount (const char *name)
{
return __umount2 (name, 0);
diff --git a/sysdeps/unix/sysv/linux/umount2.S b/sysdeps/unix/sysv/linux/umount2.S
deleted file mode 100644
index 92241bbf97..0000000000
--- a/sysdeps/unix/sysv/linux/umount2.S
+++ /dev/null
@@ -1,13 +0,0 @@
-/* umount system call with two parameters. */
-
-#include <sysdep.h>
-#if defined __NR_oldumount || defined __NR_umount2
-#ifdef __NR_oldumount
-PSEUDO (__umount2, umount, 2)
-#else
-PSEUDO (__umount2, umount2, 2)
-#endif
- ret
-PSEUDO_END(__umount2)
-weak_alias (__umount2, umount2)
-#endif
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 06/12] aarch64: fix RTLD_START for BTI
2020-05-07 19:24 ` Szabolcs Nagy
@ 2020-05-07 19:55 ` Adhemerval Zanella
2020-05-07 20:14 ` Szabolcs Nagy
0 siblings, 1 reply; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 19:55 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, Sudakshina Das
On 07/05/2020 16:24, Szabolcs Nagy wrote:
> The 05/07/2020 15:49, Adhemerval Zanella wrote:
>> On 30/04/2020 14:42, Szabolcs Nagy wrote:
>>> From 1e8662264c07e69d807761882e8d77f0916ae562 Mon Sep 17 00:00:00 2001
>>> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>> Date: Tue, 31 Mar 2020 17:32:14 +0100
>>> Subject: [PATCH 06/12] aarch64: fix RTLD_START for BTI
>>>
>>> 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.
>>
>> LGTM, thanks.
>>
>> 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\
>>
>> This is the BTI_C defined at sysdeps/aarch64/sysdep.h, why can't you use
>> it here?
>
> BTI_C is only defined for asm files, but this is inline asm in c.
Ack.
>
> thanks for the reviews so far, i'm working on a v2 of the patchset
> (changing configury bits and how all this is enabled) will post it
> when the tests finish running.
>
Could you send the new version as inline instead of attachment? It is
default of git send-email and slight easier to review with most email
clients.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 06/12] aarch64: fix RTLD_START for BTI
2020-05-07 19:55 ` Adhemerval Zanella
@ 2020-05-07 20:14 ` Szabolcs Nagy
2020-05-07 20:20 ` Adhemerval Zanella
0 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-07 20:14 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Sudakshina Das
The 05/07/2020 16:55, Adhemerval Zanella wrote:
> > thanks for the reviews so far, i'm working on a v2 of the patchset
> > (changing configury bits and how all this is enabled) will post it
> > when the tests finish running.
> >
>
> Could you send the new version as inline instead of attachment? It is
> default of git send-email and slight easier to review with most email
> clients.
ok.
i sent as attachments because on some patches
i'm not the author which is visible in an
attachment but not with inline patch..?
maybe i should make myself the 'author' and
add the original author as 'co-authored-by'?
not sure what's the best policy here.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 06/12] aarch64: fix RTLD_START for BTI
2020-05-07 20:14 ` Szabolcs Nagy
@ 2020-05-07 20:20 ` Adhemerval Zanella
0 siblings, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 20:20 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, Sudakshina Das
On 07/05/2020 17:14, Szabolcs Nagy wrote:
> The 05/07/2020 16:55, Adhemerval Zanella wrote:
>>> thanks for the reviews so far, i'm working on a v2 of the patchset
>>> (changing configury bits and how all this is enabled) will post it
>>> when the tests finish running.
>>>
>>
>> Could you send the new version as inline instead of attachment? It is
>> default of git send-email and slight easier to review with most email
>> clients.
>
> ok.
>
> i sent as attachments because on some patches
> i'm not the author which is visible in an
> attachment but not with inline patch..?
>
> maybe i should make myself the 'author' and
> add the original author as 'co-authored-by'?
>
> not sure what's the best policy here.
>
At least with git send-email, it does handle different author. For
instance https://sourceware.org/pipermail/libc-alpha/2020-April/112686.html
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 09/12] aarch64: support BTI enabled binaries
2020-04-30 17:44 ` [PATCH 09/12] aarch64: support BTI enabled binaries Szabolcs Nagy
@ 2020-05-07 21:07 ` Adhemerval Zanella
2020-05-11 11:04 ` Szabolcs Nagy
0 siblings, 1 reply; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-07 21:07 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: Sudakshina Das
On 30/04/2020 14:44, Szabolcs Nagy wrote:
> From 45c6bce5a691ecec9bba52785bd1f3a4cbc76fd4 Mon Sep 17 00:00:00 2001
> From: Sudakshina Das <sudi.das@arm.com>
> Date: Tue, 17 Mar 2020 15:54:12 +0000
> Subject: [PATCH 09/12] aarch64: support BTI enabled binaries
>
> Binaries can opt-in to using BTI via an ELF property 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.
>
> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> ---
> elf/dl-load.c | 2 +
> elf/rtld.c | 2 +
> sysdeps/aarch64/Makefile | 4 +
> sysdeps/aarch64/dl-bti.c | 54 ++++++
> sysdeps/aarch64/dl-prop.h | 170 ++++++++++++++++++
> sysdeps/aarch64/linkmap.h | 1 +
> 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 | 1 +
> 10 files changed, 269 insertions(+)
> 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 a6b80f9395..0930250619 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1145,6 +1145,8 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> l->l_relro_size = ph->p_memsz;
> break;
>
> + case PT_GNU_PROPERTY:
> + /* Fall through. PT_GNU_PROPERTY holds property notes. */
> case PT_NOTE:
> if (_dl_process_pt_note (l, ph, fd, fbp))
> {
This will print the same error message for a failure in _dl_process_pt_note
("cannot process note segment"). Wouldn't be better to use a more specific
error message, like "cannot process GNU property segment"?
> diff --git a/elf/rtld.c b/elf/rtld.c
> index b2ea21c98b..88b8e74de0 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1505,6 +1505,8 @@ 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:
> + /* Fall through. PT_GNU_PROPERTY holds property notes. */
> case PT_NOTE:
> if (_rtld_process_pt_note (main_map, ph))
> _dl_error_printf ("\
As before.
> 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..9ce697527d
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-bti.c
> @@ -0,0 +1,54 @@
> +/* AArch64 BTI initializers function.
'functions' maybe?
> + 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)
Space after cast.
> + {
> + 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_guarded)
> + enable_bti (l, program);
> +}
No implicit checks (since there are defined both as int).
> diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
> new file mode 100644
> index 0000000000..6662e4ab14
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-prop.h
> @@ -0,0 +1,170 @@
> +/* 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, 0);
It should be NULL here instead of 0 (glibc code guideline).
> +}
> +
> +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 aliged to 4 bytes in
s/aliged/aligned
> + 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;
The logic to parse the PT_GNU_PROPERTY is quite similar to the one
at sysdeps/x86/dl-prop.h to parse PT_NOTE. Would it be possible to
maybe try to consolidate the logic somewhere to avoid this code
duplication?
> +
> + 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_guarded = 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);
> + }
> +
> + /* NB: Note sections like .note.ABI-tag and .note.gnu.build-id are
> + aligned to 4 bytes in 64-bit ELF objects. */
> + note = ((const void *) note
> + + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
> + align));
> + }
> +}
> +
> +#ifdef FILEBUF_SIZE
> +static inline int __attribute__ ((unused))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
> + int fd, struct filebuf *fbp)
> +{
> + if (ph->p_type != PT_GNU_PROPERTY)
> + return 0;
> +
> + const ElfW(Nhdr) *note;
> + ElfW(Nhdr) *note_malloced = NULL;
> + ElfW(Addr) size = ph->p_filesz;
> +
> + if (ph->p_offset + size <= (size_t) fbp->len)
> + note = (const void *) (fbp->buf + ph->p_offset);
> + else
> + {
> + if (size < __MAX_ALLOCA_CUTOFF)
> + note = alloca (size);
> + else
> + note = note_malloced = malloc (size);
> + if (note == NULL)
> + return -1;
> + if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
> + {
> + if (note_malloced)
> + free (note_malloced);
> + return -1;
I wonder if we should use a scratch_buffer here instead.
> + }
> + }
> + _dl_process_aarch64_property (l, note, ph->p_filesz, ph->p_align);
> + if (note_malloced)
> + free (note_malloced);
> + return 0;
> +}
> +#endif
> +
> +static inline int __attribute__ ((unused))
> +_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> + if (ph->p_type != PT_GNU_PROPERTY)
> + return 0;
Not sure this is the right design to use the same function to handle
both PT_NOTE and PT_GNU_PROPERTY.
> + 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..cc196512d7 100644
> --- a/sysdeps/aarch64/linkmap.h
> +++ b/sysdeps/aarch64/linkmap.h
> @@ -20,4 +20,5 @@ struct link_map_machine
> {
> ElfW(Addr) plt; /* Address of .plt */
> void *tlsdesc_table; /* Address of TLS descriptor hash table. */
> + int bti_guarded; /* Branch Target Identification mechanism enabled. */
Maybe bool here?
> };
> 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)
This it not yet upstream on Linus tree (6e7f2eacf098), but follows
the arm64/for-next/bti branch (8ef8f360cf30be12).
> 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
Linux specific flags should be protected by __USE_MISC.
> +
> +#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..c2385fb498 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 enabled. */
> + 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..88983eb723 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> @@ -64,6 +64,7 @@ struct cpu_features
> {
> uint64_t midr_el1;
> unsigned zva_size;
> + int bti;
Maybe bool here?
> };
>
> #endif /* _CPU_FEATURES_AARCH64_H */
> --
> 2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 10/12] aarch64: Add pac-ret support to asm files
2020-04-30 17:44 ` [PATCH 10/12] aarch64: Add pac-ret support to asm files Szabolcs Nagy
@ 2020-05-08 16:59 ` Adhemerval Zanella
2020-05-11 8:27 ` Szabolcs Nagy
0 siblings, 1 reply; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-08 16:59 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: Sudakshina Das
On 30/04/2020 14:44, Szabolcs Nagy wrote:
> From de8968ed58686c26391de8343184a1283bb5e305 Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Date: Wed, 29 Apr 2020 11:49:20 +0100
> Subject: [PATCH 10/12] aarch64: Add pac-ret support to asm files
>
> This patch unconditionally enables pac-ret in asm files.
>
> TODO: This will need configure checks, cannot be done
> unconditionally because we cannot guarantee pac-ret
> compatibility (e.g. libgcc unwinder had no support for
> it before gcc-7 and newer libgcc had bugs that could
> cause unwind crash when pac-ret and non-pac-ret stack
> frames are mixed)
Which gcc version does it work correctly? Would it be a check against
a specific gcc version or could it be a configure to see if libgcc
provides the expected fixes?
The gcc-7 branch is now closed, so maybe we could assume gcc-8 and
have the required fixes backported?
In any case I think we should add a configure check based on compiler
-mbranch-protection= options used to enable/disable ENABLE_PAC_RET.
> ---
> sysdeps/aarch64/crti.S | 8 ++++++++
> sysdeps/aarch64/crtn.S | 6 ++++++
> sysdeps/aarch64/dl-tlsdesc.S | 8 ++++++++
> sysdeps/aarch64/dl-trampoline.S | 15 ++++++++++++++-
> sysdeps/aarch64/sysdep.h | 18 +++++++++++++++++-
> 5 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
> index 89a9e25f5b..36f58c9a01 100644
> --- a/sysdeps/aarch64/crti.S
> +++ b/sysdeps/aarch64/crti.S
> @@ -75,7 +75,11 @@ call_weak_fn:
> .hidden _init
> .type _init, %function
> _init:
> +#if ENABLE_PAC_RET
> + PACIASP
> +#else
> BTI_C
> +#endif
> stp x29, x30, [sp, -16]!
> mov x29, sp
> #if PREINIT_FUNCTION_WEAK
> @@ -90,7 +94,11 @@ _init:
> .hidden _fini
> .type _fini, %function
> _fini:
> +#if ENABLE_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 94a6f970ef..e1cb74a572 100644
> --- a/sysdeps/aarch64/crtn.S
> +++ b/sysdeps/aarch64/crtn.S
> @@ -41,10 +41,16 @@
>
> .section .init,"ax",%progbits
> ldp x29, x30, [sp], 16
> +#if ENABLE_PAC_RET
> + AUTIASP
> +#endif
> RET
>
> .section .fini,"ax",%progbits
> ldp x29, x30, [sp], 16
> +#if ENABLE_PAC_RET
> + AUTIASP
> +#endif
> RET
>
> END_FILE
> diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
> index d55e0443aa..25628d942f 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. */
> +# if ENABLE_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)
> +#if ENABLE_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 fba5689d09..c0c4c23128 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:
> +# if ENABLE_PAC_RET
> + PACIASP
> + cfi_window_save
> +# else
> BTI_C
> +# endif
> /* AArch64 we get called with:
> ip0 &PLTGOT[2]
> ip1 temp(dl resolver entry point)
> @@ -291,9 +296,17 @@ _dl_runtime_profile:
> cfi_def_cfa_register (sp)
> ldr x29, [x29, #0]
> cfi_restore(x29)
> +# if ENABLE_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
> br lr
>
> cfi_endproc
> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 07dc7858a5..63a04a70cd 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -45,6 +45,18 @@
> #define BTI_C hint 34
> #define BTI_J hint 36
>
> +/* Return address signing support (pac-ret). */
> +#define ENABLE_PAC_RET 1
> +#if ENABLE_PAC_RET
> +# define PACIASP hint 25
> +# define AUTIASP hint 29
> +# define PACIASP_AND_BTI_C PACIASP
> +#else
> +# define PACIASP
> +# define AUTIASP
> +# define PACIASP_AND_BTI_C BTI_C
> +#endif
> +
The PACIASP/AUTIASP are already protected by ENABLE_PAC_RET, why do you need
to redefine it for !ENABLE_PAC_RET?
Also, the macro PACIASP_AND_BTI_C is not used in this specific, should it
be defined where it is actually used?
> #define FEATURE_1_BTI 1
> #define FEATURE_1_PAC 2
>
> @@ -61,7 +73,11 @@
> .word features; \
> .word 0;
>
> -#define END_FILE GNU_PROPERTY(FEATURE_1_BTI)
> +#if ENABLE_PAC_RET
> +# define END_FILE GNU_PROPERTY(FEATURE_1_BTI|FEATURE_1_PAC)
> +#else
> +# define END_FILE GNU_PROPERTY(FEATURE_1_BTI)
> +#endif
>
> /* Define an entry point visible from C. */
> #define ENTRY(name) \
> --
> 2.17.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-04-30 17:45 ` [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
@ 2020-05-08 17:44 ` Adhemerval Zanella
2020-05-11 12:38 ` Szabolcs Nagy
2020-05-11 19:22 ` Florian Weimer
1 sibling, 1 reply; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-08 17:44 UTC (permalink / raw)
To: libc-alpha
On 30/04/2020 14:45, 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 that includes a
> pointer authentication code, so the definition is adjusted.
>
> XPAC is added unconditionally for now, but it's only needed if
> glibc is compiled with -mbranch-protection=pac-ret. Inline asm
> is used instead of __builtin_aarch64_xpaclri since that's an
> undocumented builtin and not available in all supported gccs.
> ---
> sysdeps/aarch64/sysdep.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 63a04a70cd..87f19b9bef 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -35,6 +35,16 @@
>
> #define PTR_SIZE (1<<PTR_LOG_SIZE)
>
> +/* Strip pointer authentication code from pointer p. */
> +#define XPAC(p) ({ \
> + register void *__ra asm ("x30") = (p); \
> + asm ("hint 7 // xpaclri" : "+r"(__ra)); \
> + __ra;})
> +
> +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
> +#undef RETURN_ADDRESS
> +#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
> +
Maybe use a inline function instead?
#ifndef __ASSEMBLER__
# include <sys/cdefs.h>
/* Strip pointer authentication code from pointer p. */
static __always_inline void *
return_address (unsigned int n)
{
register void *ra asm ("x30") = __builtin_return_address (n);
asm ("hint 7 // xpaclri" : "+r" (ra));
return ra;
}
/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
# undef RETURN_ADDRESS
# define RETURN_ADDRESS(n) return_address (n)
#endif
> #ifdef __ASSEMBLER__
>
> /* Syntactic details of assembler. */
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 12/12] aarch64: Configure option to build glibc with branch protection
2020-04-30 17:45 ` [PATCH 12/12] aarch64: Configure option to build glibc with branch protection Szabolcs Nagy
2020-04-30 19:02 ` Joseph Myers
@ 2020-05-08 17:53 ` Adhemerval Zanella
1 sibling, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-08 17:53 UTC (permalink / raw)
To: libc-alpha
On 30/04/2020 14:45, Szabolcs Nagy wrote:
> If gcc is configured with --enable-standard-branch-protection then
> the built glibc should have branch protection suppport too, which
> includes bti and pac-ret. The new configure option is only for
> additional configure checks, it does not try to add new CFLAGS
> (i.e. -mbranch-protection=standard ), it expects gcc to default to
> using branch protection, since likely the static linked compiler
> libraries are not compatible otherwise.
>
> The -z force-bti linker flag is also passed with branch protection,
> but this is not very useful: by default the BTI property marking
> is set on the linker output if all linker inputs have it and it is
> silently missing otherwise, -z force-bti at least warns if an input
> is missing the property, but that's not a fatal error.
> (Using --fatal-warnings ld flag does not work in the test system.)
Since BTI/PAC is enabled as default with nop compatible instructions,
I think it would be simpler to add a aarch64 configure check against
the compiler flags used instead of adding a configure switch. It
could be used to select set ENABLE_PAC_RET as well.
With a configure check the BTI enablement will be transparent and
controlled by the either default compiler default or the selected
CC/CFLAGS (as used by others ABI selections).
>
> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> ---
> configure | 14 +++++++++++++-
> configure.ac | 6 ++++++
> sysdeps/aarch64/Makefile | 4 ++++
> sysdeps/aarch64/configure | 31 +++++++++++++++++++++++++++++++
> sysdeps/aarch64/configure.ac | 19 +++++++++++++++++++
> 5 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 8df47d61f8..fff5734f6d 100755
> --- a/configure
> +++ b/configure
> @@ -794,6 +794,7 @@ enable_pt_chown
> enable_tunables
> enable_mathvec
> enable_cet
> +enable_standard_branch_protection
> with_cpu
> '
> ac_precious_vars='build_alias
> @@ -1471,6 +1472,9 @@ Optional Features:
> depends on architecture]
> --enable-cet enable Intel Control-flow Enforcement Technology
> (CET), x86 only
> + --enable-standard-branch-protection
> + enable AArch64 Branch Target Identification and
> + Return Address Signing, AArch64 only
>
> Optional Packages:
> --with-PACKAGE[=ARG] use PACKAGE [ARG=yes]
> @@ -3785,7 +3789,7 @@ main ()
> {
>
> #ifndef __CET__
> -#error no CET compiler support
> +# error no CET compiler support
> #endif
> ;
> return 0;
Spurious change.
> @@ -3806,6 +3810,14 @@ else
> fi
>
>
> +# Check whether --enable-standard-branch-protection was given.
> +if test "${enable_standard_branch_protection+set}" = set; then :
> + enableval=$enable_standard_branch_protection; libc_cv_branch_protection=$enableval
> +else
> + libc_cv_branch_protection=no
> +fi
> +
> +
> # We keep the original values in `$config_*' and never modify them, so we
> # can write them unchanged into config.make. Everything else uses
> # $machine, $vendor, and $os, and changes them whenever convenient.
> diff --git a/configure.ac b/configure.ac
> index 5f229679a9..e08b0f3766 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -486,6 +486,12 @@ AC_ARG_ENABLE([cet],
> [enable_cet=$enableval],
> [enable_cet=$libc_cv_compiler_default_cet])
>
> +AC_ARG_ENABLE([standard-branch-protection],
> + AC_HELP_STRING([--enable-standard-branch-protection],
> + [enable AArch64 Branch Target Identification and Return Address Signing, AArch64 only]),
> + [libc_cv_branch_protection=$enableval],
> + [libc_cv_branch_protection=no])
> +
> # We keep the original values in `$config_*' and never modify them, so we
> # can write them unchanged into config.make. Everything else uses
> # $machine, $vendor, and $os, and changes them whenever convenient.
> diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> index 5ae8b082b0..313c371e72 100644
> --- a/sysdeps/aarch64/Makefile
> +++ b/sysdeps/aarch64/Makefile
> @@ -1,5 +1,9 @@
> long-double-fcts = yes
>
> +ifeq (yes,$(enable-branch-protection))
> +sysdep-LDFLAGS += -Wl,-z,force-bti
> +endif
> +
> ifeq ($(subdir),elf)
> sysdep-dl-routines += dl-bti
> endif
> diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> index 5bd355a691..83a6c8c852 100644
> --- a/sysdeps/aarch64/configure
> +++ b/sysdeps/aarch64/configure
> @@ -172,3 +172,34 @@ else
> config_vars="$config_vars
> default-abi = lp64"
> fi
> +
> +if test "$libc_cv_branch_protection" = yes; then
> + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for branch protection support" >&5
> +$as_echo_n "checking for branch protection support... " >&6; }
> +if ${libc_cv_branch_protection_support+:} false; then :
> + $as_echo_n "(cached) " >&6
> +else
> + cat > conftest.c <<EOF
> +void foo (void) { }
> +EOF
> + libc_cv_branch_protection_support=no
> + if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -fPIC -shared -Wl,-z,force-bti,--fatal-warnings -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; }; } \
> + && LC_ALL=C $READELF -Wn conftest.so | \
> + grep -q 'NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI'; then
> + libc_cv_branch_protection_support=yes
> + fi
> + rm -rf conftest.*
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_branch_protection_support" >&5
> +$as_echo "$libc_cv_branch_protection_support" >&6; }
> + if test $libc_cv_branch_protection_support = no; then
> + as_fn_error $? "branch-protection is enabled, but the toolchain does not support it." "$LINENO" 5
> + fi
> +fi
> +config_vars="$config_vars
> +enable-branch-protection = $libc_cv_branch_protection"
> diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> index 7851dd4dac..d16ba3710a 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
> +
> +if test "$libc_cv_branch_protection" = yes; then
> + AC_CACHE_CHECK([for branch protection support],
> + [libc_cv_branch_protection_support],
> + [cat > conftest.c <<EOF
> +void foo (void) { }
> +EOF
> + libc_cv_branch_protection_support=no
> + if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -fPIC -shared -Wl,-z,force-bti,--fatal-warnings -o conftest.so conftest.c]) \
> + && LC_ALL=C $READELF -Wn conftest.so | \
> + grep -q 'NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI'; then
> + libc_cv_branch_protection_support=yes
> + fi
> + rm -rf conftest.*])
> + if test $libc_cv_branch_protection_support = no; then
> + AC_MSG_ERROR([branch-protection is enabled, but the toolchain does not support it.])
> + fi
> +fi
> +LIBC_CONFIG_VAR([enable-branch-protection], [$libc_cv_branch_protection])
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 10/12] aarch64: Add pac-ret support to asm files
2020-05-08 16:59 ` Adhemerval Zanella
@ 2020-05-11 8:27 ` Szabolcs Nagy
2020-05-11 18:39 ` Adhemerval Zanella
0 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-11 8:27 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Sudakshina Das
The 05/08/2020 13:59, Adhemerval Zanella wrote:
> On 30/04/2020 14:44, Szabolcs Nagy wrote:
> > From de8968ed58686c26391de8343184a1283bb5e305 Mon Sep 17 00:00:00 2001
> > From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> > Date: Wed, 29 Apr 2020 11:49:20 +0100
> > Subject: [PATCH 10/12] aarch64: Add pac-ret support to asm files
> >
> > This patch unconditionally enables pac-ret in asm files.
> >
> > TODO: This will need configure checks, cannot be done
> > unconditionally because we cannot guarantee pac-ret
> > compatibility (e.g. libgcc unwinder had no support for
> > it before gcc-7 and newer libgcc had bugs that could
> > cause unwind crash when pac-ret and non-pac-ret stack
> > frames are mixed)
>
> Which gcc version does it work correctly? Would it be a check against
> a specific gcc version or could it be a configure to see if libgcc
> provides the expected fixes?
>
> The gcc-7 branch is now closed, so maybe we could assume gcc-8 and
> have the required fixes backported?
the version number is not useful because the fix
can be backported (even to closed branches, just
not upstream).
but gcc 10.1 has no known bug.
(except if -pg and __builtin_return_address
handling changes, i consider current -pg
broken with pac-ret, but we might decide to
fix it up in _mcount instead of the compiler)
>
> In any case I think we should add a configure check based on compiler
> -mbranch-protection= options used to enable/disable ENABLE_PAC_RET.
yeah i changed the configury bits in v2, i didnt
do it in v1 because it's quite painful: gcc
does not set a macro to test for it
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 09/12] aarch64: support BTI enabled binaries
2020-05-07 21:07 ` Adhemerval Zanella
@ 2020-05-11 11:04 ` Szabolcs Nagy
2020-05-11 18:38 ` Adhemerval Zanella
0 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-11 11:04 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Sudakshina Das
The 05/07/2020 18:07, Adhemerval Zanella wrote:
> On 30/04/2020 14:44, Szabolcs Nagy wrote:
> > From 45c6bce5a691ecec9bba52785bd1f3a4cbc76fd4 Mon Sep 17 00:00:00 2001
> > From: Sudakshina Das <sudi.das@arm.com>
> > Date: Tue, 17 Mar 2020 15:54:12 +0000
> > Subject: [PATCH 09/12] aarch64: support BTI enabled binaries
> >
> > Binaries can opt-in to using BTI via an ELF property 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.
> >
> > Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> > ---
> > elf/dl-load.c | 2 +
> > elf/rtld.c | 2 +
> > sysdeps/aarch64/Makefile | 4 +
> > sysdeps/aarch64/dl-bti.c | 54 ++++++
> > sysdeps/aarch64/dl-prop.h | 170 ++++++++++++++++++
> > sysdeps/aarch64/linkmap.h | 1 +
> > 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 | 1 +
> > 10 files changed, 269 insertions(+)
> > 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 a6b80f9395..0930250619 100644
> > --- a/elf/dl-load.c
> > +++ b/elf/dl-load.c
> > @@ -1145,6 +1145,8 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> > l->l_relro_size = ph->p_memsz;
> > break;
> >
> > + case PT_GNU_PROPERTY:
> > + /* Fall through. PT_GNU_PROPERTY holds property notes. */
> > case PT_NOTE:
> > if (_dl_process_pt_note (l, ph, fd, fbp))
> > {
>
> This will print the same error message for a failure in _dl_process_pt_note
> ("cannot process note segment"). Wouldn't be better to use a more specific
> error message, like "cannot process GNU property segment"?
i was struggling to deal with this, i'm happy
to create a new hook for pt_gnu_property
(that's actually cleaner in aarch64, but x86
will have to continue to look at PT_NOTE for
the same).
it requires more generic changes though and
related code repetitions.
> > + 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;
>
> The logic to parse the PT_GNU_PROPERTY is quite similar to the one
> at sysdeps/x86/dl-prop.h to parse PT_NOTE. Would it be possible to
> maybe try to consolidate the logic somewhere to avoid this code
> duplication?
yes it's similar but not the same.
x86 tries to deal with multiple property notes
which does not happen on aarch64.
i can try to refactor the code and see if
that works.
> > + if (ph->p_offset + size <= (size_t) fbp->len)
> > + note = (const void *) (fbp->buf + ph->p_offset);
> > + else
> > + {
> > + if (size < __MAX_ALLOCA_CUTOFF)
> > + note = alloca (size);
> > + else
> > + note = note_malloced = malloc (size);
> > + if (note == NULL)
> > + return -1;
> > + if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
> > + {
> > + if (note_malloced)
> > + free (note_malloced);
> > + return -1;
>
> I wonder if we should use a scratch_buffer here instead.
this logic is copied from x86, on aarch64 local buffer
should work with current linkers (since they will only
add at most one gnu property note to PT_GNU_PROPERTY),
but we don't know what happens in the future so the
malloc fallback is probably required.
i think ideally the segment is mmaped into memory and
we can just use that, but i assumed the logic is there
for a reason on x86.
> > +++ b/sysdeps/aarch64/linkmap.h
> > @@ -20,4 +20,5 @@ struct link_map_machine
> > {
> > ElfW(Addr) plt; /* Address of .plt */
> > void *tlsdesc_table; /* Address of TLS descriptor hash table. */
> > + int bti_guarded; /* Branch Target Identification mechanism enabled. */
>
> Maybe bool here?
ok.
> > +++ 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)
>
> This it not yet upstream on Linus tree (6e7f2eacf098), but follows
> the arm64/for-next/bti branch (8ef8f360cf30be12).
yes, now renamed to bti-user (because the kernel
code can also use bti protection) it is scheduled
for next linux, this patchset depends on that work,
but they have to be tested together.
> > +/* AArch64 specific definitions, should be in sync with
> > + arch/arm64/include/uapi/asm/mman.h. */
> > +
> > +#define PROT_BTI 0x10
>
> Linux specific flags should be protected by __USE_MISC.
in posix the PROT_ prefix is reserved for sys/mman.h
so there is no namespace issue with this.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
see also PROT_SAO on powerpc.
> > +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> > @@ -64,6 +64,7 @@ struct cpu_features
> > {
> > uint64_t midr_el1;
> > unsigned zva_size;
> > + int bti;
>
> Maybe bool here?
ok.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 03/12] aarch64: Add BTI landing pads to assembly code
2020-05-07 16:55 ` Adhemerval Zanella
@ 2020-05-11 11:38 ` Szabolcs Nagy
2020-05-11 19:13 ` Adhemerval Zanella
0 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-11 11:38 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
The 05/07/2020 13:55, Adhemerval Zanella via Libc-alpha wrote:
> On 30/04/2020 14:39, Szabolcs Nagy wrote:
> > From 550fe66ed93e13c0f063955e81bfcb8db386413c Mon Sep 17 00:00:00 2001
> > From: Sudakshina Das <sudi.das@arm.com>
> > Date: Tue, 17 Mar 2020 15:44:18 +0000
> > Subject: [PATCH 03/12] aarch64: Add BTI landing pads to assembly code
> >
> > Adding the landing pads and the ELF markings are required if
> > glibc is built with branch protection. For the handful of asm
> > files this is done unconditionally, this simplifies maintenance
> > and avoids complications where code layout is carefully aligned
> > such that conditionally turning BTI off may cause performance
> > regression (e.g. string functions).
>
> I am seeing the warning the cover letter warned on old binutils:
>
> unsupported GNU_PROPERTY_TYPE (5) type: 0xc0000000
>
> In this case, how exactly binutils handles it? Does it still emit
> the expected correct notes, and if so, is the expected layout?
ok, v2 takes care of this (with config checks).
(linkers that don't handle the bti property will either
emit a note that is not valid so ignored at runtime or
not emit any note, so the warnings don't mean broken
behaviour just turned off security feature)
> > +/* Branch Target Identitication support. */
> > +#define BTI_C hint 34
> > +#define BTI_J hint 36
> > +
> > +#define FEATURE_1_BTI 1
> > +#define FEATURE_1_PAC 2
>
> There are GNU_PROPERTY_AARCH64_FEATURE_1_BTI and GNU_PROPERTY_AARCH64_FEATURE_1_PAC,
> why not use them?
i need definitions suitable for inclusion
into assembly code.
(i shortened the name so it does not go
off the screen, but i can use the same
name as in elf.h if that's less confusing)
> > +#define END_FILE GNU_PROPERTY(FEATURE_1_BTI)
>
> END_FILE name does not really give much information. Would be better
> to use ADD_GNU_PROPERTIES or something?
the assumption was that we may need to
add other things to asm files and don't
want to touch all asm files when that
happens, just reuse the END_FILE hook.
on the other hand i'm not sure how much
value this hook adds: i can do what arm
does for attributes: add them in sysdep.h
at the top of asm files that include it.
the sysdep.h solution means less changes
but may be easier to miss when new asm
files are added: we need to ensure that
all newly added asm code is compatible
with the added properties, including asm
like syscall-template.S that is shared
across targets. Explicitly adding END_FILE
implies the person who added it verified
the necessary properties, but it's not a
strong assurance and you could say the
same for adding 'include <sysdep.h>'.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 07/12] aarch64: fix syscalls for BTI
2020-05-07 19:40 ` Adhemerval Zanella
@ 2020-05-11 11:46 ` Szabolcs Nagy
0 siblings, 0 replies; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-11 11:46 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
The 05/07/2020 16:40, Adhemerval Zanella via Libc-alpha wrote:
> > +#include <sysdep.h>
> > +PSEUDO (__umount2, umount2, 2)
> > + ret
> > +PSEUDO_END(__umount2)
> > +weak_alias (__umount2, umount2)
> > +END_FILE
>
> I think it is better to adapt the linux umount2.S to a C file instead of
> adding another arch-specific implementation. Only alpha and ia64 do
> not support __NR_umount2 (exported as __NR_umount), but recent kernel
> fixes (74cd2184833f for ia64, 12b57c5c70f39 for alpha) add the alias.
> So we can use __NR_umount without the need to check its definition:
yes i think this is a useful thing to do
(independently of aarch64 bti work).
doesn't the patch add new umount2 symbol on
ia64 and alpha though?
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 0326f92c40..62ac921e8f 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -54,7 +54,7 @@ CFLAGS-malloc.c += -DMORECORE_CLEARS=2
> endif
>
> ifeq ($(subdir),misc)
> -sysdep_routines += adjtimex clone umount umount2 readahead sysctl \
> +sysdep_routines += adjtimex clone umount readahead sysctl \
> setfsuid setfsgid epoll_pwait signalfd \
> eventfd eventfd_read eventfd_write prlimit \
> personality epoll_wait tee vmsplice splice \
> diff --git a/sysdeps/unix/sysv/linux/ia64/syscalls.list b/sysdeps/unix/sysv/linux/ia64/syscalls.list
> index 56f4138c43..bd39441d3c 100644
> --- a/sysdeps/unix/sysv/linux/ia64/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/ia64/syscalls.list
> @@ -1,7 +1,5 @@
> # File name Caller Syscall name # args Strong name Weak names
>
> -umount2 - umount 2 __umount2 umount2
> -
> getpriority - getpriority i:ii __getpriority getpriority
>
> # proper socket implementations:
> diff --git a/sysdeps/unix/sysv/linux/umount.c b/sysdeps/unix/sysv/linux/umount.c
> index ab6a20f596..cdc8759118 100644
> --- a/sysdeps/unix/sysv/linux/umount.c
> +++ b/sysdeps/unix/sysv/linux/umount.c
> @@ -16,12 +16,17 @@
> License along with the GNU C Library. If not, see
> <https://www.gnu.org/licenses/>. */
>
> -/* Since the generic Linux syscall ABI doesn't have an oldumount system call,
> - do what the kernel does down here. */
> +#include <sys/mount.h>
> +#include <sysdep.h>
>
> -extern long int __umount2 (const char *name, int flags);
> +int
> +__umount2 (const char *special_file, int flags)
> +{
> + return INLINE_SYSCALL_CALL (umount2, special_file, flags);
> +}
> +weak_alias (__umount2, umount2)
>
> -long int
> +int
> __umount (const char *name)
> {
> return __umount2 (name, 0);
> diff --git a/sysdeps/unix/sysv/linux/umount2.S b/sysdeps/unix/sysv/linux/umount2.S
> deleted file mode 100644
> index 92241bbf97..0000000000
> --- a/sysdeps/unix/sysv/linux/umount2.S
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -/* umount system call with two parameters. */
> -
> -#include <sysdep.h>
> -#if defined __NR_oldumount || defined __NR_umount2
> -#ifdef __NR_oldumount
> -PSEUDO (__umount2, umount, 2)
> -#else
> -PSEUDO (__umount2, umount2, 2)
> -#endif
> - ret
> -PSEUDO_END(__umount2)
> -weak_alias (__umount2, umount2)
> -#endif
>
>
--
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-05-08 17:44 ` Adhemerval Zanella
@ 2020-05-11 12:38 ` Szabolcs Nagy
2020-05-11 19:15 ` Adhemerval Zanella
0 siblings, 1 reply; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-11 12:38 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
The 05/08/2020 14:44, Adhemerval Zanella via Libc-alpha wrote:
> On 30/04/2020 14:45, Szabolcs Nagy wrote:
> > +++ b/sysdeps/aarch64/sysdep.h
> > @@ -35,6 +35,16 @@
> >
> > #define PTR_SIZE (1<<PTR_LOG_SIZE)
> >
> > +/* Strip pointer authentication code from pointer p. */
> > +#define XPAC(p) ({ \
> > + register void *__ra asm ("x30") = (p); \
> > + asm ("hint 7 // xpaclri" : "+r"(__ra)); \
> > + __ra;})
> > +
> > +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
> > +#undef RETURN_ADDRESS
> > +#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
> > +
>
> Maybe use a inline function instead?
macro seems more reliable to me than always_inline
when poking at __builtin_return_address and x30,
but i'm not against always_inline if that's
considered better.
i'd prefer separate xpac (since it can be used
not just with __builtin_return_address e.g. for
stored code address in jmpbuf, which currently
uses ptrmangling)
> #ifndef __ASSEMBLER__
> # include <sys/cdefs.h>
what is cdefs.h for?
> /* Strip pointer authentication code from pointer p. */
> static __always_inline void *
> return_address (unsigned int n)
> {
> register void *ra asm ("x30") = __builtin_return_address (n);
> asm ("hint 7 // xpaclri" : "+r" (ra));
> return ra;
> }
>
> /* This is needed when glibc is built with -mbranch-protection=pac-ret. */
> # undef RETURN_ADDRESS
> # define RETURN_ADDRESS(n) return_address (n)
> #endif
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 09/12] aarch64: support BTI enabled binaries
2020-05-11 11:04 ` Szabolcs Nagy
@ 2020-05-11 18:38 ` Adhemerval Zanella
0 siblings, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-11 18:38 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, Sudakshina Das
On 11/05/2020 08:04, Szabolcs Nagy wrote:
> The 05/07/2020 18:07, Adhemerval Zanella wrote:
>> On 30/04/2020 14:44, Szabolcs Nagy wrote:
>>> From 45c6bce5a691ecec9bba52785bd1f3a4cbc76fd4 Mon Sep 17 00:00:00 2001
>>> From: Sudakshina Das <sudi.das@arm.com>
>>> Date: Tue, 17 Mar 2020 15:54:12 +0000
>>> Subject: [PATCH 09/12] aarch64: support BTI enabled binaries
>>>
>>> Binaries can opt-in to using BTI via an ELF property 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.
>>>
>>> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>> ---
>>> elf/dl-load.c | 2 +
>>> elf/rtld.c | 2 +
>>> sysdeps/aarch64/Makefile | 4 +
>>> sysdeps/aarch64/dl-bti.c | 54 ++++++
>>> sysdeps/aarch64/dl-prop.h | 170 ++++++++++++++++++
>>> sysdeps/aarch64/linkmap.h | 1 +
>>> 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 | 1 +
>>> 10 files changed, 269 insertions(+)
>>> 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 a6b80f9395..0930250619 100644
>>> --- a/elf/dl-load.c
>>> +++ b/elf/dl-load.c
>>> @@ -1145,6 +1145,8 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>>> l->l_relro_size = ph->p_memsz;
>>> break;
>>>
>>> + case PT_GNU_PROPERTY:
>>> + /* Fall through. PT_GNU_PROPERTY holds property notes. */
>>> case PT_NOTE:
>>> if (_dl_process_pt_note (l, ph, fd, fbp))
>>> {
>>
>> This will print the same error message for a failure in _dl_process_pt_note
>> ("cannot process note segment"). Wouldn't be better to use a more specific
>> error message, like "cannot process GNU property segment"?
>
> i was struggling to deal with this, i'm happy
> to create a new hook for pt_gnu_property
> (that's actually cleaner in aarch64, but x86
> will have to continue to look at PT_NOTE for
> the same).
>
> it requires more generic changes though and
> related code repetitions.
I think it might be worth, specially since the _dl_process_pt_note
stills used the __MAX_ALLOCA_CUTOFF (which I think we should replace
with a scratch_buffer).
>
>>> + 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;
>>
>> The logic to parse the PT_GNU_PROPERTY is quite similar to the one
>> at sysdeps/x86/dl-prop.h to parse PT_NOTE. Would it be possible to
>> maybe try to consolidate the logic somewhere to avoid this code
>> duplication?
>
> yes it's similar but not the same.
>
> x86 tries to deal with multiple property notes
> which does not happen on aarch64.
>
> i can try to refactor the code and see if
> that works.
Maybe parametrize the logic of number of property notes?
>
>>> + if (ph->p_offset + size <= (size_t) fbp->len)
>>> + note = (const void *) (fbp->buf + ph->p_offset);
>>> + else
>>> + {
>>> + if (size < __MAX_ALLOCA_CUTOFF)
>>> + note = alloca (size);
>>> + else
>>> + note = note_malloced = malloc (size);
>>> + if (note == NULL)
>>> + return -1;
>>> + if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
>>> + {
>>> + if (note_malloced)
>>> + free (note_malloced);
>>> + return -1;
>>
>> I wonder if we should use a scratch_buffer here instead.
>
> this logic is copied from x86, on aarch64 local buffer
> should work with current linkers (since they will only
> add at most one gnu property note to PT_GNU_PROPERTY),
> but we don't know what happens in the future so the
> malloc fallback is probably required.
>
> i think ideally the segment is mmaped into memory and
> we can just use that, but i assumed the logic is there
> for a reason on x86.
Would mmap/unmap the segment a better strategy than use a small stack
allocation for most of cases with a malloc fallback (which I think
won't be used unless a ill formatted note)?
>
>>> +++ b/sysdeps/aarch64/linkmap.h
>>> @@ -20,4 +20,5 @@ struct link_map_machine
>>> {
>>> ElfW(Addr) plt; /* Address of .plt */
>>> void *tlsdesc_table; /* Address of TLS descriptor hash table. */
>>> + int bti_guarded; /* Branch Target Identification mechanism enabled. */
>>
>> Maybe bool here?
>
> ok.
>
>>> +++ 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)
>>
>> This it not yet upstream on Linus tree (6e7f2eacf098), but follows
>> the arm64/for-next/bti branch (8ef8f360cf30be12).
>
> yes, now renamed to bti-user (because the kernel
> code can also use bti protection) it is scheduled
> for next linux, this patchset depends on that work,
> but they have to be tested together.
Ack, we just need to avoid another BZ#25971.
>
>>> +/* AArch64 specific definitions, should be in sync with
>>> + arch/arm64/include/uapi/asm/mman.h. */
>>> +
>>> +#define PROT_BTI 0x10
>>
>> Linux specific flags should be protected by __USE_MISC.
>
> in posix the PROT_ prefix is reserved for sys/mman.h
> so there is no namespace issue with this.
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
>
> see also PROT_SAO on powerpc.
>
MMAP_ as well, but we still protect Linux ones with __USE_MISC.
The conform/data/sys/mman.h-data does not trigger a namespace issue
for neither MMAP_ nor PROT_, so I am not sure which would be the
best policy here.
>>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>>> @@ -64,6 +64,7 @@ struct cpu_features
>>> {
>>> uint64_t midr_el1;
>>> unsigned zva_size;
>>> + int bti;
>>
>> Maybe bool here?
>
> ok.
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 10/12] aarch64: Add pac-ret support to asm files
2020-05-11 8:27 ` Szabolcs Nagy
@ 2020-05-11 18:39 ` Adhemerval Zanella
0 siblings, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-11 18:39 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, Sudakshina Das
On 11/05/2020 05:27, Szabolcs Nagy wrote:
> The 05/08/2020 13:59, Adhemerval Zanella wrote:
>> On 30/04/2020 14:44, Szabolcs Nagy wrote:
>>> From de8968ed58686c26391de8343184a1283bb5e305 Mon Sep 17 00:00:00 2001
>>> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>> Date: Wed, 29 Apr 2020 11:49:20 +0100
>>> Subject: [PATCH 10/12] aarch64: Add pac-ret support to asm files
>>>
>>> This patch unconditionally enables pac-ret in asm files.
>>>
>>> TODO: This will need configure checks, cannot be done
>>> unconditionally because we cannot guarantee pac-ret
>>> compatibility (e.g. libgcc unwinder had no support for
>>> it before gcc-7 and newer libgcc had bugs that could
>>> cause unwind crash when pac-ret and non-pac-ret stack
>>> frames are mixed)
>>
>> Which gcc version does it work correctly? Would it be a check against
>> a specific gcc version or could it be a configure to see if libgcc
>> provides the expected fixes?
>>
>> The gcc-7 branch is now closed, so maybe we could assume gcc-8 and
>> have the required fixes backported?
>
> the version number is not useful because the fix
> can be backported (even to closed branches, just
> not upstream).
>
> but gcc 10.1 has no known bug.
> (except if -pg and __builtin_return_address
> handling changes, i consider current -pg
> broken with pac-ret, but we might decide to
> fix it up in _mcount instead of the compiler)
Ack.
>
>>
>> In any case I think we should add a configure check based on compiler
>> -mbranch-protection= options used to enable/disable ENABLE_PAC_RET.
>
> yeah i changed the configury bits in v2, i didnt
> do it in v1 because it's quite painful: gcc
> does not set a macro to test for it
>
Yes, such checks requires to build a code snippet and parse for specific
instructions patterns.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 03/12] aarch64: Add BTI landing pads to assembly code
2020-05-11 11:38 ` Szabolcs Nagy
@ 2020-05-11 19:13 ` Adhemerval Zanella
0 siblings, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-11 19:13 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha
On 11/05/2020 08:38, Szabolcs Nagy wrote:
> The 05/07/2020 13:55, Adhemerval Zanella via Libc-alpha wrote:
>> On 30/04/2020 14:39, Szabolcs Nagy wrote:
>>> From 550fe66ed93e13c0f063955e81bfcb8db386413c Mon Sep 17 00:00:00 2001
>>> From: Sudakshina Das <sudi.das@arm.com>
>>> Date: Tue, 17 Mar 2020 15:44:18 +0000
>>> Subject: [PATCH 03/12] aarch64: Add BTI landing pads to assembly code
>>>
>>> Adding the landing pads and the ELF markings are required if
>>> glibc is built with branch protection. For the handful of asm
>>> files this is done unconditionally, this simplifies maintenance
>>> and avoids complications where code layout is carefully aligned
>>> such that conditionally turning BTI off may cause performance
>>> regression (e.g. string functions).
>>
>> I am seeing the warning the cover letter warned on old binutils:
>>
>> unsupported GNU_PROPERTY_TYPE (5) type: 0xc0000000
>>
>> In this case, how exactly binutils handles it? Does it still emit
>> the expected correct notes, and if so, is the expected layout?
>
> ok, v2 takes care of this (with config checks).
Ack.
>
> (linkers that don't handle the bti property will either
> emit a note that is not valid so ignored at runtime or
> not emit any note, so the warnings don't mean broken
> behaviour just turned off security feature)
>
>>> +/* Branch Target Identitication support. */
>>> +#define BTI_C hint 34
>>> +#define BTI_J hint 36
>>> +
>>> +#define FEATURE_1_BTI 1
>>> +#define FEATURE_1_PAC 2
>>
>> There are GNU_PROPERTY_AARCH64_FEATURE_1_BTI and GNU_PROPERTY_AARCH64_FEATURE_1_PAC,
>> why not use them?
>
> i need definitions suitable for inclusion
> into assembly code.
> (i shortened the name so it does not go
> off the screen, but i can use the same
> name as in elf.h if that's less confusing)
Ack.
>
>>> +#define END_FILE GNU_PROPERTY(FEATURE_1_BTI)
>>
>> END_FILE name does not really give much information. Would be better
>> to use ADD_GNU_PROPERTIES or something?
>
> the assumption was that we may need to
> add other things to asm files and don't
> want to touch all asm files when that
> happens, just reuse the END_FILE hook.
>
> on the other hand i'm not sure how much
> value this hook adds: i can do what arm
> does for attributes: add them in sysdep.h
> at the top of asm files that include it.
>
> the sysdep.h solution means less changes
> but may be easier to miss when new asm
> files are added: we need to ensure that
> all newly added asm code is compatible
> with the added properties, including asm
> like syscall-template.S that is shared
> across targets. Explicitly adding END_FILE
> implies the person who added it verified
> the necessary properties, but it's not a
> strong assurance and you could say the
> same for adding 'include <sysdep.h>'.
>
I was referring to the define name in fact. But now that you brought
the different ways of define it, maybe setting it as arm attributes
might be a better alternative.
Although not required, current practice is to use sysdep.h macros
to define function entrypoints. So I think it should be feasible
to enforce its inclusion for all assembly implementations.
But I don't have a strong opinion here which mechanism is better.
What I think it would be useful is a way at build time to certify
that if BTI is enabled, the linker warns if a object has a missing
property.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-05-11 12:38 ` Szabolcs Nagy
@ 2020-05-11 19:15 ` Adhemerval Zanella
2020-05-11 19:21 ` Florian Weimer
0 siblings, 1 reply; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-11 19:15 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha
On 11/05/2020 09:38, Szabolcs Nagy wrote:
> The 05/08/2020 14:44, Adhemerval Zanella via Libc-alpha wrote:
>> On 30/04/2020 14:45, Szabolcs Nagy wrote:
>>> +++ b/sysdeps/aarch64/sysdep.h
>>> @@ -35,6 +35,16 @@
>>>
>>> #define PTR_SIZE (1<<PTR_LOG_SIZE)
>>>
>>> +/* Strip pointer authentication code from pointer p. */
>>> +#define XPAC(p) ({ \
>>> + register void *__ra asm ("x30") = (p); \
>>> + asm ("hint 7 // xpaclri" : "+r"(__ra)); \
>>> + __ra;})
>>> +
>>> +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
>>> +#undef RETURN_ADDRESS
>>> +#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
>>> +
>>
>> Maybe use a inline function instead?
>
> macro seems more reliable to me than always_inline
> when poking at __builtin_return_address and x30,
> but i'm not against always_inline if that's
> considered better.
I would prefer a static inline unless a macro is really required
(either due some compiler limitation or bug).
>
> i'd prefer separate xpac (since it can be used
> not just with __builtin_return_address e.g. for
> stored code address in jmpbuf, which currently
> uses ptrmangling)
Ack.
>
>> #ifndef __ASSEMBLER__
>> # include <sys/cdefs.h>
>
> what is cdefs.h for?
The __always_inline macro.
>
>> /* Strip pointer authentication code from pointer p. */
>> static __always_inline void *
>> return_address (unsigned int n)
>> {
>> register void *ra asm ("x30") = __builtin_return_address (n);
>> asm ("hint 7 // xpaclri" : "+r" (ra));
>> return ra;
>> }
>>
>> /* This is needed when glibc is built with -mbranch-protection=pac-ret. */
>> # undef RETURN_ADDRESS
>> # define RETURN_ADDRESS(n) return_address (n)
>> #endif
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-05-11 19:15 ` Adhemerval Zanella
@ 2020-05-11 19:21 ` Florian Weimer
2020-05-11 20:13 ` Adhemerval Zanella
0 siblings, 1 reply; 48+ messages in thread
From: Florian Weimer @ 2020-05-11 19:21 UTC (permalink / raw)
To: Adhemerval Zanella via Libc-alpha
* Adhemerval Zanella via Libc-alpha:
> On 11/05/2020 09:38, Szabolcs Nagy wrote:
>> The 05/08/2020 14:44, Adhemerval Zanella via Libc-alpha wrote:
>>> On 30/04/2020 14:45, Szabolcs Nagy wrote:
>>>> +++ b/sysdeps/aarch64/sysdep.h
>>>> @@ -35,6 +35,16 @@
>>>>
>>>> #define PTR_SIZE (1<<PTR_LOG_SIZE)
>>>>
>>>> +/* Strip pointer authentication code from pointer p. */
>>>> +#define XPAC(p) ({ \
>>>> + register void *__ra asm ("x30") = (p); \
>>>> + asm ("hint 7 // xpaclri" : "+r"(__ra)); \
>>>> + __ra;})
>>>> +
>>>> +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
>>>> +#undef RETURN_ADDRESS
>>>> +#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
>>>> +
>>>
>>> Maybe use a inline function instead?
>>
>> macro seems more reliable to me than always_inline
>> when poking at __builtin_return_address and x30,
>> but i'm not against always_inline if that's
>> considered better.
>
> I would prefer a static inline unless a macro is really required
> (either due some compiler limitation or bug).
I think __builtin_return_address is ill-defined: Does the frame count
that vanishes due to inlining?
So it's probably a case similar to alloca, where a macro has to be
used.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-04-30 17:45 ` [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
2020-05-08 17:44 ` Adhemerval Zanella
@ 2020-05-11 19:22 ` Florian Weimer
2020-05-11 20:45 ` Adhemerval Zanella
2020-05-12 8:42 ` Szabolcs Nagy
1 sibling, 2 replies; 48+ messages in thread
From: Florian Weimer @ 2020-05-11 19:22 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, Sudakshina Das
* Szabolcs Nagy:
> +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
> +#undef RETURN_ADDRESS
> +#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
This looks suspicious. Is __builtin_return_address ever useful
without the decoding? If not, why doesn't GCC emit the PAC removal
itself?
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-05-11 19:21 ` Florian Weimer
@ 2020-05-11 20:13 ` Adhemerval Zanella
2020-05-11 20:18 ` Florian Weimer
0 siblings, 1 reply; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-11 20:13 UTC (permalink / raw)
To: Florian Weimer, Adhemerval Zanella via Libc-alpha
On 11/05/2020 16:21, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
>
>> On 11/05/2020 09:38, Szabolcs Nagy wrote:
>>> The 05/08/2020 14:44, Adhemerval Zanella via Libc-alpha wrote:
>>>> On 30/04/2020 14:45, Szabolcs Nagy wrote:
>>>>> +++ b/sysdeps/aarch64/sysdep.h
>>>>> @@ -35,6 +35,16 @@
>>>>>
>>>>> #define PTR_SIZE (1<<PTR_LOG_SIZE)
>>>>>
>>>>> +/* Strip pointer authentication code from pointer p. */
>>>>> +#define XPAC(p) ({ \
>>>>> + register void *__ra asm ("x30") = (p); \
>>>>> + asm ("hint 7 // xpaclri" : "+r"(__ra)); \
>>>>> + __ra;})
>>>>> +
>>>>> +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
>>>>> +#undef RETURN_ADDRESS
>>>>> +#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
>>>>> +
>>>>
>>>> Maybe use a inline function instead?
>>>
>>> macro seems more reliable to me than always_inline
>>> when poking at __builtin_return_address and x30,
>>> but i'm not against always_inline if that's
>>> considered better.
>>
>> I would prefer a static inline unless a macro is really required
>> (either due some compiler limitation or bug).
>
> I think __builtin_return_address is ill-defined: Does the frame count
> that vanishes due to inlining?
>
> So it's probably a case similar to alloca, where a macro has to be
> used.
This is at least what documentation states [1]:
"When inlining the expected behavior is that the function returns the address
of the function that is returned to. To work around this behavior use the
noinline function attribute."
[1] https://gcc.gnu.org/onlinedocs/gcc/Return-Address.html
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-05-11 20:13 ` Adhemerval Zanella
@ 2020-05-11 20:18 ` Florian Weimer
0 siblings, 0 replies; 48+ messages in thread
From: Florian Weimer @ 2020-05-11 20:18 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: Adhemerval Zanella via Libc-alpha, Szabolcs Nagy
* Adhemerval Zanella:
> On 11/05/2020 16:21, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>>
>>> On 11/05/2020 09:38, Szabolcs Nagy wrote:
>>>> The 05/08/2020 14:44, Adhemerval Zanella via Libc-alpha wrote:
>>>>> On 30/04/2020 14:45, Szabolcs Nagy wrote:
>>>>>> +++ b/sysdeps/aarch64/sysdep.h
>>>>>> @@ -35,6 +35,16 @@
>>>>>>
>>>>>> #define PTR_SIZE (1<<PTR_LOG_SIZE)
>>>>>>
>>>>>> +/* Strip pointer authentication code from pointer p. */
>>>>>> +#define XPAC(p) ({ \
>>>>>> + register void *__ra asm ("x30") = (p); \
>>>>>> + asm ("hint 7 // xpaclri" : "+r"(__ra)); \
>>>>>> + __ra;})
>>>>>> +
>>>>>> +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
>>>>>> +#undef RETURN_ADDRESS
>>>>>> +#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
>>>>>> +
>>>>>
>>>>> Maybe use a inline function instead?
>>>>
>>>> macro seems more reliable to me than always_inline
>>>> when poking at __builtin_return_address and x30,
>>>> but i'm not against always_inline if that's
>>>> considered better.
>>>
>>> I would prefer a static inline unless a macro is really required
>>> (either due some compiler limitation or bug).
>>
>> I think __builtin_return_address is ill-defined: Does the frame count
>> that vanishes due to inlining?
>>
>> So it's probably a case similar to alloca, where a macro has to be
>> used.
>
> This is at least what documentation states [1]:
>
> "When inlining the expected behavior is that the function returns the address
> of the function that is returned to. To work around this behavior use the
> noinline function attribute."
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Return-Address.html
Hmm, okay. It's still weird not to count those frames, but at least
it's documented.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-05-11 19:22 ` Florian Weimer
@ 2020-05-11 20:45 ` Adhemerval Zanella
2020-05-12 8:42 ` Szabolcs Nagy
1 sibling, 0 replies; 48+ messages in thread
From: Adhemerval Zanella @ 2020-05-11 20:45 UTC (permalink / raw)
To: libc-alpha
On 11/05/2020 16:22, Florian Weimer wrote:
> * Szabolcs Nagy:
>
>> +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
>> +#undef RETURN_ADDRESS
>> +#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
>
> This looks suspicious. Is __builtin_return_address ever useful
> without the decoding? If not, why doesn't GCC emit the PAC removal
> itself?
>
Kernel seems to be working on same assumptions [1], and I would say
the builtin works with the assumption it should return the return
address as is in current frame state. Maybe extend gcc should
extern __builtin_extract_return_addr to remove the PAC bits?
[1] https://patchwork.kernel.org/patch/11195099/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-05-11 19:22 ` Florian Weimer
2020-05-11 20:45 ` Adhemerval Zanella
@ 2020-05-12 8:42 ` Szabolcs Nagy
1 sibling, 0 replies; 48+ messages in thread
From: Szabolcs Nagy @ 2020-05-12 8:42 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Sudakshina Das
The 05/11/2020 21:22, Florian Weimer wrote:
> * Szabolcs Nagy:
>
> > +/* This is needed when glibc is built with -mbranch-protection=pac-ret. */
> > +#undef RETURN_ADDRESS
> > +#define RETURN_ADDRESS(n) XPAC(__builtin_return_address(n))
>
> This looks suspicious. Is __builtin_return_address ever useful
> without the decoding? If not, why doesn't GCC emit the PAC removal
> itself?
the only user that needs it is libgcc because it
has dwarf info about the stack pointer and pac
state so it can authenticate the return address,
but normally that's not available (and code that
cares about this would need pac-specific changes).
i consider this a major bug in the current pac-ret
implementation that makes it unusable in existing
software (it's unreasonable to recommend XPAC for
__builtin_return_address users, they will have to
disable pac-ret, but there is no easy way to test
for pac-ret or to disable it without disabling other
things), but not everybody agrees with me on this.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94891
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2020-05-12 8:42 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 17:34 [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
2020-04-30 17:37 ` [PATCH 01/12] elf.h: Add PT_GNU_PROPERTY Szabolcs Nagy
2020-05-07 14:49 ` Adhemerval Zanella
2020-04-30 17:37 ` [PATCH 02/12] elf.h: add aarch64 property definitions Szabolcs Nagy
2020-05-07 14:50 ` Adhemerval Zanella
2020-04-30 17:39 ` [PATCH 03/12] aarch64: Add BTI landing pads to assembly code Szabolcs Nagy
2020-05-07 16:55 ` Adhemerval Zanella
2020-05-11 11:38 ` Szabolcs Nagy
2020-05-11 19:13 ` Adhemerval Zanella
2020-04-30 17:40 ` [PATCH 04/12] aarch64: Rename place holder .S files to .c Szabolcs Nagy
2020-05-07 18:29 ` Adhemerval Zanella
2020-04-30 17:41 ` [PATCH 05/12] aarch64: fix swapcontext for BTI Szabolcs Nagy
2020-05-07 18:42 ` Adhemerval Zanella
2020-04-30 17:42 ` [PATCH 06/12] aarch64: fix RTLD_START " Szabolcs Nagy
2020-05-07 18:49 ` Adhemerval Zanella
2020-05-07 19:24 ` Szabolcs Nagy
2020-05-07 19:55 ` Adhemerval Zanella
2020-05-07 20:14 ` Szabolcs Nagy
2020-05-07 20:20 ` Adhemerval Zanella
2020-04-30 17:42 ` [PATCH 07/12] aarch64: fix syscalls " Szabolcs Nagy
2020-05-07 19:40 ` Adhemerval Zanella
2020-05-11 11:46 ` Szabolcs Nagy
2020-04-30 17:43 ` [PATCH 08/12] Rewrite abi-note.S in C Szabolcs Nagy
2020-04-30 20:07 ` Zack Weinberg
2020-05-01 9:23 ` Szabolcs Nagy
2020-05-01 14:07 ` Zack Weinberg
2020-04-30 17:44 ` [PATCH 09/12] aarch64: support BTI enabled binaries Szabolcs Nagy
2020-05-07 21:07 ` Adhemerval Zanella
2020-05-11 11:04 ` Szabolcs Nagy
2020-05-11 18:38 ` Adhemerval Zanella
2020-04-30 17:44 ` [PATCH 10/12] aarch64: Add pac-ret support to asm files Szabolcs Nagy
2020-05-08 16:59 ` Adhemerval Zanella
2020-05-11 8:27 ` Szabolcs Nagy
2020-05-11 18:39 ` Adhemerval Zanella
2020-04-30 17:45 ` [PATCH 11/12] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
2020-05-08 17:44 ` Adhemerval Zanella
2020-05-11 12:38 ` Szabolcs Nagy
2020-05-11 19:15 ` Adhemerval Zanella
2020-05-11 19:21 ` Florian Weimer
2020-05-11 20:13 ` Adhemerval Zanella
2020-05-11 20:18 ` Florian Weimer
2020-05-11 19:22 ` Florian Weimer
2020-05-11 20:45 ` Adhemerval Zanella
2020-05-12 8:42 ` Szabolcs Nagy
2020-04-30 17:45 ` [PATCH 12/12] aarch64: Configure option to build glibc with branch protection Szabolcs Nagy
2020-04-30 19:02 ` Joseph Myers
2020-05-08 17:53 ` Adhemerval Zanella
2020-05-04 11:27 ` [PATCH 00/12] aarch64: branch protection support Szabolcs Nagy
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).