* [PATCH v6 00/14] aarch64: branch protection support
@ 2020-07-01 14:37 Szabolcs Nagy
2020-07-01 14:37 ` [PATCH v6 01/14] Rewrite abi-note.S in C Szabolcs Nagy
` (13 more replies)
0 siblings, 14 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:37 UTC (permalink / raw)
To: libc-alpha
Indirect branch target identification (BTI, armv8.5-a) and return
address signing using pointer authentication (PAC-RET, armv8.3-a)
can be used for security hardening against some control flow hijack
attacks.
In gcc these are exposed via -mbranch-protection=bti+pac-ret which
is the same as -mbranch-protection=standard and gcc can be configured
via --enable-standard-branch-protection to use them by default.
BTI requires libc support: it is an opt-in feature per ELF module
via a GNU property note that the dynamic linker has to check and
mprotect the executable pages with PROT_BTI. And libc objects that
are statically linked into user binaries must be BTI compatible
for the GNU property note to be present. (The property note is
handled by linux for static linked executables and for the ld.so.)
PAC-RET does not require libc runtime support, but, just like BTI,
it can be used in libc binaries for security hardening.
There are unresolved GCC PAC-RET and BTI issues, but it is possible
to support GCC 10 even without those fixed so this patch set includes
two PAC-RET patches that are GCC bug workarounds and outlined atomics
in libgcc in current GCC 10 is not BTI compatible so for testing
'CC=gcc -mno-outline-atomics' was used. User code will likely need a
fixed GCC for deploying branch protection. The PAC-RET related GCC
discussion is at
https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547402.html
The HWCAP_BTI and PROT_BTI values depend on linux changes that
are scheduled for the Linux 5.8 release.
I plan to commit this set to glibc 2.32.
The patches are also available in the nsz/pacbti-v6 branch.
v6:
- reordered two patches: renaming empty .S to .c and BTI fix for .S.
- the BTI fix for .S now fixes some .S that are conditionally empty
(and thus conditionally missed BTI marking)
- added a new patch to catch BTI incompatible objects at link time.
- merged the PT_GNU_PROPERTY and PT_NOTE rtld fixes (from H.J.Lu).
v5:
- split the BTI runtime enablement patch (07) and added
the PT_NOTE handling cleanup patch from H.J.Lu.
- PATCH 07: rtld changes for PT_GNU_PROPERTY handling.
- PATCH 08: rtld changes to cleanup PT_NOTE handling:
i changed the PHDR processing to scan backward.
- PATCH 09: updated the property handling code.
v4:
- changed plan not to wait for final resolution on gcc issues.
gcc-10 can be made to work.
- the elf.h changes are now committed.
- added Reviewed-by annotations.
- PATCH 01: use ElfW(Nhdr). (this has been sent already on its own).
- PATCH 02: use #define HAVE_AARCH64_BTI 0 (and not #undef).
- PATCH 03: use #if HAVE_AARCH64_BTI (and not #ifdef).
- PATCH 07: use errno in _dl_signal_error and not EINVAL.
- PATCH 07: more comment about the second pass over program headers.
- PATCH 08: use #define HAVE_AARCH64_PAC_RET 0.
- PATCH 09: use #if HAVE_AARCH64_PAC_RET.
- PATCH 10: _mcount patch is written so it's backportable.
no Reviewed-by because this changed significantly.
- PATCH 11: strip_pac is moved to PATCH 10
no Reviewed-by because this changed significantly.
- PATCH 12: new patch: news entry.
v3:
- instead of END_FILE add note in sysdep.h.
- dropped the syscall template patch (END_FILE is not needed).
- PATCH 05: remove END_FILE macros.
- PATCH 05: clarify the GNU_PROPERTY macro and related defines.
- PATCH 09: separate hook for PT_GNU_PROPERTY handling.
- PATCH 09: modified rtld.c and dl-load.c accordingly.
- PATCH 09: rename linkmap->bti_guarded to linkmap->bti.
- PATCH 13: new patch, update _mcount for pac-ret.
- fixed TODOs except for the last two patches, which are written
for current gcc behaviour.
- I'm waiting for a review of PATCH 03 and welcome comments on
the rest of the set, which i consider done unless there are
changes on the gcc or linux side.
v2:
- removed --enable-branch-protection-standard configure option,
branch protection in glibc is enabled based on the compiler default.
- GNU property notes are disabled if compiler/linker has no support.
- pac-ret is enabled based on compiler defaults.
- PATCH 03: cleaner csu/abi-note.c and fix arm/abi-note.S.
- PATCH 04: new (bti config check).
- PATCH 09: drop the umount2 change.
- PATCH 10: use bool instead of int.
- PATCH 10: fix code style and comments.
- PATCH 10: add linux version requirement to description.
- PATCH 11: new (pac-ret config check).
- PATCH 12: only use pac-ret if HAVE_AARCH64_PAC_RET.
- PATCH 12: fix pac-ret use in dl-trampoline.S.
- PATCH 13: use static inline instead of macro, update description.
- addressed some of the reviews from Adhemerval, the remaining ones
are marked as TODO in the descriptions and will require another
test run or agreement on the design.
Sudakshina Das (2):
aarch64: Add BTI support to assembly files
aarch64: enable BTI at runtime
Szabolcs Nagy (12):
Rewrite abi-note.S in C.
aarch64: configure test for BTI support
aarch64: Rename place holder .S files to .c
aarch64: fix swapcontext for BTI
aarch64: fix RTLD_START for BTI
rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling
aarch64: ensure objects are BTI compatible
aarch64: configure check for pac-ret code generation
aarch64: Add pac-ret support to assembly files
aarch64: fix pac-ret support in _mcount
aarch64: redefine RETURN_ADDRESS to strip PAC
aarch64: add NEWS entry about branch protection support
NEWS | 12 +++
config.h.in | 6 ++
csu/{abi-note.S => abi-note.c} | 23 +++--
elf/dl-load.c | 94 +++++++++++++++++--
elf/rtld.c | 14 ++-
sysdeps/aarch64/Makefile | 12 +++
.../aarch64/{bsd-_setjmp.S => bsd-_setjmp.c} | 0
.../aarch64/{bsd-setjmp.S => bsd-setjmp.c} | 0
sysdeps/aarch64/configure | 83 ++++++++++++++++
sysdeps/aarch64/configure.ac | 41 ++++++++
sysdeps/aarch64/crti.S | 10 ++
sysdeps/aarch64/crtn.S | 8 ++
sysdeps/aarch64/dl-bti.c | 54 +++++++++++
sysdeps/aarch64/dl-machine.h | 5 +-
sysdeps/aarch64/dl-prop.h | 63 +++++++++++++
sysdeps/aarch64/dl-tlsdesc.S | 11 +++
sysdeps/aarch64/dl-trampoline.S | 20 ++++
sysdeps/aarch64/linkmap.h | 3 +
sysdeps/aarch64/machine-gmon.h | 3 +-
sysdeps/aarch64/{memmove.S => memmove.c} | 0
sysdeps/aarch64/multiarch/memset_emag.S | 2 +
sysdeps/aarch64/multiarch/memset_falkor.S | 1 +
sysdeps/aarch64/multiarch/memset_generic.S | 2 +
sysdeps/aarch64/multiarch/rtld-memset.S | 2 +
sysdeps/aarch64/start.S | 1 +
sysdeps/aarch64/sysdep.h | 58 +++++++++++-
sysdeps/arm/abi-note.S | 8 --
sysdeps/generic/dl-prop.h | 23 +++--
sysdeps/generic/ldsodefs.h | 4 +
sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 ++++++
.../unix/sysv/linux/aarch64/cpu-features.c | 3 +
.../unix/sysv/linux/aarch64/cpu-features.h | 2 +
sysdeps/unix/sysv/linux/aarch64/swapcontext.S | 14 ++-
sysdeps/x86/dl-prop.h | 47 ++--------
35 files changed, 574 insertions(+), 87 deletions(-)
rename csu/{abi-note.S => abi-note.c} (90%)
rename sysdeps/aarch64/{bsd-_setjmp.S => bsd-_setjmp.c} (100%)
rename sysdeps/aarch64/{bsd-setjmp.S => bsd-setjmp.c} (100%)
create mode 100644 sysdeps/aarch64/dl-bti.c
create mode 100644 sysdeps/aarch64/dl-prop.h
rename sysdeps/aarch64/{memmove.S => memmove.c} (100%)
delete mode 100644 sysdeps/arm/abi-note.S
create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 01/14] Rewrite abi-note.S in C.
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
@ 2020-07-01 14:37 ` Szabolcs Nagy
2020-07-01 14:41 ` H.J. Lu
2020-07-01 14:38 ` [PATCH v6 02/14] aarch64: configure test for BTI support Szabolcs Nagy
` (12 subsequent siblings)
13 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:37 UTC (permalink / raw)
To: libc-alpha
Using C code allows the compiler to add target specific object file
markings based on CFLAGS.
The arm specific abi-note.S is removed and similar object file fix
up will be avoided on AArch64 with standard branch-prtection.
---
csu/{abi-note.S => abi-note.c} | 23 +++++++++++++----------
sysdeps/arm/abi-note.S | 8 --------
2 files changed, 13 insertions(+), 18 deletions(-)
rename csu/{abi-note.S => abi-note.c} (90%)
delete mode 100644 sysdeps/arm/abi-note.S
diff --git a/csu/abi-note.S b/csu/abi-note.c
similarity index 90%
rename from csu/abi-note.S
rename to csu/abi-note.c
index 2b4b5f8824..db195c7ab7 100644
--- a/csu/abi-note.S
+++ b/csu/abi-note.c
@@ -53,6 +53,8 @@ offset length contents
identify the earliest release of that OS that supports this ABI.
See abi-tags (top level) for details. */
+#include <link.h>
+#include <stdint.h>
#include <config.h>
#include <abi-tag.h> /* OS-specific ABI tag value */
@@ -60,13 +62,14 @@ offset length contents
name begins with `.note' and creates a PT_NOTE program header entry
pointing at it. */
- .section ".note.ABI-tag", "a"
- .p2align 2
- .long 1f - 0f /* name length */
- .long 3f - 2f /* data length */
- .long 1 /* note type */
-0: .asciz "GNU" /* vendor name */
-1: .p2align 2
-2: .long __ABI_TAG_OS /* note data: the ABI tag */
- .long __ABI_TAG_VERSION
-3: .p2align 2 /* pad out section */
+__attribute__ ((used, aligned (4), section (".note.ABI-tag")))
+static const struct
+{
+ ElfW(Nhdr) nhdr;
+ char name[4];
+ int32_t desc[4];
+} __abi_tag = {
+ { .n_namesz = 4, .n_descsz = 16, .n_type = 1 },
+ "GNU",
+ { __ABI_TAG_OS, __ABI_TAG_VERSION }
+};
diff --git a/sysdeps/arm/abi-note.S b/sysdeps/arm/abi-note.S
deleted file mode 100644
index 07bd4c4619..0000000000
--- a/sysdeps/arm/abi-note.S
+++ /dev/null
@@ -1,8 +0,0 @@
-/* Tag_ABI_align8_preserved: This code preserves 8-byte
- alignment in any callee. */
- .eabi_attribute 25, 1
-/* Tag_ABI_align8_needed: This code may require 8-byte alignment from
- the caller. */
- .eabi_attribute 24, 1
-
-#include <csu/abi-note.S>
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 02/14] aarch64: configure test for BTI support
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
2020-07-01 14:37 ` [PATCH v6 01/14] Rewrite abi-note.S in C Szabolcs Nagy
@ 2020-07-01 14:38 ` Szabolcs Nagy
2020-07-06 14:11 ` Adhemerval Zanella
2020-07-01 14:38 ` [PATCH v6 03/14] aarch64: Rename place holder .S files to .c Szabolcs Nagy
` (11 subsequent siblings)
13 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:38 UTC (permalink / raw)
To: libc-alpha
Check BTI support in the compiler and linker. The check also
requires READELF that understands the BTI GNU property note.
It is expected to succeed with gcc >=gcc-9 configured with
--enable-standard-branch-protection and binutils >=binutils-2.33.
Note: passing -mbranch-protection=bti in CFLAGS when building glibc
may not be enough to get a glibc that supports BTI because crtbegin*
and crtend* provided by the compiler needs to be BTI compatible too.
---
config.h.in | 3 +++
sysdeps/aarch64/configure | 42 ++++++++++++++++++++++++++++++++++++
sysdeps/aarch64/configure.ac | 19 ++++++++++++++++
3 files changed, 64 insertions(+)
diff --git a/config.h.in b/config.h.in
index 831eca2fe1..67169e5d01 100644
--- a/config.h.in
+++ b/config.h.in
@@ -109,6 +109,9 @@
/* AArch64 big endian ABI */
#undef HAVE_AARCH64_BE
+/* AArch64 BTI support enabled. */
+#define HAVE_AARCH64_BTI 0
+
/* C-SKY ABI version. */
#undef CSKYABI
diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
index 5bd355a691..70477a7fa5 100644
--- a/sysdeps/aarch64/configure
+++ b/sysdeps/aarch64/configure
@@ -172,3 +172,45 @@ else
config_vars="$config_vars
default-abi = lp64"
fi
+
+# Only consider BTI supported if -mbranch-protection=bti is
+# on by default in the compiler and the linker produces
+# binaries with GNU property notes in PT_GNU_PROPERTY segment.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for BTI support" >&5
+$as_echo_n "checking for BTI support... " >&6; }
+if ${libc_cv_aarch64_bti+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat > conftest.c <<EOF
+void foo (void) { }
+EOF
+ libc_cv_aarch64_bti=no
+ if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; } \
+ && { ac_try='$READELF -lW conftest.so | grep -q GNU_PROPERTY'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; } \
+ && { ac_try='$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; }
+ then
+ libc_cv_aarch64_bti=yes
+ fi
+ rm -rf conftest.*
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
+$as_echo "$libc_cv_aarch64_bti" >&6; }
+if test $libc_cv_aarch64_bti = yes; then
+ $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
+
+fi
diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
index 7851dd4dac..798f494740 100644
--- a/sysdeps/aarch64/configure.ac
+++ b/sysdeps/aarch64/configure.ac
@@ -20,3 +20,22 @@ if test $libc_cv_aarch64_be = yes; then
else
LIBC_CONFIG_VAR([default-abi], [lp64])
fi
+
+# Only consider BTI supported if -mbranch-protection=bti is
+# on by default in the compiler and the linker produces
+# binaries with GNU property notes in PT_GNU_PROPERTY segment.
+AC_CACHE_CHECK([for BTI support], [libc_cv_aarch64_bti], [dnl
+ cat > conftest.c <<EOF
+void foo (void) { }
+EOF
+ libc_cv_aarch64_bti=no
+ if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c]) \
+ && AC_TRY_COMMAND([$READELF -lW conftest.so | grep -q GNU_PROPERTY]) \
+ && AC_TRY_COMMAND([$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"])
+ then
+ libc_cv_aarch64_bti=yes
+ fi
+ rm -rf conftest.*])
+if test $libc_cv_aarch64_bti = yes; then
+ AC_DEFINE(HAVE_AARCH64_BTI)
+fi
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 03/14] aarch64: Rename place holder .S files to .c
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
2020-07-01 14:37 ` [PATCH v6 01/14] Rewrite abi-note.S in C Szabolcs Nagy
2020-07-01 14:38 ` [PATCH v6 02/14] aarch64: configure test for BTI support Szabolcs Nagy
@ 2020-07-01 14:38 ` Szabolcs Nagy
2020-07-01 14:38 ` [PATCH v6 04/14] aarch64: Add BTI support to assembly files Szabolcs Nagy
` (10 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:38 UTC (permalink / raw)
To: libc-alpha
The compiler can add required elf markings based on CFLAGS
but the assembler cannot, so using C code for empty files
creates less of a maintenance problem.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
sysdeps/aarch64/{bsd-_setjmp.S => bsd-_setjmp.c} | 0
sysdeps/aarch64/{bsd-setjmp.S => bsd-setjmp.c} | 0
sysdeps/aarch64/{memmove.S => memmove.c} | 0
3 files changed, 0 insertions(+), 0 deletions(-)
rename sysdeps/aarch64/{bsd-_setjmp.S => bsd-_setjmp.c} (100%)
rename sysdeps/aarch64/{bsd-setjmp.S => bsd-setjmp.c} (100%)
rename sysdeps/aarch64/{memmove.S => memmove.c} (100%)
diff --git a/sysdeps/aarch64/bsd-_setjmp.S b/sysdeps/aarch64/bsd-_setjmp.c
similarity index 100%
rename from sysdeps/aarch64/bsd-_setjmp.S
rename to sysdeps/aarch64/bsd-_setjmp.c
diff --git a/sysdeps/aarch64/bsd-setjmp.S b/sysdeps/aarch64/bsd-setjmp.c
similarity index 100%
rename from sysdeps/aarch64/bsd-setjmp.S
rename to sysdeps/aarch64/bsd-setjmp.c
diff --git a/sysdeps/aarch64/memmove.S b/sysdeps/aarch64/memmove.c
similarity index 100%
rename from sysdeps/aarch64/memmove.S
rename to sysdeps/aarch64/memmove.c
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 04/14] aarch64: Add BTI support to assembly files
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (2 preceding siblings ...)
2020-07-01 14:38 ` [PATCH v6 03/14] aarch64: Rename place holder .S files to .c Szabolcs Nagy
@ 2020-07-01 14:38 ` Szabolcs Nagy
2020-07-03 16:19 ` Szabolcs Nagy
2020-07-01 14:38 ` [PATCH v6 05/14] aarch64: fix swapcontext for BTI Szabolcs Nagy
` (9 subsequent siblings)
13 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:38 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
From: Sudakshina Das <sudi.das@arm.com>
To enable building glibc with branch protection, assembly code
needs BTI landing pads and ELF object file markings in the form
of a GNU property note.
The landing pads are unconditionally added to all functions that
may be indirectly called. When the code segment is not mapped
with PROT_BTI these instructions are nops. They are kept in the
code when BTI is not supported so that the layout of performance
critical code is unchanged across configurations.
The GNU property notes are only added when there is support for
BTI in the toolchain, because old binutils does not handle the
notes right. (Does not know how to merge them nor to put them in
PT_GNU_PROPERTY segment instead of PT_NOTE, and some versions
of binutils emit warnings about the unknown GNU property. In
such cases the produced libc binaries would not have valid
ELF marking so BTI would not be enabled.)
Note: functions using ENTRY or ENTRY_ALIGN now start with an
additional BTI c, so alignment of the following code changes,
but ENTRY_ALIGN_AND_PAD was fixed so there is no change to the
existing code layout. Some string functions may need to be
tuned for optimal performance after this commit.
Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
sysdeps/aarch64/crti.S | 2 ++
sysdeps/aarch64/crtn.S | 2 ++
sysdeps/aarch64/dl-tlsdesc.S | 3 ++
sysdeps/aarch64/dl-trampoline.S | 2 ++
sysdeps/aarch64/multiarch/memset_emag.S | 2 ++
sysdeps/aarch64/multiarch/memset_falkor.S | 1 +
sysdeps/aarch64/multiarch/memset_generic.S | 2 ++
sysdeps/aarch64/multiarch/rtld-memset.S | 2 ++
sysdeps/aarch64/start.S | 1 +
sysdeps/aarch64/sysdep.h | 34 +++++++++++++++++++++-
10 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
index 1728eac37a..c346bcad72 100644
--- a/sysdeps/aarch64/crti.S
+++ b/sysdeps/aarch64/crti.S
@@ -75,6 +75,7 @@ call_weak_fn:
.hidden _init
.type _init, %function
_init:
+ BTI_C
stp x29, x30, [sp, -16]!
mov x29, sp
#if PREINIT_FUNCTION_WEAK
@@ -89,5 +90,6 @@ _init:
.hidden _fini
.type _fini, %function
_fini:
+ BTI_C
stp x29, x30, [sp, -16]!
mov x29, sp
diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
index c3e97cc449..0c1ef112c2 100644
--- a/sysdeps/aarch64/crtn.S
+++ b/sysdeps/aarch64/crtn.S
@@ -37,6 +37,8 @@
/* crtn.S puts function epilogues in the .init and .fini sections
corresponding to the prologues in crti.S. */
+#include <sysdep.h>
+
.section .init,"ax",%progbits
ldp x29, x30, [sp], 16
RET
diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
index 557ad1d505..9d96c8632a 100644
--- a/sysdeps/aarch64/dl-tlsdesc.S
+++ b/sysdeps/aarch64/dl-tlsdesc.S
@@ -74,6 +74,7 @@
cfi_startproc
.align 2
_dl_tlsdesc_return:
+ BTI_C
DELOUSE (0)
ldr PTR_REG (0), [x0, #PTR_SIZE]
RET
@@ -95,6 +96,7 @@ _dl_tlsdesc_return:
cfi_startproc
.align 2
_dl_tlsdesc_undefweak:
+ BTI_C
str x1, [sp, #-16]!
cfi_adjust_cfa_offset (16)
DELOUSE (0)
@@ -142,6 +144,7 @@ _dl_tlsdesc_undefweak:
cfi_startproc
.align 2
_dl_tlsdesc_dynamic:
+ BTI_C
DELOUSE (0)
/* Save just enough registers to support fast path, if we fall
diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
index 94e965c096..2cbfa81434 100644
--- a/sysdeps/aarch64/dl-trampoline.S
+++ b/sysdeps/aarch64/dl-trampoline.S
@@ -35,6 +35,7 @@
cfi_startproc
.align 2
_dl_runtime_resolve:
+ BTI_C
/* AArch64 we get called with:
ip0 &PLTGOT[2]
ip1 temp(dl resolver entry point)
@@ -126,6 +127,7 @@ _dl_runtime_resolve:
cfi_startproc
.align 2
_dl_runtime_profile:
+ BTI_C
/* AArch64 we get called with:
ip0 &PLTGOT[2]
ip1 temp(dl resolver entry point)
diff --git a/sysdeps/aarch64/multiarch/memset_emag.S b/sysdeps/aarch64/multiarch/memset_emag.S
index c4d3533c14..3c2e9d2903 100644
--- a/sysdeps/aarch64/multiarch/memset_emag.S
+++ b/sysdeps/aarch64/multiarch/memset_emag.S
@@ -17,6 +17,8 @@
License along with the GNU C Library. If not, see
<https://www.gnu.org/licenses/>. */
+#include <sysdep.h>
+
#if IS_IN (libc)
# define MEMSET __memset_emag
diff --git a/sysdeps/aarch64/multiarch/memset_falkor.S b/sysdeps/aarch64/multiarch/memset_falkor.S
index 54fd5abffb..154527398f 100644
--- a/sysdeps/aarch64/multiarch/memset_falkor.S
+++ b/sysdeps/aarch64/multiarch/memset_falkor.S
@@ -17,6 +17,7 @@
License along with the GNU C Library. If not, see
<https://www.gnu.org/licenses/>. */
+#include <sysdep.h>
#include <memset-reg.h>
/* Reading dczid_el0 is expensive on falkor so move it into the ifunc
diff --git a/sysdeps/aarch64/multiarch/memset_generic.S b/sysdeps/aarch64/multiarch/memset_generic.S
index 46c5329cdf..d746d1d00c 100644
--- a/sysdeps/aarch64/multiarch/memset_generic.S
+++ b/sysdeps/aarch64/multiarch/memset_generic.S
@@ -17,6 +17,8 @@
License along with the GNU C Library. If not, see
<https://www.gnu.org/licenses/>. */
+#include <sysdep.h>
+
#if IS_IN (libc)
# define MEMSET __memset_generic
/* Add a hidden definition for use within libc.so. */
diff --git a/sysdeps/aarch64/multiarch/rtld-memset.S b/sysdeps/aarch64/multiarch/rtld-memset.S
index 44bc479411..f9845bdd62 100644
--- a/sysdeps/aarch64/multiarch/rtld-memset.S
+++ b/sysdeps/aarch64/multiarch/rtld-memset.S
@@ -17,6 +17,8 @@
License along with the GNU C Library. If not, see
<https://www.gnu.org/licenses/>. */
+#include <sysdep.h>
+
#if IS_IN (rtld)
# define MEMSET memset
# include <sysdeps/aarch64/memset.S>
diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
index d96cf57e2d..75393e1c18 100644
--- a/sysdeps/aarch64/start.S
+++ b/sysdeps/aarch64/start.S
@@ -46,6 +46,7 @@
.globl _start
.type _start,#function
_start:
+ BTI_C
/* Create an initial frame with 0 LR and FP */
mov x29, #0
mov x30, #0
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 604c489170..4664329d3a 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -41,12 +41,42 @@
#define ASM_SIZE_DIRECTIVE(name) .size name,.-name
+/* Branch Target Identitication support. */
+#define BTI_C hint 34
+#define BTI_J hint 36
+
+/* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code. */
+#define FEATURE_1_AND 0xc0000000
+#define FEATURE_1_BTI 1
+#define FEATURE_1_PAC 2
+
+/* Add a NT_GNU_PROPERTY_TYPE_0 note. */
+#define GNU_PROPERTY(type, value) \
+ .section .note.gnu.property, "a"; \
+ .p2align 3; \
+ .word 4; \
+ .word 16; \
+ .word 5; \
+ .asciz "GNU"; \
+ .word type; \
+ .word 4; \
+ .word value; \
+ .word 0; \
+ .text
+
+/* Add GNU property note with the supported features to all asm code
+ where sysdep.h is included. */
+#if HAVE_AARCH64_BTI
+GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
+#endif
+
/* Define an entry point visible from C. */
#define ENTRY(name) \
.globl C_SYMBOL_NAME(name); \
.type C_SYMBOL_NAME(name),%function; \
.align 4; \
C_LABEL(name) \
+ BTI_C; \
cfi_startproc; \
CALL_MCOUNT
@@ -56,6 +86,7 @@
.type C_SYMBOL_NAME(name),%function; \
.p2align align; \
C_LABEL(name) \
+ BTI_C; \
cfi_startproc; \
CALL_MCOUNT
@@ -68,10 +99,11 @@
.globl C_SYMBOL_NAME(name); \
.type C_SYMBOL_NAME(name),%function; \
.p2align align; \
- .rep padding; \
+ .rep padding - 1; /* -1 for bti c. */ \
nop; \
.endr; \
C_LABEL(name) \
+ BTI_C; \
cfi_startproc; \
CALL_MCOUNT
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 05/14] aarch64: fix swapcontext for BTI
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (3 preceding siblings ...)
2020-07-01 14:38 ` [PATCH v6 04/14] aarch64: Add BTI support to assembly files Szabolcs Nagy
@ 2020-07-01 14:38 ` Szabolcs Nagy
2020-07-01 14:39 ` [PATCH v6 06/14] aarch64: fix RTLD_START " Szabolcs Nagy
` (8 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:38 UTC (permalink / raw)
To: libc-alpha
setcontext returns to the specified context via an indirect jump,
so there should be a BTI j.
In case of getcontext (and all other returns_twice functions) the
compiler adds BTI j at the call site, but swapcontext is a normal
c call that is currently not handled specially by the compiler.
So we change swapcontext such that the saved context returns to a
local address that has BTI j and then swapcontext returns to the
caller via a normal RET. For this we save the original return
address in the slot for x1 of the context because x1 need not be
preserved by swapcontext but it is restored when the context saved
by swapcontext is resumed.
The alternative fix (which is done on x86) would make swapcontext
special in the compiler so BTI j is emitted at call sites, on
x86 there is an indirect_return attribute for this, on AArch64
we would have to use returns_twice. It was decided against because
such fix may need user code updates: the attribute has to be added
when swapcontext is called via a function pointer and it breaks
always_inline functions with swapcontext.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
sysdeps/unix/sysv/linux/aarch64/swapcontext.S | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/aarch64/swapcontext.S b/sysdeps/unix/sysv/linux/aarch64/swapcontext.S
index d30c543e6f..f8c66f0ef0 100644
--- a/sysdeps/unix/sysv/linux/aarch64/swapcontext.S
+++ b/sysdeps/unix/sysv/linux/aarch64/swapcontext.S
@@ -28,8 +28,12 @@
.text
ENTRY(__swapcontext)
DELOUSE (0)
- /* Set the value returned when swapcontext() returns in this context. */
- str xzr, [x0, oX0 + 0 * SZREG]
+ /* Set the value returned when swapcontext() returns in this context.
+ And set up x1 to become the return address of the caller, so we
+ can return there with a normal RET instead of an indirect jump. */
+ stp xzr, x30, [x0, oX0 + 0 * SZREG]
+ /* Arrange the oucp context to return to 2f. */
+ adr x30, 2f
stp x18, x19, [x0, oX0 + 18 * SZREG]
stp x20, x21, [x0, oX0 + 20 * SZREG]
@@ -97,5 +101,11 @@ ENTRY(__swapcontext)
1:
b C_SYMBOL_NAME(__syscall_error)
+2:
+ /* The oucp context is restored here via an indirect branch,
+ x1 must be restored too which has the real return address. */
+ BTI_J
+ mov x30, x1
+ RET
PSEUDO_END (__swapcontext)
weak_alias (__swapcontext, swapcontext)
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 06/14] aarch64: fix RTLD_START for BTI
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (4 preceding siblings ...)
2020-07-01 14:38 ` [PATCH v6 05/14] aarch64: fix swapcontext for BTI Szabolcs Nagy
@ 2020-07-01 14:39 ` Szabolcs Nagy
2020-07-01 14:39 ` [PATCH v6 07/14] rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling Szabolcs Nagy
` (7 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:39 UTC (permalink / raw)
To: libc-alpha
Tailcalls must use x16 or x17 for the indirect branch instruction
to be compatible with code that uses BTI c at function entries.
(Other forms of indirect branches can only land on BTI j.)
Also added a BTI c at the ELF entry point of rtld, this is not
strictly necessary since the kernel does not use indirect branch
to get there, but it seems safest once building glibc itself with
BTI is supported.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
sysdeps/aarch64/dl-machine.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
index db3335e5ad..70b9ed3925 100644
--- a/sysdeps/aarch64/dl-machine.h
+++ b/sysdeps/aarch64/dl-machine.h
@@ -125,6 +125,8 @@ elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
.globl _dl_start_user \n\
.type _dl_start_user, %function \n\
_start: \n\
+ // bti c \n\
+ hint 34 \n\
mov " PTR "0, " PTR_SP " \n\
bl _dl_start \n\
// returns user entry point in x0 \n\
@@ -178,7 +180,8 @@ _dl_start_user: \n\
adrp x0, _dl_fini \n\
add " PTR "0, " PTR "0, #:lo12:_dl_fini \n\
// jump to the user_s entry point \n\
- br x21 \n\
+ mov x16, x21 \n\
+ br x16 \n\
");
#define elf_machine_type_class(type) \
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 07/14] rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (5 preceding siblings ...)
2020-07-01 14:39 ` [PATCH v6 06/14] aarch64: fix RTLD_START " Szabolcs Nagy
@ 2020-07-01 14:39 ` Szabolcs Nagy
2020-07-06 16:11 ` Adhemerval Zanella
2020-07-01 14:39 ` [PATCH v6 08/14] aarch64: enable BTI at runtime Szabolcs Nagy
` (6 subsequent siblings)
13 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:39 UTC (permalink / raw)
To: libc-alpha
Add generic code to handle PT_GNU_PROPERTY notes. Invalid
content is ignored, _dl_process_pt_gnu_property is always called
after PT_LOAD segments are mapped and it has no failure modes.
Currently only one NT_GNU_PROPERTY_TYPE_0 note is handled, which
contains target specific properties: the _dl_process_gnu_property
hook is called for each property.
The old _dl_process_pt_note and _rtld_process_pt_note differ in how
the program header is read. The old _dl_process_pt_note is called
before PT_LOAD segments are mapped and _rtld_process_pt_note is called
after PT_LOAD segments are mapped. The old _rtld_process_pt_note is
removed and _dl_process_pt_note is always called after PT_LOAD
segments are mapped and now it has no failure modes.
The program headers are scanned backwards so that PT_NOTE can be
skipped if PT_GNU_PROPERTY exists.
Co-Authored-By: H.J. Lu <hjl.tools@gmail.com>
---
elf/dl-load.c | 94 ++++++++++++++++++++++++++++++++++----
elf/rtld.c | 14 ++++--
sysdeps/generic/dl-prop.h | 23 +++++-----
sysdeps/generic/ldsodefs.h | 4 ++
sysdeps/x86/dl-prop.h | 47 +++----------------
5 files changed, 118 insertions(+), 64 deletions(-)
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 06f2ba7264..32c74f79ef 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -853,6 +853,77 @@ lose (int code, int fd, const char *name, char *realname, struct link_map *l,
}
+/* Process PT_GNU_PROPERTY program header PH in module L after
+ PT_LOAD segments are mapped. Only one NT_GNU_PROPERTY_TYPE_0
+ note is handled which contains processor specific properties. */
+
+void
+_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
+{
+ const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
+ const ElfW(Addr) size = ph->p_memsz;
+ const ElfW(Addr) align = ph->p_align;
+
+ /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
+ 32-bit objects and to 8 bytes in 64-bit objects. Skip notes
+ with incorrect alignment. */
+ if (align != (__ELF_NATIVE_CLASS / 8))
+ return;
+
+ const ElfW(Addr) start = (ElfW(Addr)) note;
+ unsigned int 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;
+
+ /* Target specific property processing. */
+ if (_dl_process_gnu_property(l, type, datasz, ptr) == 0)
+ return;
+
+ /* Check the next property item. */
+ ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
+ }
+ while ((ptr_end - ptr) >= 8);
+
+ /* Only handle one NT_GNU_PROPERTY_TYPE_0. */
+ return;
+ }
+
+ note = ((const void *) note
+ + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
+ align));
+ }
+}
+
+
/* Map in the shared object NAME, actually located in REALNAME, and already
opened on FD. */
@@ -1145,14 +1216,6 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
l->l_relro_addr = ph->p_vaddr;
l->l_relro_size = ph->p_memsz;
break;
-
- case PT_NOTE:
- if (_dl_process_pt_note (l, ph, fd, fbp))
- {
- errstring = N_("cannot process note segment");
- goto call_lose;
- }
- break;
}
if (__glibc_unlikely (nloadcmds == 0))
@@ -1188,6 +1251,21 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
maplength, has_holes, loader);
if (__glibc_unlikely (errstring != NULL))
goto call_lose;
+
+ /* Process program headers again after load segments are mapped in
+ case processing requires accessing those segments. Scan program
+ headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY
+ exits. */
+ for (ph = &phdr[l->l_phnum]; ph != phdr; --ph)
+ switch (ph[-1].p_type)
+ {
+ case PT_NOTE:
+ _dl_process_pt_note (l, &ph[-1]);
+ break;
+ case PT_GNU_PROPERTY:
+ _dl_process_pt_gnu_property (l, &ph[-1]);
+ break;
+ }
}
if (l->l_ld == 0)
diff --git a/elf/rtld.c b/elf/rtld.c
index 882b070cc0..f4c2602d65 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1507,11 +1507,17 @@ of this helper program; chances are you did not intend to run this program.\n\
main_map->l_relro_addr = ph->p_vaddr;
main_map->l_relro_size = ph->p_memsz;
break;
-
+ }
+ /* Process program headers again, but scan them backwards so
+ that PT_NOTE can be skipped if PT_GNU_PROPERTY exits. */
+ for (ph = &phdr[phnum]; ph != phdr; --ph)
+ switch (ph[-1].p_type)
+ {
case PT_NOTE:
- if (_rtld_process_pt_note (main_map, ph))
- _dl_error_printf ("\
-ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
+ _dl_process_pt_note (main_map, &ph[-1]);
+ break;
+ case PT_GNU_PROPERTY:
+ _dl_process_pt_gnu_property (main_map, &ph[-1]);
break;
}
diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
index 6b0f2aa95a..f1cf576fe3 100644
--- a/sysdeps/generic/dl-prop.h
+++ b/sysdeps/generic/dl-prop.h
@@ -20,11 +20,11 @@
#define _DL_PROP_H
/* The following functions are used by the dynamic loader and the
- dlopen machinery to process PT_NOTE entries in the binary or
- shared object. The notes can be used to change the behaviour of
- the loader, and as such offer a flexible mechanism for hooking in
- various checks related to ABI tags or implementing "flag day" ABI
- transitions. */
+ dlopen machinery to process PT_NOTE and PT_GNU_PROPERTY entries in
+ the binary or shared object. The notes can be used to change the
+ behaviour of the loader, and as such offer a flexible mechanism
+ for hooking in various checks related to ABI tags or implementing
+ "flag day" ABI transitions. */
static inline void __attribute__ ((always_inline))
_rtld_main_check (struct link_map *m, const char *program)
@@ -36,17 +36,16 @@ _dl_open_check (struct link_map *m)
{
}
-#ifdef FILEBUF_SIZE
-static inline int __attribute__ ((always_inline))
-_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
- int fd, struct filebuf *fbp)
+static inline void __attribute__ ((always_inline))
+_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
{
- return 0;
}
-#endif
+/* Called for each property in the NT_GNU_PROPERTY_TYPE_0 note of L,
+ processing of the properties continues until this returns 0. */
static inline int __attribute__ ((always_inline))
-_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
+_dl_process_gnu_property (struct link_map *l, uint32_t type, uint32_t datasz,
+ void *data)
{
return 0;
}
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index d08b97a5ef..c525ffa12c 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -910,6 +910,10 @@ extern void _dl_setup_hash (struct link_map *map) attribute_hidden;
extern void _dl_rtld_di_serinfo (struct link_map *loader,
Dl_serinfo *si, bool counting);
+/* Process PT_GNU_PROPERTY program header PH in module L after
+ PT_LOAD segments are mapped. */
+void _dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph);
+
/* Search loaded objects' symbol tables for a definition of the symbol
referred to by UNDEF. *SYM is the symbol table entry containing the
diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index 516f88ea80..89911e19e2 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -19,8 +19,6 @@
#ifndef _DL_PROP_H
#define _DL_PROP_H
-#include <not-cancel.h>
-
extern void _dl_cet_check (struct link_map *, const char *)
attribute_hidden;
extern void _dl_cet_open_check (struct link_map *)
@@ -146,48 +144,17 @@ _dl_process_cet_property_note (struct link_map *l,
#endif
}
-#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)
+static inline void __attribute__ ((unused))
+_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
{
-# if CET_ENABLED
- 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_malloced = malloc (size);
- note = note_malloced;
- }
- if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
- {
- if (note_malloced)
- free (note_malloced);
- return -1;
- }
- }
-
- _dl_process_cet_property_note (l, note, size, ph->p_align);
- if (note_malloced)
- free (note_malloced);
-# endif
- return 0;
+ const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
+ _dl_process_cet_property_note (l, note, ph->p_memsz, ph->p_align);
}
-#endif
-static inline int __attribute__ ((unused))
-_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
+static inline int __attribute__ ((always_inline))
+_dl_process_gnu_property (struct link_map *l, uint32_t type, uint32_t datasz,
+ void *data)
{
- const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
- _dl_process_cet_property_note (l, note, ph->p_memsz, ph->p_align);
return 0;
}
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 08/14] aarch64: enable BTI at runtime
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (6 preceding siblings ...)
2020-07-01 14:39 ` [PATCH v6 07/14] rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling Szabolcs Nagy
@ 2020-07-01 14:39 ` Szabolcs Nagy
2020-07-06 17:28 ` Adhemerval Zanella
2020-07-11 15:58 ` Richard Henderson
2020-07-01 14:39 ` [PATCH v6 09/14] aarch64: ensure objects are BTI compatible Szabolcs Nagy
` (5 subsequent siblings)
13 siblings, 2 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:39 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
From: Sudakshina Das <sudi.das@arm.com>
Binaries can opt-in to using BTI via an ELF object file marking.
The dynamic linker has to then mprotect the executable segments
with PROT_BTI. In case of static linked executables or in case
of the dynamic linker itself, PROT_BTI protection is done by the
operating system.
On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
the properties of a binary because PT_NOTE can be unreliable with
old linkers (old linkers just append the notes of input objects
together and add them to the output without checking them for
consistency which means multiple incompatible GNU property notes
can be present in PT_NOTE).
BTI property is handled in the loader even if glibc is not built
with BTI support, so in theory user code can be BTI protected
independently of glibc. In practice though user binaries are not
marked with the BTI property if glibc has no support because the
static linked libc objects (crt files, libc_nonshared.a) are
unmarked.
This patch relies on Linux userspace API that is not yet in a
linux release but in v5.8-rc1 so scheduled to be in Linux 5.8.
Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
sysdeps/aarch64/Makefile | 4 ++
sysdeps/aarch64/dl-bti.c | 54 ++++++++++++++++
sysdeps/aarch64/dl-prop.h | 63 +++++++++++++++++++
sysdeps/aarch64/linkmap.h | 3 +
sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 +++++++++
.../unix/sysv/linux/aarch64/cpu-features.c | 3 +
.../unix/sysv/linux/aarch64/cpu-features.h | 2 +
8 files changed, 161 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/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..3c92377cc8
--- /dev/null
+++ b/sysdeps/aarch64/dl-bti.c
@@ -0,0 +1,54 @@
+/* AArch64 BTI functions.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <unistd.h>
+#include <errno.h>
+#include <libintl.h>
+#include <ldsodefs.h>
+
+static int
+enable_bti (struct link_map *map, const char *program)
+{
+ const ElfW(Phdr) *phdr;
+ unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
+
+ for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
+ if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
+ {
+ ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
+ ElfW(Addr) len = phdr->p_memsz;
+ if (__mprotect ((void *) start, len, prot) < 0)
+ {
+ if (program)
+ _dl_fatal_printf ("%s: mprotect failed to turn on BTI\n",
+ map->l_name);
+ else
+ _dl_signal_error (errno, map->l_name, "dlopen",
+ N_("mprotect failed to turn on BTI"));
+ }
+ }
+ return 0;
+}
+
+/* Enable BTI for L if required. */
+
+void
+_dl_bti_check (struct link_map *l, const char *program)
+{
+ if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti)
+ enable_bti (l, program);
+}
diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
new file mode 100644
index 0000000000..b0785bda83
--- /dev/null
+++ b/sysdeps/aarch64/dl-prop.h
@@ -0,0 +1,63 @@
+/* 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
+
+extern void _dl_bti_check (struct link_map *, const char *)
+ attribute_hidden;
+
+static inline void __attribute__ ((always_inline))
+_rtld_main_check (struct link_map *m, const char *program)
+{
+ _dl_bti_check (m, program);
+}
+
+static inline void __attribute__ ((always_inline))
+_dl_open_check (struct link_map *m)
+{
+ _dl_bti_check (m, NULL);
+}
+
+static inline void __attribute__ ((always_inline))
+_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
+{
+}
+
+static inline int
+_dl_process_gnu_property (struct link_map *l, uint32_t type, uint32_t datasz,
+ void *data)
+{
+ if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
+ {
+ /* Stop if the property note is ill-formed. */
+ if (datasz != 4)
+ return 0;
+
+ unsigned int feature_1 = *(unsigned int *) data;
+ if (feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)
+ l->l_mach.bti = true;
+
+ /* Stop if we processed the property note. */
+ return 0;
+ }
+ /* Continue. */
+ return 1;
+}
+
+#endif /* _DL_PROP_H */
diff --git a/sysdeps/aarch64/linkmap.h b/sysdeps/aarch64/linkmap.h
index 943a9ee9e4..847a03ace2 100644
--- a/sysdeps/aarch64/linkmap.h
+++ b/sysdeps/aarch64/linkmap.h
@@ -16,8 +16,11 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
+#include <stdbool.h>
+
struct link_map_machine
{
ElfW(Addr) plt; /* Address of .plt */
void *tlsdesc_table; /* Address of TLS descriptor hash table. */
+ bool bti; /* Branch Target Identification is enabled. */
};
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
index 4ee14b4208..af90d8a626 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
@@ -72,3 +72,4 @@
#define HWCAP2_BF16 (1 << 14)
#define HWCAP2_DGH (1 << 15)
#define HWCAP2_RNG (1 << 16)
+#define HWCAP2_BTI (1 << 17)
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
new file mode 100644
index 0000000000..ecae046344
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
@@ -0,0 +1,31 @@
+/* Definitions for POSIX memory map interface. Linux/AArch64 version.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_MMAN_H
+# error "Never use <bits/mman.h> directly; include <sys/mman.h> instead."
+#endif
+
+/* AArch64 specific definitions, should be in sync with
+ arch/arm64/include/uapi/asm/mman.h. */
+
+#define PROT_BTI 0x10
+
+#include <bits/mman-map-flags-generic.h>
+
+/* Include generic Linux declarations. */
+#include <bits/mman-linux.h>
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
index 896c588fee..b9ab827aca 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
@@ -83,4 +83,7 @@ init_cpu_features (struct cpu_features *cpu_features)
if ((dczid & DCZID_DZP_MASK) == 0)
cpu_features->zva_size = 4 << (dczid & DCZID_BS_MASK);
+
+ /* Check if BTI is supported. */
+ cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
}
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
index 1389cea1b3..a81f186ec2 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
@@ -20,6 +20,7 @@
#define _CPU_FEATURES_AARCH64_H
#include <stdint.h>
+#include <stdbool.h>
#define MIDR_PARTNUM_SHIFT 4
#define MIDR_PARTNUM_MASK (0xfff << MIDR_PARTNUM_SHIFT)
@@ -64,6 +65,7 @@ struct cpu_features
{
uint64_t midr_el1;
unsigned zva_size;
+ bool bti;
};
#endif /* _CPU_FEATURES_AARCH64_H */
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 09/14] aarch64: ensure objects are BTI compatible
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (7 preceding siblings ...)
2020-07-01 14:39 ` [PATCH v6 08/14] aarch64: enable BTI at runtime Szabolcs Nagy
@ 2020-07-01 14:39 ` Szabolcs Nagy
2020-07-06 17:37 ` Adhemerval Zanella
2020-07-01 14:40 ` [PATCH v6 10/14] aarch64: configure check for pac-ret code generation Szabolcs Nagy
` (4 subsequent siblings)
13 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:39 UTC (permalink / raw)
To: libc-alpha
When glibc is built with branch protection (i.e. with a gcc configured
with --enable-standard-branch-protection), all glibc binaries should
be BTI compatible and marked as such.
It is easy to link BTI incompatible objects by accident and this is
silent currently which is usually not the expectation, so this is
changed into a link error. (There is no linker flag for failing on
BTI incompatible inputs so all warnings are turned into fatal errors
outside the test system when building glibc with branch protection.)
Unfortunately, outlined atomic functions are not BTI compatible in
libgcc (PR libgcc/96001), so to build glibc with current gcc use
'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon
and then glibc can be built and tested without such workarounds.
---
sysdeps/aarch64/Makefile | 8 ++++++++
sysdeps/aarch64/configure | 2 ++
sysdeps/aarch64/configure.ac | 1 +
3 files changed, 11 insertions(+)
diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
index 5ae8b082b0..7f82fc055e 100644
--- a/sysdeps/aarch64/Makefile
+++ b/sysdeps/aarch64/Makefile
@@ -1,5 +1,13 @@
long-double-fcts = yes
+ifeq (yes,$(aarch64-bti))
+# Mark linker output BTI compatible, it warns on non-BTI inputs.
+sysdep-LDFLAGS += -Wl,-z,force-bti
+# Make warnings fatal outside the test system.
+LDFLAGS-lib.so += -Wl,--fatal-warnings
+LDFLAGS-rtld += -Wl,-z,force-bti,--fatal-warnings
+endif
+
ifeq ($(subdir),elf)
sysdep-dl-routines += dl-bti
endif
diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
index 70477a7fa5..c637540436 100644
--- a/sysdeps/aarch64/configure
+++ b/sysdeps/aarch64/configure
@@ -210,6 +210,8 @@ EOF
fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
$as_echo "$libc_cv_aarch64_bti" >&6; }
+config_vars="$config_vars
+aarch64-bti = $libc_cv_aarch64_bti"
if test $libc_cv_aarch64_bti = yes; then
$as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
index 798f494740..2c2817514d 100644
--- a/sysdeps/aarch64/configure.ac
+++ b/sysdeps/aarch64/configure.ac
@@ -36,6 +36,7 @@ EOF
libc_cv_aarch64_bti=yes
fi
rm -rf conftest.*])
+LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti])
if test $libc_cv_aarch64_bti = yes; then
AC_DEFINE(HAVE_AARCH64_BTI)
fi
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 10/14] aarch64: configure check for pac-ret code generation
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (8 preceding siblings ...)
2020-07-01 14:39 ` [PATCH v6 09/14] aarch64: ensure objects are BTI compatible Szabolcs Nagy
@ 2020-07-01 14:40 ` Szabolcs Nagy
2020-07-01 14:40 ` [PATCH v6 11/14] aarch64: Add pac-ret support to assembly files Szabolcs Nagy
` (3 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:40 UTC (permalink / raw)
To: libc-alpha
Return address signing requires unwinder support, which is
present in libgcc since >=gcc-7, however due to bugs the
support may be broken in <gcc-10 (and similarly there may
be issues in custom unwinders), so pac-ret is not always
safe to use. So in assembly code glibc should only use
pac-ret if the compiler uses it too. Unfortunately there
is no predefined feature macro for it set by the compiler
so pac-ret is inferred from the code generation.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
config.h.in | 3 +++
sysdeps/aarch64/configure | 39 ++++++++++++++++++++++++++++++++++++
sysdeps/aarch64/configure.ac | 21 +++++++++++++++++++
3 files changed, 63 insertions(+)
diff --git a/config.h.in b/config.h.in
index 67169e5d01..7921917ad2 100644
--- a/config.h.in
+++ b/config.h.in
@@ -112,6 +112,9 @@
/* AArch64 BTI support enabled. */
#define HAVE_AARCH64_BTI 0
+/* AArch64 PAC-RET code generation is enabled. */
+#define HAVE_AARCH64_PAC_RET 0
+
/* C-SKY ABI version. */
#undef CSKYABI
diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
index c637540436..ac3cf6fd36 100644
--- a/sysdeps/aarch64/configure
+++ b/sysdeps/aarch64/configure
@@ -216,3 +216,42 @@ if test $libc_cv_aarch64_bti = yes; then
$as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
fi
+
+# Check if glibc is built with return address signing, i.e.
+# if -mbranch-protection=pac-ret is on. We need this because
+# pac-ret relies on unwinder support so it's not safe to use
+# it in assembly code unconditionally, but there is no
+# feature test macro for it in gcc.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking if pac-ret is enabled" >&5
+$as_echo_n "checking if pac-ret is enabled... " >&6; }
+if ${libc_cv_aarch64_pac_ret+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat > conftest.c <<EOF
+int bar (void);
+int foo (void) { return bar () + 1; }
+EOF
+ libc_cv_aarch64_pac_ret=no
+ if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -S -o conftest.s conftest.c'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; } \
+ && { ac_try='grep -q -E '\''(hint( | )+25|paciasp)'\'' conftest.s'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; }
+ then
+ libc_cv_aarch64_pac_ret=yes
+ fi
+ rm -rf conftest.*
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_pac_ret" >&5
+$as_echo "$libc_cv_aarch64_pac_ret" >&6; }
+if test $libc_cv_aarch64_pac_ret = yes; then
+ $as_echo "#define HAVE_AARCH64_PAC_RET 1" >>confdefs.h
+
+fi
diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
index 2c2817514d..8b042d6d05 100644
--- a/sysdeps/aarch64/configure.ac
+++ b/sysdeps/aarch64/configure.ac
@@ -40,3 +40,24 @@ LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti])
if test $libc_cv_aarch64_bti = yes; then
AC_DEFINE(HAVE_AARCH64_BTI)
fi
+
+# Check if glibc is built with return address signing, i.e.
+# if -mbranch-protection=pac-ret is on. We need this because
+# pac-ret relies on unwinder support so it's not safe to use
+# it in assembly code unconditionally, but there is no
+# feature test macro for it in gcc.
+AC_CACHE_CHECK([if pac-ret is enabled], [libc_cv_aarch64_pac_ret], [dnl
+ cat > conftest.c <<EOF
+int bar (void);
+int foo (void) { return bar () + 1; }
+EOF
+ libc_cv_aarch64_pac_ret=no
+ if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS -S -o conftest.s conftest.c]) \
+ && AC_TRY_COMMAND([grep -q -E '\''(hint( | )+25|paciasp)'\'' conftest.s])
+ then
+ libc_cv_aarch64_pac_ret=yes
+ fi
+ rm -rf conftest.*])
+if test $libc_cv_aarch64_pac_ret = yes; then
+ AC_DEFINE(HAVE_AARCH64_PAC_RET)
+fi
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 11/14] aarch64: Add pac-ret support to assembly files
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (9 preceding siblings ...)
2020-07-01 14:40 ` [PATCH v6 10/14] aarch64: configure check for pac-ret code generation Szabolcs Nagy
@ 2020-07-01 14:40 ` Szabolcs Nagy
2020-07-01 14:40 ` [PATCH v6 12/14] aarch64: fix pac-ret support in _mcount Szabolcs Nagy
` (2 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:40 UTC (permalink / raw)
To: libc-alpha
Use return address signing in assembly files for functions that save
LR when pac-ret is enabled in the compiler.
The GNU property note for PAC-RET is not meaningful to the dynamic
linker so it is not strictly required, but it may be used to track
the security property of binaries. (The PAC-RET property is only set
if BTI is set too because BTI implies working GNU property support.)
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
sysdeps/aarch64/crti.S | 8 ++++++++
sysdeps/aarch64/crtn.S | 6 ++++++
sysdeps/aarch64/dl-tlsdesc.S | 8 ++++++++
sysdeps/aarch64/dl-trampoline.S | 18 ++++++++++++++++++
sysdeps/aarch64/sysdep.h | 8 +++++++-
5 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
index c346bcad72..c486e9ca44 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 HAVE_AARCH64_PAC_RET
+ PACIASP
+#else
BTI_C
+#endif
stp x29, x30, [sp, -16]!
mov x29, sp
#if PREINIT_FUNCTION_WEAK
@@ -90,6 +94,10 @@ _init:
.hidden _fini
.type _fini, %function
_fini:
+#if HAVE_AARCH64_PAC_RET
+ PACIASP
+#else
BTI_C
+#endif
stp x29, x30, [sp, -16]!
mov x29, sp
diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
index 0c1ef112c2..d21c21c203 100644
--- a/sysdeps/aarch64/crtn.S
+++ b/sysdeps/aarch64/crtn.S
@@ -41,8 +41,14 @@
.section .init,"ax",%progbits
ldp x29, x30, [sp], 16
+#if HAVE_AARCH64_PAC_RET
+ AUTIASP
+#endif
RET
.section .fini,"ax",%progbits
ldp x29, x30, [sp], 16
+#if HAVE_AARCH64_PAC_RET
+ AUTIASP
+#endif
RET
diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
index 9d96c8632a..db8a064322 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 HAVE_AARCH64_PAC_RET
+ PACIASP
+ cfi_window_save
+# endif
# define NSAVEXREGPAIRS 8
stp x29, x30, [sp,#-16*NSAVEXREGPAIRS]!
cfi_adjust_cfa_offset (16*NSAVEXREGPAIRS)
@@ -233,6 +237,10 @@ _dl_tlsdesc_dynamic:
cfi_adjust_cfa_offset (-16*NSAVEXREGPAIRS)
cfi_restore (x29)
cfi_restore (x30)
+# if HAVE_AARCH64_PAC_RET
+ AUTIASP
+ cfi_window_save
+# endif
b 1b
cfi_endproc
.size _dl_tlsdesc_dynamic, .-_dl_tlsdesc_dynamic
diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
index 2cbfa81434..794876fffa 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 HAVE_AARCH64_PAC_RET
+ PACIASP
+ cfi_window_save
+# else
BTI_C
+# endif
/* AArch64 we get called with:
ip0 &PLTGOT[2]
ip1 temp(dl resolver entry point)
@@ -239,8 +244,17 @@ _dl_runtime_profile:
cfi_restore(x29)
cfi_restore(x30)
+# if HAVE_AARCH64_PAC_RET
+ add sp, sp, SF_SIZE
+ cfi_adjust_cfa_offset (-SF_SIZE)
+ AUTIASP
+ cfi_window_save
+ add sp, sp, 16
+ cfi_adjust_cfa_offset (-16)
+# else
add sp, sp, SF_SIZE + 16
cfi_adjust_cfa_offset (- SF_SIZE - 16)
+# endif
/* Jump to the newly found address. */
br ip0
@@ -287,6 +301,10 @@ _dl_runtime_profile:
/* LR from within La_aarch64_reg */
ldr lr, [x29, #OFFSET_RG + DL_OFFSET_RG_LR]
cfi_restore(lr)
+# if HAVE_AARCH64_PAC_RET
+ /* Note: LR restored from La_aarch64_reg has no PAC. */
+ cfi_window_save
+# endif
mov sp, x29
cfi_def_cfa_register (sp)
ldr x29, [x29, #0]
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 4664329d3a..500c272745 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -45,6 +45,10 @@
#define BTI_C hint 34
#define BTI_J hint 36
+/* Return address signing support (pac-ret). */
+#define PACIASP hint 25
+#define AUTIASP hint 29
+
/* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code. */
#define FEATURE_1_AND 0xc0000000
#define FEATURE_1_BTI 1
@@ -66,7 +70,9 @@
/* Add GNU property note with the supported features to all asm code
where sysdep.h is included. */
-#if HAVE_AARCH64_BTI
+#if HAVE_AARCH64_BTI && HAVE_AARCH64_PAC_RET
+GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI|FEATURE_1_PAC)
+#elif HAVE_AARCH64_BTI
GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
#endif
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 12/14] aarch64: fix pac-ret support in _mcount
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (10 preceding siblings ...)
2020-07-01 14:40 ` [PATCH v6 11/14] aarch64: Add pac-ret support to assembly files Szabolcs Nagy
@ 2020-07-01 14:40 ` Szabolcs Nagy
2020-07-06 18:33 ` Adhemerval Zanella
2020-07-01 14:40 ` [PATCH v6 13/14] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
2020-07-01 14:41 ` [PATCH v6 14/14] aarch64: add NEWS entry about branch protection support Szabolcs Nagy
13 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:40 UTC (permalink / raw)
To: libc-alpha
Currently gcc -pg -mbranch-protection=pac-ret passes signed return
address to _mcount, so _mcount now has to always strip pac from the
frompc since that's from user code that may be built with pac-ret.
This is gcc PR target/94791: signed pointers should not escape and get
passed across extern call boundaries, since that's an ABI break, but
because existing gcc has this issue we work it around in glibc until
that is resolved. This is compatible with a fixed gcc and it is a nop
on systems without PAuth support. The bug was introduced in gcc-7 with
-msign-return-address=non-leaf|all support which in gcc-9 got renamed
to -mbranch-protection=pac-ret|pac-ret+leaf|standard.
strip_pac uses inline asm instead of __builtin_aarch64_xpaclri since
that is not a documented api and not available in all supported gccs.
---
sysdeps/aarch64/machine-gmon.h | 3 ++-
sysdeps/aarch64/sysdep.h | 11 +++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h
index 730a23b781..a687298b1c 100644
--- a/sysdeps/aarch64/machine-gmon.h
+++ b/sysdeps/aarch64/machine-gmon.h
@@ -27,8 +27,9 @@ static void mcount_internal (u_long frompc, u_long selfpc);
#define _MCOUNT_DECL(frompc, selfpc) \
static inline void mcount_internal (u_long frompc, u_long selfpc)
+/* Note: strip_pac is needed for frompc because of gcc PR target/94791. */
#define MCOUNT \
void __mcount (void *frompc) \
{ \
- mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
+ mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \
}
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 500c272745..2879aeaa5c 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -35,6 +35,17 @@
#define PTR_SIZE (1<<PTR_LOG_SIZE)
+#ifndef __ASSEMBLER__
+/* Strip pointer authentication code from pointer p. */
+static inline void *
+strip_pac (void *p)
+{
+ register void *ra asm ("x30") = (p);
+ asm ("hint 7 // xpaclri" : "+r"(ra));
+ return ra;
+}
+#endif
+
#ifdef __ASSEMBLER__
/* Syntactic details of assembler. */
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 13/14] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (11 preceding siblings ...)
2020-07-01 14:40 ` [PATCH v6 12/14] aarch64: fix pac-ret support in _mcount Szabolcs Nagy
@ 2020-07-01 14:40 ` Szabolcs Nagy
2020-07-06 18:34 ` Adhemerval Zanella
2020-07-01 14:41 ` [PATCH v6 14/14] aarch64: add NEWS entry about branch protection support Szabolcs Nagy
13 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:40 UTC (permalink / raw)
To: libc-alpha
RETURN_ADDRESS is used at several places in glibc to mean a valid
code address of the call site, but with pac-ret it may contain a
pointer authentication code (PAC), so its definition is adjusted.
This is gcc PR target/94891: __builtin_return_address should not
expose signed pointers to user code where it can cause ABI issues.
In glibc RETURN_ADDRESS is only changed if it is built with pac-ret.
There is no detection for the specific gcc issue because it is
hard to test and the additional xpac does not cause problems.
---
sysdeps/aarch64/sysdep.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 2879aeaa5c..48fa8e9e90 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -44,6 +44,13 @@ strip_pac (void *p)
asm ("hint 7 // xpaclri" : "+r"(ra));
return ra;
}
+
+/* This is needed when glibc is built with -mbranch-protection=pac-ret
+ with a gcc that is affected by PR target/94891. */
+# if HAVE_AARCH64_PAC_RET
+# undef RETURN_ADDRESS
+# define RETURN_ADDRESS(n) strip_pac (__builtin_return_address (n))
+# endif
#endif
#ifdef __ASSEMBLER__
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 14/14] aarch64: add NEWS entry about branch protection support
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
` (12 preceding siblings ...)
2020-07-01 14:40 ` [PATCH v6 13/14] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
@ 2020-07-01 14:41 ` Szabolcs Nagy
2020-07-06 18:41 ` Adhemerval Zanella
13 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 14:41 UTC (permalink / raw)
To: libc-alpha
This is a new security feature that relies on architecture
extensions and needs glibc to be built with a gcc configured
with branch protection.
---
NEWS | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/NEWS b/NEWS
index a660fc59a8..7d0ca3f520 100644
--- a/NEWS
+++ b/NEWS
@@ -31,6 +31,18 @@ Major new features:
pthread_attr_getsigmask_np have been added. They allow applications
to specify the signal mask of a thread created with pthread_create.
+* AArch64 now supports standard branch protection security hardening
+ in glibc when it is built with a GCC that is configured with
+ --enable-standard-branch-protection. This includes branch target
+ identification (BTI) and pointer authentication for return addresses
+ (PAC-RET). They require armv8.5-a and armv8.3-a architecture
+ extensions respectively for the protection to be effective,
+ otherwise the used instructions are nops. User code can use PAC-RET
+ without libc support, but BTI requires a libc that is built with BTI
+ support, otherwise runtime objects linked into user code will not be
+ BTI compatible. It is recommended to use GCC 10 or newer when
+ building glibc with branch protection.
+
Deprecated and removed features, and other changes affecting compatibility:
* The deprecated <sys/sysctl.h> header and the sysctl function have been
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 01/14] Rewrite abi-note.S in C.
2020-07-01 14:37 ` [PATCH v6 01/14] Rewrite abi-note.S in C Szabolcs Nagy
@ 2020-07-01 14:41 ` H.J. Lu
2020-07-01 17:31 ` Szabolcs Nagy
0 siblings, 1 reply; 41+ messages in thread
From: H.J. Lu @ 2020-07-01 14:41 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: GNU C Library
On Wed, Jul 1, 2020 at 7:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> Using C code allows the compiler to add target specific object file
> markings based on CFLAGS.
>
> The arm specific abi-note.S is removed and similar object file fix
> up will be avoided on AArch64 with standard branch-prtection.
> ---
> csu/{abi-note.S => abi-note.c} | 23 +++++++++++++----------
> sysdeps/arm/abi-note.S | 8 --------
> 2 files changed, 13 insertions(+), 18 deletions(-)
> rename csu/{abi-note.S => abi-note.c} (90%)
> delete mode 100644 sysdeps/arm/abi-note.S
>
> diff --git a/csu/abi-note.S b/csu/abi-note.c
> similarity index 90%
> rename from csu/abi-note.S
> rename to csu/abi-note.c
> index 2b4b5f8824..db195c7ab7 100644
> --- a/csu/abi-note.S
> +++ b/csu/abi-note.c
> @@ -53,6 +53,8 @@ offset length contents
> identify the earliest release of that OS that supports this ABI.
> See abi-tags (top level) for details. */
>
> +#include <link.h>
> +#include <stdint.h>
> #include <config.h>
> #include <abi-tag.h> /* OS-specific ABI tag value */
>
> @@ -60,13 +62,14 @@ offset length contents
> name begins with `.note' and creates a PT_NOTE program header entry
> pointing at it. */
>
> - .section ".note.ABI-tag", "a"
> - .p2align 2
> - .long 1f - 0f /* name length */
> - .long 3f - 2f /* data length */
> - .long 1 /* note type */
> -0: .asciz "GNU" /* vendor name */
> -1: .p2align 2
> -2: .long __ABI_TAG_OS /* note data: the ABI tag */
> - .long __ABI_TAG_VERSION
> -3: .p2align 2 /* pad out section */
> +__attribute__ ((used, aligned (4), section (".note.ABI-tag")))
> +static const struct
> +{
> + ElfW(Nhdr) nhdr;
> + char name[4];
> + int32_t desc[4];
> +} __abi_tag = {
> + { .n_namesz = 4, .n_descsz = 16, .n_type = 1 },
sizeof "GNU"? and sizeof desc?
> + "GNU",
> + { __ABI_TAG_OS, __ABI_TAG_VERSION }
> +};
> diff --git a/sysdeps/arm/abi-note.S b/sysdeps/arm/abi-note.S
> deleted file mode 100644
> index 07bd4c4619..0000000000
> --- a/sysdeps/arm/abi-note.S
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -/* Tag_ABI_align8_preserved: This code preserves 8-byte
> - alignment in any callee. */
> - .eabi_attribute 25, 1
> -/* Tag_ABI_align8_needed: This code may require 8-byte alignment from
> - the caller. */
> - .eabi_attribute 24, 1
> -
> -#include <csu/abi-note.S>
> --
> 2.17.1
>
--
H.J.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 01/14] Rewrite abi-note.S in C.
2020-07-01 14:41 ` H.J. Lu
@ 2020-07-01 17:31 ` Szabolcs Nagy
2020-07-01 17:43 ` H.J. Lu
0 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-01 17:31 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
The 07/01/2020 07:41, H.J. Lu wrote:
> On Wed, Jul 1, 2020 at 7:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > Using C code allows the compiler to add target specific object file
> > markings based on CFLAGS.
> >
> > The arm specific abi-note.S is removed and similar object file fix
> > up will be avoided on AArch64 with standard branch-prtection.
> > ---
> > csu/{abi-note.S => abi-note.c} | 23 +++++++++++++----------
> > sysdeps/arm/abi-note.S | 8 --------
> > 2 files changed, 13 insertions(+), 18 deletions(-)
> > rename csu/{abi-note.S => abi-note.c} (90%)
> > delete mode 100644 sysdeps/arm/abi-note.S
> >
> > diff --git a/csu/abi-note.S b/csu/abi-note.c
> > similarity index 90%
> > rename from csu/abi-note.S
> > rename to csu/abi-note.c
> > index 2b4b5f8824..db195c7ab7 100644
> > --- a/csu/abi-note.S
> > +++ b/csu/abi-note.c
> > @@ -53,6 +53,8 @@ offset length contents
> > identify the earliest release of that OS that supports this ABI.
> > See abi-tags (top level) for details. */
> >
> > +#include <link.h>
> > +#include <stdint.h>
> > #include <config.h>
> > #include <abi-tag.h> /* OS-specific ABI tag value */
> >
> > @@ -60,13 +62,14 @@ offset length contents
> > name begins with `.note' and creates a PT_NOTE program header entry
> > pointing at it. */
> >
> > - .section ".note.ABI-tag", "a"
> > - .p2align 2
> > - .long 1f - 0f /* name length */
> > - .long 3f - 2f /* data length */
> > - .long 1 /* note type */
> > -0: .asciz "GNU" /* vendor name */
> > -1: .p2align 2
> > -2: .long __ABI_TAG_OS /* note data: the ABI tag */
> > - .long __ABI_TAG_VERSION
> > -3: .p2align 2 /* pad out section */
> > +__attribute__ ((used, aligned (4), section (".note.ABI-tag")))
> > +static const struct
> > +{
> > + ElfW(Nhdr) nhdr;
> > + char name[4];
> > + int32_t desc[4];
> > +} __abi_tag = {
> > + { .n_namesz = 4, .n_descsz = 16, .n_type = 1 },
>
> sizeof "GNU"? and sizeof desc?
i can do
{ .n_namesz = sizeof __abi_tag.name,
.n_descsz = sizeof __abi_tag.desc,
.n_type = 1 },
is that better? (for me the int literal looks
clear in the context and shorter. i don't like
sizeof "GNU" i think that's less clear.)
> > + "GNU",
> > + { __ABI_TAG_OS, __ABI_TAG_VERSION }
> > +};
> > diff --git a/sysdeps/arm/abi-note.S b/sysdeps/arm/abi-note.S
> > deleted file mode 100644
> > index 07bd4c4619..0000000000
> > --- a/sysdeps/arm/abi-note.S
> > +++ /dev/null
> > @@ -1,8 +0,0 @@
> > -/* Tag_ABI_align8_preserved: This code preserves 8-byte
> > - alignment in any callee. */
> > - .eabi_attribute 25, 1
> > -/* Tag_ABI_align8_needed: This code may require 8-byte alignment from
> > - the caller. */
> > - .eabi_attribute 24, 1
> > -
> > -#include <csu/abi-note.S>
> > --
> > 2.17.1
> >
>
>
> --
> H.J.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 01/14] Rewrite abi-note.S in C.
2020-07-01 17:31 ` Szabolcs Nagy
@ 2020-07-01 17:43 ` H.J. Lu
2020-07-02 8:39 ` Szabolcs Nagy
0 siblings, 1 reply; 41+ messages in thread
From: H.J. Lu @ 2020-07-01 17:43 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: GNU C Library
On Wed, Jul 1, 2020 at 10:31 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 07/01/2020 07:41, H.J. Lu wrote:
> > On Wed, Jul 1, 2020 at 7:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > Using C code allows the compiler to add target specific object file
> > > markings based on CFLAGS.
> > >
> > > The arm specific abi-note.S is removed and similar object file fix
> > > up will be avoided on AArch64 with standard branch-prtection.
> > > ---
> > > csu/{abi-note.S => abi-note.c} | 23 +++++++++++++----------
> > > sysdeps/arm/abi-note.S | 8 --------
> > > 2 files changed, 13 insertions(+), 18 deletions(-)
> > > rename csu/{abi-note.S => abi-note.c} (90%)
> > > delete mode 100644 sysdeps/arm/abi-note.S
> > >
> > > diff --git a/csu/abi-note.S b/csu/abi-note.c
> > > similarity index 90%
> > > rename from csu/abi-note.S
> > > rename to csu/abi-note.c
> > > index 2b4b5f8824..db195c7ab7 100644
> > > --- a/csu/abi-note.S
> > > +++ b/csu/abi-note.c
> > > @@ -53,6 +53,8 @@ offset length contents
> > > identify the earliest release of that OS that supports this ABI.
> > > See abi-tags (top level) for details. */
> > >
> > > +#include <link.h>
> > > +#include <stdint.h>
> > > #include <config.h>
> > > #include <abi-tag.h> /* OS-specific ABI tag value */
> > >
> > > @@ -60,13 +62,14 @@ offset length contents
> > > name begins with `.note' and creates a PT_NOTE program header entry
> > > pointing at it. */
> > >
> > > - .section ".note.ABI-tag", "a"
> > > - .p2align 2
> > > - .long 1f - 0f /* name length */
> > > - .long 3f - 2f /* data length */
> > > - .long 1 /* note type */
> > > -0: .asciz "GNU" /* vendor name */
> > > -1: .p2align 2
> > > -2: .long __ABI_TAG_OS /* note data: the ABI tag */
> > > - .long __ABI_TAG_VERSION
> > > -3: .p2align 2 /* pad out section */
> > > +__attribute__ ((used, aligned (4), section (".note.ABI-tag")))
> > > +static const struct
> > > +{
> > > + ElfW(Nhdr) nhdr;
> > > + char name[4];
> > > + int32_t desc[4];
> > > +} __abi_tag = {
> > > + { .n_namesz = 4, .n_descsz = 16, .n_type = 1 },
> >
> > sizeof "GNU"? and sizeof desc?
>
> i can do
>
> { .n_namesz = sizeof __abi_tag.name,
> .n_descsz = sizeof __abi_tag.desc,
> .n_type = 1 },
>
> is that better? (for me the int literal looks
> clear in the context and shorter. i don't like
> sizeof "GNU" i think that's less clear.)
It is better.
--
H.J.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 01/14] Rewrite abi-note.S in C.
2020-07-01 17:43 ` H.J. Lu
@ 2020-07-02 8:39 ` Szabolcs Nagy
0 siblings, 0 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-02 8:39 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
The 07/01/2020 10:43, H.J. Lu wrote:
> On Wed, Jul 1, 2020 at 10:31 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > The 07/01/2020 07:41, H.J. Lu wrote:
> > > sizeof "GNU"? and sizeof desc?
> >
> > i can do
> >
> > { .n_namesz = sizeof __abi_tag.name,
> > .n_descsz = sizeof __abi_tag.desc,
> > .n_type = 1 },
> >
> > is that better? (for me the int literal looks
> > clear in the context and shorter. i don't like
> > sizeof "GNU" i think that's less clear.)
>
> It is better.
fixed and force pushed into nsz/pacbti-v6.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 04/14] aarch64: Add BTI support to assembly files
2020-07-01 14:38 ` [PATCH v6 04/14] aarch64: Add BTI support to assembly files Szabolcs Nagy
@ 2020-07-03 16:19 ` Szabolcs Nagy
0 siblings, 0 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-03 16:19 UTC (permalink / raw)
To: libc-alpha; +Cc: Sudakshina Das
The 07/01/2020 15:38, Szabolcs Nagy wrote:
> /* 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
found a minor issue here: BTI_C should
come after the startproc cfi directive.
i fixed it locally.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 02/14] aarch64: configure test for BTI support
2020-07-01 14:38 ` [PATCH v6 02/14] aarch64: configure test for BTI support Szabolcs Nagy
@ 2020-07-06 14:11 ` Adhemerval Zanella
2020-07-06 18:07 ` Szabolcs Nagy
0 siblings, 1 reply; 41+ messages in thread
From: Adhemerval Zanella @ 2020-07-06 14:11 UTC (permalink / raw)
To: libc-alpha
On 01/07/2020 11:38, Szabolcs Nagy wrote:
> Check BTI support in the compiler and linker. The check also
> requires READELF that understands the BTI GNU property note.
> It is expected to succeed with gcc >=gcc-9 configured with
> --enable-standard-branch-protection and binutils >=binutils-2.33.
>
> Note: passing -mbranch-protection=bti in CFLAGS when building glibc
> may not be enough to get a glibc that supports BTI because crtbegin*
> and crtend* provided by the compiler needs to be BTI compatible too.
If I read correctly this scenario should be covered by the configure
test, right?
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> config.h.in | 3 +++
> sysdeps/aarch64/configure | 42 ++++++++++++++++++++++++++++++++++++
> sysdeps/aarch64/configure.ac | 19 ++++++++++++++++
> 3 files changed, 64 insertions(+)
>
> diff --git a/config.h.in b/config.h.in
> index 831eca2fe1..67169e5d01 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -109,6 +109,9 @@
> /* AArch64 big endian ABI */
> #undef HAVE_AARCH64_BE
>
> +/* AArch64 BTI support enabled. */
> +#define HAVE_AARCH64_BTI 0
> +
> /* C-SKY ABI version. */
> #undef CSKYABI
>
Ok.
> diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> index 5bd355a691..70477a7fa5 100644
> --- a/sysdeps/aarch64/configure
> +++ b/sysdeps/aarch64/configure
> @@ -172,3 +172,45 @@ else
> config_vars="$config_vars
> default-abi = lp64"
> fi
> +
> +# Only consider BTI supported if -mbranch-protection=bti is
> +# on by default in the compiler and the linker produces
> +# binaries with GNU property notes in PT_GNU_PROPERTY segment.
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for BTI support" >&5
> +$as_echo_n "checking for BTI support... " >&6; }
> +if ${libc_cv_aarch64_bti+:} false; then :
> + $as_echo_n "(cached) " >&6
> +else
> + cat > conftest.c <<EOF
> +void foo (void) { }
> +EOF
> + libc_cv_aarch64_bti=no
> + if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; } \
> + && { ac_try='$READELF -lW conftest.so | grep -q GNU_PROPERTY'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; } \
> + && { ac_try='$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; }
> + then
> + libc_cv_aarch64_bti=yes
> + fi
> + rm -rf conftest.*
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
> +$as_echo "$libc_cv_aarch64_bti" >&6; }
> +if test $libc_cv_aarch64_bti = yes; then
> + $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
> +
> +fi
> diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> index 7851dd4dac..798f494740 100644
> --- a/sysdeps/aarch64/configure.ac
> +++ b/sysdeps/aarch64/configure.ac
> @@ -20,3 +20,22 @@ if test $libc_cv_aarch64_be = yes; then
> else
> LIBC_CONFIG_VAR([default-abi], [lp64])
> fi
> +
> +# Only consider BTI supported if -mbranch-protection=bti is
> +# on by default in the compiler and the linker produces
> +# binaries with GNU property notes in PT_GNU_PROPERTY segment.
> +AC_CACHE_CHECK([for BTI support], [libc_cv_aarch64_bti], [dnl
> + cat > conftest.c <<EOF
> +void foo (void) { }
> +EOF
> + libc_cv_aarch64_bti=no
> + if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c]) \
> + && AC_TRY_COMMAND([$READELF -lW conftest.so | grep -q GNU_PROPERTY]) \
> + && AC_TRY_COMMAND([$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"])
> + then
> + libc_cv_aarch64_bti=yes
> + fi
> + rm -rf conftest.*])
> +if test $libc_cv_aarch64_bti = yes; then
> + AC_DEFINE(HAVE_AARCH64_BTI)
> +fi
>
Ok.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 07/14] rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling
2020-07-01 14:39 ` [PATCH v6 07/14] rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling Szabolcs Nagy
@ 2020-07-06 16:11 ` Adhemerval Zanella
0 siblings, 0 replies; 41+ messages in thread
From: Adhemerval Zanella @ 2020-07-06 16:11 UTC (permalink / raw)
To: libc-alpha
On 01/07/2020 11:39, Szabolcs Nagy wrote:
> Add generic code to handle PT_GNU_PROPERTY notes. Invalid
> content is ignored, _dl_process_pt_gnu_property is always called
> after PT_LOAD segments are mapped and it has no failure modes.
> Currently only one NT_GNU_PROPERTY_TYPE_0 note is handled, which
> contains target specific properties: the _dl_process_gnu_property
> hook is called for each property.
>
> The old _dl_process_pt_note and _rtld_process_pt_note differ in how
> the program header is read. The old _dl_process_pt_note is called
> before PT_LOAD segments are mapped and _rtld_process_pt_note is called
> after PT_LOAD segments are mapped. The old _rtld_process_pt_note is
> removed and _dl_process_pt_note is always called after PT_LOAD
> segments are mapped and now it has no failure modes.
>
> The program headers are scanned backwards so that PT_NOTE can be
> skipped if PT_GNU_PROPERTY exists.
>
> Co-Authored-By: H.J. Lu <hjl.tools@gmail.com>
LGTM, with a minor style nit below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> elf/dl-load.c | 94 ++++++++++++++++++++++++++++++++++----
> elf/rtld.c | 14 ++++--
> sysdeps/generic/dl-prop.h | 23 +++++-----
> sysdeps/generic/ldsodefs.h | 4 ++
> sysdeps/x86/dl-prop.h | 47 +++----------------
> 5 files changed, 118 insertions(+), 64 deletions(-)
>
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 06f2ba7264..32c74f79ef 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -853,6 +853,77 @@ lose (int code, int fd, const char *name, char *realname, struct link_map *l,
> }
>
>
> +/* Process PT_GNU_PROPERTY program header PH in module L after
> + PT_LOAD segments are mapped. Only one NT_GNU_PROPERTY_TYPE_0
> + note is handled which contains processor specific properties. */
> +
> +void
> +_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> + const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
> + const ElfW(Addr) size = ph->p_memsz;
> + const ElfW(Addr) align = ph->p_align;
> +
> + /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
> + 32-bit objects and to 8 bytes in 64-bit objects. Skip notes
> + with incorrect alignment. */
> + if (align != (__ELF_NATIVE_CLASS / 8))
> + return;
> +
> + const ElfW(Addr) start = (ElfW(Addr)) note;
> + unsigned int 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;
> +
> + /* Target specific property processing. */
> + if (_dl_process_gnu_property(l, type, datasz, ptr) == 0)
Space before open parenthesis.
> + return;
> +
> + /* Check the next property item. */
> + ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> + }
> + while ((ptr_end - ptr) >= 8);
> +
> + /* Only handle one NT_GNU_PROPERTY_TYPE_0. */
> + return;
> + }
> +
> + note = ((const void *) note
> + + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
> + align));
> + }
> +}
> +
> +
> /* Map in the shared object NAME, actually located in REALNAME, and already
> opened on FD. */
>
> @@ -1145,14 +1216,6 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> l->l_relro_addr = ph->p_vaddr;
> l->l_relro_size = ph->p_memsz;
> break;
> -
> - case PT_NOTE:
> - if (_dl_process_pt_note (l, ph, fd, fbp))
> - {
> - errstring = N_("cannot process note segment");
> - goto call_lose;
> - }
> - break;
> }
>
> if (__glibc_unlikely (nloadcmds == 0))
> @@ -1188,6 +1251,21 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> maplength, has_holes, loader);
> if (__glibc_unlikely (errstring != NULL))
> goto call_lose;
> +
> + /* Process program headers again after load segments are mapped in
> + case processing requires accessing those segments. Scan program
> + headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY
> + exits. */
> + for (ph = &phdr[l->l_phnum]; ph != phdr; --ph)
> + switch (ph[-1].p_type)
> + {
> + case PT_NOTE:
> + _dl_process_pt_note (l, &ph[-1]);
> + break;
> + case PT_GNU_PROPERTY:
> + _dl_process_pt_gnu_property (l, &ph[-1]);
> + break;
> + }
> }
>
> if (l->l_ld == 0)
Ok.
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 882b070cc0..f4c2602d65 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1507,11 +1507,17 @@ of this helper program; chances are you did not intend to run this program.\n\
> main_map->l_relro_addr = ph->p_vaddr;
> main_map->l_relro_size = ph->p_memsz;
> break;
> -
> + }
> + /* Process program headers again, but scan them backwards so
> + that PT_NOTE can be skipped if PT_GNU_PROPERTY exits. */
> + for (ph = &phdr[phnum]; ph != phdr; --ph)
> + switch (ph[-1].p_type)
> + {
> case PT_NOTE:
> - if (_rtld_process_pt_note (main_map, ph))
> - _dl_error_printf ("\
> -ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
> + _dl_process_pt_note (main_map, &ph[-1]);
> + break;
> + case PT_GNU_PROPERTY:
> + _dl_process_pt_gnu_property (main_map, &ph[-1]);
> break;
> }
>
Ok.
> diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
> index 6b0f2aa95a..f1cf576fe3 100644
> --- a/sysdeps/generic/dl-prop.h
> +++ b/sysdeps/generic/dl-prop.h
> @@ -20,11 +20,11 @@
> #define _DL_PROP_H
>
> /* The following functions are used by the dynamic loader and the
> - dlopen machinery to process PT_NOTE entries in the binary or
> - shared object. The notes can be used to change the behaviour of
> - the loader, and as such offer a flexible mechanism for hooking in
> - various checks related to ABI tags or implementing "flag day" ABI
> - transitions. */
> + dlopen machinery to process PT_NOTE and PT_GNU_PROPERTY entries in
> + the binary or shared object. The notes can be used to change the
> + behaviour of the loader, and as such offer a flexible mechanism
> + for hooking in various checks related to ABI tags or implementing
> + "flag day" ABI transitions. */
>
> static inline void __attribute__ ((always_inline))
> _rtld_main_check (struct link_map *m, const char *program)
> @@ -36,17 +36,16 @@ _dl_open_check (struct link_map *m)
> {
> }
>
> -#ifdef FILEBUF_SIZE
> -static inline int __attribute__ ((always_inline))
> -_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
> - int fd, struct filebuf *fbp)
> +static inline void __attribute__ ((always_inline))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> {
> - return 0;
> }
> -#endif
>
> +/* Called for each property in the NT_GNU_PROPERTY_TYPE_0 note of L,
> + processing of the properties continues until this returns 0. */
> static inline int __attribute__ ((always_inline))
> -_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> +_dl_process_gnu_property (struct link_map *l, uint32_t type, uint32_t datasz,
> + void *data)
> {
> return 0;
> }
Ok.
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index d08b97a5ef..c525ffa12c 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -910,6 +910,10 @@ extern void _dl_setup_hash (struct link_map *map) attribute_hidden;
> extern void _dl_rtld_di_serinfo (struct link_map *loader,
> Dl_serinfo *si, bool counting);
>
> +/* Process PT_GNU_PROPERTY program header PH in module L after
> + PT_LOAD segments are mapped. */
> +void _dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph);
> +
>
> /* Search loaded objects' symbol tables for a definition of the symbol
> referred to by UNDEF. *SYM is the symbol table entry containing the
Ok.
> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index 516f88ea80..89911e19e2 100644
> --- a/sysdeps/x86/dl-prop.h
> +++ b/sysdeps/x86/dl-prop.h
> @@ -19,8 +19,6 @@
> #ifndef _DL_PROP_H
> #define _DL_PROP_H
>
> -#include <not-cancel.h>
> -
> extern void _dl_cet_check (struct link_map *, const char *)
> attribute_hidden;
> extern void _dl_cet_open_check (struct link_map *)
> @@ -146,48 +144,17 @@ _dl_process_cet_property_note (struct link_map *l,
> #endif
> }
>
> -#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)
> +static inline void __attribute__ ((unused))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> {
> -# if CET_ENABLED
> - 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_malloced = malloc (size);
> - note = note_malloced;
> - }
> - if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
> - {
> - if (note_malloced)
> - free (note_malloced);
> - return -1;
> - }
> - }
> -
> - _dl_process_cet_property_note (l, note, size, ph->p_align);
> - if (note_malloced)
> - free (note_malloced);
> -# endif
> - return 0;
> + const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
> + _dl_process_cet_property_note (l, note, ph->p_memsz, ph->p_align);
> }
> -#endif
>
> -static inline int __attribute__ ((unused))
> -_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> +static inline int __attribute__ ((always_inline))
> +_dl_process_gnu_property (struct link_map *l, uint32_t type, uint32_t datasz,
> + void *data)
> {
> - const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
> - _dl_process_cet_property_note (l, note, ph->p_memsz, ph->p_align);
> return 0;
> }
>
>
Ok.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 08/14] aarch64: enable BTI at runtime
2020-07-01 14:39 ` [PATCH v6 08/14] aarch64: enable BTI at runtime Szabolcs Nagy
@ 2020-07-06 17:28 ` Adhemerval Zanella
2020-07-11 15:58 ` Richard Henderson
1 sibling, 0 replies; 41+ messages in thread
From: Adhemerval Zanella @ 2020-07-06 17:28 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: Sudakshina Das
On 01/07/2020 11:39, Szabolcs Nagy wrote:
> From: Sudakshina Das <sudi.das@arm.com>
>
> Binaries can opt-in to using BTI via an ELF object file marking.
> The dynamic linker has to then mprotect the executable segments
> with PROT_BTI. In case of static linked executables or in case
> of the dynamic linker itself, PROT_BTI protection is done by the
> operating system.
>
> On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
> the properties of a binary because PT_NOTE can be unreliable with
> old linkers (old linkers just append the notes of input objects
> together and add them to the output without checking them for
> consistency which means multiple incompatible GNU property notes
> can be present in PT_NOTE).
>
> BTI property is handled in the loader even if glibc is not built
> with BTI support, so in theory user code can be BTI protected
> independently of glibc. In practice though user binaries are not
> marked with the BTI property if glibc has no support because the
> static linked libc objects (crt files, libc_nonshared.a) are
> unmarked.
>
> This patch relies on Linux userspace API that is not yet in a
> linux release but in v5.8-rc1 so scheduled to be in Linux 5.8.
>
> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
LGTM, with a jsut nit below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/aarch64/Makefile | 4 ++
> sysdeps/aarch64/dl-bti.c | 54 ++++++++++++++++
> sysdeps/aarch64/dl-prop.h | 63 +++++++++++++++++++
> sysdeps/aarch64/linkmap.h | 3 +
> sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
> sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 +++++++++
> .../unix/sysv/linux/aarch64/cpu-features.c | 3 +
> .../unix/sysv/linux/aarch64/cpu-features.h | 2 +
> 8 files changed, 161 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/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..3c92377cc8
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-bti.c
> @@ -0,0 +1,54 @@
> +/* AArch64 BTI functions.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <unistd.h>
> +#include <errno.h>
> +#include <libintl.h>
> +#include <ldsodefs.h>
> +
> +static int
> +enable_bti (struct link_map *map, const char *program)
> +{
> + const ElfW(Phdr) *phdr;
> + unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
> +
> + for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
> + if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
> + {
> + ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
> + ElfW(Addr) len = phdr->p_memsz;
I don't think ElfW(Addr) is the correct type here (although it will be
mapped to uint64_t anyway). Why not use size_t here since the idea is
really to use on __mprotect?
> + 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 (errno, map->l_name, "dlopen",
> + N_("mprotect failed to turn on BTI"));
> + }
> + }
> + return 0;
> +}
> +
> +/* Enable BTI for L if required. */
> +
> +void
> +_dl_bti_check (struct link_map *l, const char *program)
> +{
> + if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti)
> + enable_bti (l, program);
> +}
Ok.
> diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
> new file mode 100644
> index 0000000000..b0785bda83
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-prop.h
> @@ -0,0 +1,63 @@
> +/* 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
> +
> +extern void _dl_bti_check (struct link_map *, const char *)
> + attribute_hidden;
> +
> +static inline void __attribute__ ((always_inline))
> +_rtld_main_check (struct link_map *m, const char *program)
> +{
> + _dl_bti_check (m, program);
> +}
> +
> +static inline void __attribute__ ((always_inline))
> +_dl_open_check (struct link_map *m)
> +{
> + _dl_bti_check (m, NULL);
> +}
> +
> +static inline void __attribute__ ((always_inline))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> +}
> +
> +static inline int
> +_dl_process_gnu_property (struct link_map *l, uint32_t type, uint32_t datasz,
> + void *data)
> +{
> + if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
> + {
> + /* Stop if the property note is ill-formed. */
> + if (datasz != 4)
> + return 0;
> +
> + unsigned int feature_1 = *(unsigned int *) data;
> + if (feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)
> + l->l_mach.bti = true;
> +
> + /* Stop if we processed the property note. */
> + return 0;
> + }
> + /* Continue. */
> + return 1;
> +}
> +
> +#endif /* _DL_PROP_H */
Ok.
> diff --git a/sysdeps/aarch64/linkmap.h b/sysdeps/aarch64/linkmap.h
> index 943a9ee9e4..847a03ace2 100644
> --- a/sysdeps/aarch64/linkmap.h
> +++ b/sysdeps/aarch64/linkmap.h
> @@ -16,8 +16,11 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> +#include <stdbool.h>
> +
> struct link_map_machine
> {
> ElfW(Addr) plt; /* Address of .plt */
> void *tlsdesc_table; /* Address of TLS descriptor hash table. */
> + bool bti; /* Branch Target Identification is enabled. */
> };
Ok.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> index 4ee14b4208..af90d8a626 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> @@ -72,3 +72,4 @@
> #define HWCAP2_BF16 (1 << 14)
> #define HWCAP2_DGH (1 << 15)
> #define HWCAP2_RNG (1 << 16)
> +#define HWCAP2_BTI (1 << 17)
Ok.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> new file mode 100644
> index 0000000000..ecae046344
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> @@ -0,0 +1,31 @@
> +/* Definitions for POSIX memory map interface. Linux/AArch64 version.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_MMAN_H
> +# error "Never use <bits/mman.h> directly; include <sys/mman.h> instead."
> +#endif
> +
> +/* AArch64 specific definitions, should be in sync with
> + arch/arm64/include/uapi/asm/mman.h. */
> +
> +#define PROT_BTI 0x10
> +
> +#include <bits/mman-map-flags-generic.h>
> +
> +/* Include generic Linux declarations. */
> +#include <bits/mman-linux.h>
Ok.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> index 896c588fee..b9ab827aca 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -83,4 +83,7 @@ init_cpu_features (struct cpu_features *cpu_features)
>
> if ((dczid & DCZID_DZP_MASK) == 0)
> cpu_features->zva_size = 4 << (dczid & DCZID_BS_MASK);
> +
> + /* Check if BTI is supported. */
> + cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
> }
Ok.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> index 1389cea1b3..a81f186ec2 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> @@ -20,6 +20,7 @@
> #define _CPU_FEATURES_AARCH64_H
>
> #include <stdint.h>
> +#include <stdbool.h>
>
> #define MIDR_PARTNUM_SHIFT 4
> #define MIDR_PARTNUM_MASK (0xfff << MIDR_PARTNUM_SHIFT)
> @@ -64,6 +65,7 @@ struct cpu_features
> {
> uint64_t midr_el1;
> unsigned zva_size;
> + bool bti;
> };
>
> #endif /* _CPU_FEATURES_AARCH64_H */
>
Ok.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 09/14] aarch64: ensure objects are BTI compatible
2020-07-01 14:39 ` [PATCH v6 09/14] aarch64: ensure objects are BTI compatible Szabolcs Nagy
@ 2020-07-06 17:37 ` Adhemerval Zanella
2020-07-06 18:01 ` Szabolcs Nagy
0 siblings, 1 reply; 41+ messages in thread
From: Adhemerval Zanella @ 2020-07-06 17:37 UTC (permalink / raw)
To: libc-alpha
On 01/07/2020 11:39, Szabolcs Nagy wrote:
> When glibc is built with branch protection (i.e. with a gcc configured
> with --enable-standard-branch-protection), all glibc binaries should
> be BTI compatible and marked as such.
>
> It is easy to link BTI incompatible objects by accident and this is
> silent currently which is usually not the expectation, so this is
> changed into a link error. (There is no linker flag for failing on
> BTI incompatible inputs so all warnings are turned into fatal errors
> outside the test system when building glibc with branch protection.)
>
> Unfortunately, outlined atomic functions are not BTI compatible in
> libgcc (PR libgcc/96001), so to build glibc with current gcc use
> 'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon
> and then glibc can be built and tested without such workarounds.
Is the outlined atomic present in any current released gcc? If so
should we add a configure handler to add -mno-outline-atomics for
such cases?
Besides that LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/aarch64/Makefile | 8 ++++++++
> sysdeps/aarch64/configure | 2 ++
> sysdeps/aarch64/configure.ac | 1 +
> 3 files changed, 11 insertions(+)
>
> diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> index 5ae8b082b0..7f82fc055e 100644
> --- a/sysdeps/aarch64/Makefile
> +++ b/sysdeps/aarch64/Makefile
> @@ -1,5 +1,13 @@
> long-double-fcts = yes
>
> +ifeq (yes,$(aarch64-bti))
> +# Mark linker output BTI compatible, it warns on non-BTI inputs.
> +sysdep-LDFLAGS += -Wl,-z,force-bti
> +# Make warnings fatal outside the test system.
> +LDFLAGS-lib.so += -Wl,--fatal-warnings
> +LDFLAGS-rtld += -Wl,-z,force-bti,--fatal-warnings
> +endif
> +
> ifeq ($(subdir),elf)
> sysdep-dl-routines += dl-bti
> endif
Ok.
> diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> index 70477a7fa5..c637540436 100644
> --- a/sysdeps/aarch64/configure
> +++ b/sysdeps/aarch64/configure
> @@ -210,6 +210,8 @@ EOF
> fi
> { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
> $as_echo "$libc_cv_aarch64_bti" >&6; }
> +config_vars="$config_vars
> +aarch64-bti = $libc_cv_aarch64_bti"
> if test $libc_cv_aarch64_bti = yes; then
> $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
>
> diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> index 798f494740..2c2817514d 100644
> --- a/sysdeps/aarch64/configure.ac
> +++ b/sysdeps/aarch64/configure.ac
> @@ -36,6 +36,7 @@ EOF
> libc_cv_aarch64_bti=yes
> fi
> rm -rf conftest.*])
> +LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti])
> if test $libc_cv_aarch64_bti = yes; then
> AC_DEFINE(HAVE_AARCH64_BTI)
> fi
>
Ok.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 09/14] aarch64: ensure objects are BTI compatible
2020-07-06 17:37 ` Adhemerval Zanella
@ 2020-07-06 18:01 ` Szabolcs Nagy
2020-07-06 18:17 ` Adhemerval Zanella
0 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-06 18:01 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
The 07/06/2020 14:37, Adhemerval Zanella via Libc-alpha wrote:
> On 01/07/2020 11:39, Szabolcs Nagy wrote:
> > When glibc is built with branch protection (i.e. with a gcc configured
> > with --enable-standard-branch-protection), all glibc binaries should
> > be BTI compatible and marked as such.
> >
> > It is easy to link BTI incompatible objects by accident and this is
> > silent currently which is usually not the expectation, so this is
> > changed into a link error. (There is no linker flag for failing on
> > BTI incompatible inputs so all warnings are turned into fatal errors
> > outside the test system when building glibc with branch protection.)
> >
> > Unfortunately, outlined atomic functions are not BTI compatible in
> > libgcc (PR libgcc/96001), so to build glibc with current gcc use
> > 'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon
> > and then glibc can be built and tested without such workarounds.
>
> Is the outlined atomic present in any current released gcc? If so
> should we add a configure handler to add -mno-outline-atomics for
> such cases?
outlined atomics is supported for a while now (since
gcc-9 i think) but it is now the default in gcc-10.
i am testing patches to fix libgcc in gcc trunk and
then backport to gcc-10 branch.
my preference is to only support fixed gcc and use
the -mno-outline-atomics as a temporary workaround
so we can test the patches with gcc-10.1.0
it's not a good workaround for using the resulting
toolchain for building user code with branch protection
(which will have the same problem: silently dropping
bti protection) so i think it's better to fail the build.
>
> Besides that LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> > ---
> > sysdeps/aarch64/Makefile | 8 ++++++++
> > sysdeps/aarch64/configure | 2 ++
> > sysdeps/aarch64/configure.ac | 1 +
> > 3 files changed, 11 insertions(+)
> >
> > diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> > index 5ae8b082b0..7f82fc055e 100644
> > --- a/sysdeps/aarch64/Makefile
> > +++ b/sysdeps/aarch64/Makefile
> > @@ -1,5 +1,13 @@
> > long-double-fcts = yes
> >
> > +ifeq (yes,$(aarch64-bti))
> > +# Mark linker output BTI compatible, it warns on non-BTI inputs.
> > +sysdep-LDFLAGS += -Wl,-z,force-bti
> > +# Make warnings fatal outside the test system.
> > +LDFLAGS-lib.so += -Wl,--fatal-warnings
> > +LDFLAGS-rtld += -Wl,-z,force-bti,--fatal-warnings
> > +endif
> > +
> > ifeq ($(subdir),elf)
> > sysdep-dl-routines += dl-bti
> > endif
>
> Ok.
>
> > diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> > index 70477a7fa5..c637540436 100644
> > --- a/sysdeps/aarch64/configure
> > +++ b/sysdeps/aarch64/configure
> > @@ -210,6 +210,8 @@ EOF
> > fi
> > { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
> > $as_echo "$libc_cv_aarch64_bti" >&6; }
> > +config_vars="$config_vars
> > +aarch64-bti = $libc_cv_aarch64_bti"
> > if test $libc_cv_aarch64_bti = yes; then
> > $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
> >
> > diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> > index 798f494740..2c2817514d 100644
> > --- a/sysdeps/aarch64/configure.ac
> > +++ b/sysdeps/aarch64/configure.ac
> > @@ -36,6 +36,7 @@ EOF
> > libc_cv_aarch64_bti=yes
> > fi
> > rm -rf conftest.*])
> > +LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti])
> > if test $libc_cv_aarch64_bti = yes; then
> > AC_DEFINE(HAVE_AARCH64_BTI)
> > fi
> >
>
> Ok.
--
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 02/14] aarch64: configure test for BTI support
2020-07-06 14:11 ` Adhemerval Zanella
@ 2020-07-06 18:07 ` Szabolcs Nagy
2020-07-06 18:12 ` Adhemerval Zanella
0 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-06 18:07 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
The 07/06/2020 11:11, Adhemerval Zanella via Libc-alpha wrote:
>
>
> On 01/07/2020 11:38, Szabolcs Nagy wrote:
> > Check BTI support in the compiler and linker. The check also
> > requires READELF that understands the BTI GNU property note.
> > It is expected to succeed with gcc >=gcc-9 configured with
> > --enable-standard-branch-protection and binutils >=binutils-2.33.
> >
> > Note: passing -mbranch-protection=bti in CFLAGS when building glibc
> > may not be enough to get a glibc that supports BTI because crtbegin*
> > and crtend* provided by the compiler needs to be BTI compatible too.
>
> If I read correctly this scenario should be covered by the configure
> test, right?
it is not covered because the config checks use -nostdlib
and -nostartfiles (otherwise objects from the libc that
the toolchain uses would leak into the checks).
so if gcc startfiles are not bti compatible then configure
succeeds (but later on there will be link failures so the
build fails).
>
> LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> > ---
> > config.h.in | 3 +++
> > sysdeps/aarch64/configure | 42 ++++++++++++++++++++++++++++++++++++
> > sysdeps/aarch64/configure.ac | 19 ++++++++++++++++
> > 3 files changed, 64 insertions(+)
> >
> > diff --git a/config.h.in b/config.h.in
> > index 831eca2fe1..67169e5d01 100644
> > --- a/config.h.in
> > +++ b/config.h.in
> > @@ -109,6 +109,9 @@
> > /* AArch64 big endian ABI */
> > #undef HAVE_AARCH64_BE
> >
> > +/* AArch64 BTI support enabled. */
> > +#define HAVE_AARCH64_BTI 0
> > +
> > /* C-SKY ABI version. */
> > #undef CSKYABI
> >
>
> Ok.
>
> > diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> > index 5bd355a691..70477a7fa5 100644
> > --- a/sysdeps/aarch64/configure
> > +++ b/sysdeps/aarch64/configure
> > @@ -172,3 +172,45 @@ else
> > config_vars="$config_vars
> > default-abi = lp64"
> > fi
> > +
> > +# Only consider BTI supported if -mbranch-protection=bti is
> > +# on by default in the compiler and the linker produces
> > +# binaries with GNU property notes in PT_GNU_PROPERTY segment.
> > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for BTI support" >&5
> > +$as_echo_n "checking for BTI support... " >&6; }
> > +if ${libc_cv_aarch64_bti+:} false; then :
> > + $as_echo_n "(cached) " >&6
> > +else
> > + cat > conftest.c <<EOF
> > +void foo (void) { }
> > +EOF
> > + libc_cv_aarch64_bti=no
> > + if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c'
> > + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> > + (eval $ac_try) 2>&5
> > + ac_status=$?
> > + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> > + test $ac_status = 0; }; } \
> > + && { ac_try='$READELF -lW conftest.so | grep -q GNU_PROPERTY'
> > + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> > + (eval $ac_try) 2>&5
> > + ac_status=$?
> > + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> > + test $ac_status = 0; }; } \
> > + && { ac_try='$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"'
> > + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> > + (eval $ac_try) 2>&5
> > + ac_status=$?
> > + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> > + test $ac_status = 0; }; }
> > + then
> > + libc_cv_aarch64_bti=yes
> > + fi
> > + rm -rf conftest.*
> > +fi
> > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
> > +$as_echo "$libc_cv_aarch64_bti" >&6; }
> > +if test $libc_cv_aarch64_bti = yes; then
> > + $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
> > +
> > +fi
> > diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> > index 7851dd4dac..798f494740 100644
> > --- a/sysdeps/aarch64/configure.ac
> > +++ b/sysdeps/aarch64/configure.ac
> > @@ -20,3 +20,22 @@ if test $libc_cv_aarch64_be = yes; then
> > else
> > LIBC_CONFIG_VAR([default-abi], [lp64])
> > fi
> > +
> > +# Only consider BTI supported if -mbranch-protection=bti is
> > +# on by default in the compiler and the linker produces
> > +# binaries with GNU property notes in PT_GNU_PROPERTY segment.
> > +AC_CACHE_CHECK([for BTI support], [libc_cv_aarch64_bti], [dnl
> > + cat > conftest.c <<EOF
> > +void foo (void) { }
> > +EOF
> > + libc_cv_aarch64_bti=no
> > + if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostdlib -nostartfiles $no_ssp -shared -fPIC -o conftest.so conftest.c]) \
> > + && AC_TRY_COMMAND([$READELF -lW conftest.so | grep -q GNU_PROPERTY]) \
> > + && AC_TRY_COMMAND([$READELF -nW conftest.so | grep -q "NT_GNU_PROPERTY_TYPE_0.*AArch64 feature:.* BTI"])
> > + then
> > + libc_cv_aarch64_bti=yes
> > + fi
> > + rm -rf conftest.*])
> > +if test $libc_cv_aarch64_bti = yes; then
> > + AC_DEFINE(HAVE_AARCH64_BTI)
> > +fi
> >
>
> Ok.
--
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 02/14] aarch64: configure test for BTI support
2020-07-06 18:07 ` Szabolcs Nagy
@ 2020-07-06 18:12 ` Adhemerval Zanella
2020-07-07 14:26 ` Szabolcs Nagy
0 siblings, 1 reply; 41+ messages in thread
From: Adhemerval Zanella @ 2020-07-06 18:12 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha
On 06/07/2020 15:07, Szabolcs Nagy wrote:
> The 07/06/2020 11:11, Adhemerval Zanella via Libc-alpha wrote:
>>
>>
>> On 01/07/2020 11:38, Szabolcs Nagy wrote:
>>> Check BTI support in the compiler and linker. The check also
>>> requires READELF that understands the BTI GNU property note.
>>> It is expected to succeed with gcc >=gcc-9 configured with
>>> --enable-standard-branch-protection and binutils >=binutils-2.33.
>>>
>>> Note: passing -mbranch-protection=bti in CFLAGS when building glibc
>>> may not be enough to get a glibc that supports BTI because crtbegin*
>>> and crtend* provided by the compiler needs to be BTI compatible too.
>>
>> If I read correctly this scenario should be covered by the configure
>> test, right?
>
> it is not covered because the config checks use -nostdlib
> and -nostartfiles (otherwise objects from the libc that
> the toolchain uses would leak into the checks).
>
> so if gcc startfiles are not bti compatible then configure
> succeeds (but later on there will be link failures so the
> build fails).
Right, would be hard to try to fail early on configure for this
case?
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 09/14] aarch64: ensure objects are BTI compatible
2020-07-06 18:01 ` Szabolcs Nagy
@ 2020-07-06 18:17 ` Adhemerval Zanella
0 siblings, 0 replies; 41+ messages in thread
From: Adhemerval Zanella @ 2020-07-06 18:17 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha
On 06/07/2020 15:01, Szabolcs Nagy wrote:
> The 07/06/2020 14:37, Adhemerval Zanella via Libc-alpha wrote:
>> On 01/07/2020 11:39, Szabolcs Nagy wrote:
>>> When glibc is built with branch protection (i.e. with a gcc configured
>>> with --enable-standard-branch-protection), all glibc binaries should
>>> be BTI compatible and marked as such.
>>>
>>> It is easy to link BTI incompatible objects by accident and this is
>>> silent currently which is usually not the expectation, so this is
>>> changed into a link error. (There is no linker flag for failing on
>>> BTI incompatible inputs so all warnings are turned into fatal errors
>>> outside the test system when building glibc with branch protection.)
>>>
>>> Unfortunately, outlined atomic functions are not BTI compatible in
>>> libgcc (PR libgcc/96001), so to build glibc with current gcc use
>>> 'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon
>>> and then glibc can be built and tested without such workarounds.
>>
>> Is the outlined atomic present in any current released gcc? If so
>> should we add a configure handler to add -mno-outline-atomics for
>> such cases?
>
> outlined atomics is supported for a while now (since
> gcc-9 i think) but it is now the default in gcc-10.
>
> i am testing patches to fix libgcc in gcc trunk and
> then backport to gcc-10 branch.
>
> my preference is to only support fixed gcc and use
> the -mno-outline-atomics as a temporary workaround
> so we can test the patches with gcc-10.1.0
>
> it's not a good workaround for using the resulting
> toolchain for building user code with branch protection
> (which will have the same problem: silently dropping
> bti protection) so i think it's better to fail the build.
Right, I think it should be ok to sign it with a build failure.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 12/14] aarch64: fix pac-ret support in _mcount
2020-07-01 14:40 ` [PATCH v6 12/14] aarch64: fix pac-ret support in _mcount Szabolcs Nagy
@ 2020-07-06 18:33 ` Adhemerval Zanella
0 siblings, 0 replies; 41+ messages in thread
From: Adhemerval Zanella @ 2020-07-06 18:33 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha
On 01/07/2020 11:40, Szabolcs Nagy wrote:
> Currently gcc -pg -mbranch-protection=pac-ret passes signed return
> address to _mcount, so _mcount now has to always strip pac from the
> frompc since that's from user code that may be built with pac-ret.
>
> This is gcc PR target/94791: signed pointers should not escape and get
> passed across extern call boundaries, since that's an ABI break, but
> because existing gcc has this issue we work it around in glibc until
> that is resolved. This is compatible with a fixed gcc and it is a nop
> on systems without PAuth support. The bug was introduced in gcc-7 with
> -msign-return-address=non-leaf|all support which in gcc-9 got renamed
> to -mbranch-protection=pac-ret|pac-ret+leaf|standard.
>
> strip_pac uses inline asm instead of __builtin_aarch64_xpaclri since
> that is not a documented api and not available in all supported gccs.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/aarch64/machine-gmon.h | 3 ++-
> sysdeps/aarch64/sysdep.h | 11 +++++++++++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h
> index 730a23b781..a687298b1c 100644
> --- a/sysdeps/aarch64/machine-gmon.h
> +++ b/sysdeps/aarch64/machine-gmon.h
> @@ -27,8 +27,9 @@ static void mcount_internal (u_long frompc, u_long selfpc);
> #define _MCOUNT_DECL(frompc, selfpc) \
> static inline void mcount_internal (u_long frompc, u_long selfpc)
>
> +/* Note: strip_pac is needed for frompc because of gcc PR target/94791. */
> #define MCOUNT \
> void __mcount (void *frompc) \
> { \
> - mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
> + mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \
> }
Ok.
> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 500c272745..2879aeaa5c 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -35,6 +35,17 @@
>
> #define PTR_SIZE (1<<PTR_LOG_SIZE)
>
> +#ifndef __ASSEMBLER__
> +/* Strip pointer authentication code from pointer p. */
> +static inline void *
> +strip_pac (void *p)
> +{
> + register void *ra asm ("x30") = (p);
> + asm ("hint 7 // xpaclri" : "+r"(ra));
> + return ra;
> +}
> +#endif
> +
> #ifdef __ASSEMBLER__
>
> /* Syntactic details of assembler. */
>
Ok.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 13/14] aarch64: redefine RETURN_ADDRESS to strip PAC
2020-07-01 14:40 ` [PATCH v6 13/14] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
@ 2020-07-06 18:34 ` Adhemerval Zanella
0 siblings, 0 replies; 41+ messages in thread
From: Adhemerval Zanella @ 2020-07-06 18:34 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha
On 01/07/2020 11:40, Szabolcs Nagy wrote:
> RETURN_ADDRESS is used at several places in glibc to mean a valid
> code address of the call site, but with pac-ret it may contain a
> pointer authentication code (PAC), so its definition is adjusted.
>
> This is gcc PR target/94891: __builtin_return_address should not
> expose signed pointers to user code where it can cause ABI issues.
> In glibc RETURN_ADDRESS is only changed if it is built with pac-ret.
> There is no detection for the specific gcc issue because it is
> hard to test and the additional xpac does not cause problems.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/aarch64/sysdep.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 2879aeaa5c..48fa8e9e90 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -44,6 +44,13 @@ strip_pac (void *p)
> asm ("hint 7 // xpaclri" : "+r"(ra));
> return ra;
> }
> +
> +/* This is needed when glibc is built with -mbranch-protection=pac-ret
> + with a gcc that is affected by PR target/94891. */
> +# if HAVE_AARCH64_PAC_RET
> +# undef RETURN_ADDRESS
> +# define RETURN_ADDRESS(n) strip_pac (__builtin_return_address (n))
> +# endif
> #endif
>
> #ifdef __ASSEMBLER__
>
Ok.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 14/14] aarch64: add NEWS entry about branch protection support
2020-07-01 14:41 ` [PATCH v6 14/14] aarch64: add NEWS entry about branch protection support Szabolcs Nagy
@ 2020-07-06 18:41 ` Adhemerval Zanella
2020-07-08 10:04 ` Szabolcs Nagy
0 siblings, 1 reply; 41+ messages in thread
From: Adhemerval Zanella @ 2020-07-06 18:41 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha
On 01/07/2020 11:41, Szabolcs Nagy wrote:
> This is a new security feature that relies on architecture
> extensions and needs glibc to be built with a gcc configured
> with branch protection.
> ---
> NEWS | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index a660fc59a8..7d0ca3f520 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -31,6 +31,18 @@ Major new features:
> pthread_attr_getsigmask_np have been added. They allow applications
> to specify the signal mask of a thread created with pthread_create.
>
> +* AArch64 now supports standard branch protection security hardening
> + in glibc when it is built with a GCC that is configured with
> + --enable-standard-branch-protection. This includes branch target
Should we state that user can also set the required flags on compiler
specification as well (CC='gcc -mbranch-protection=pac-ret+bti -O2)?
> + identification (BTI) and pointer authentication for return addresses
> + (PAC-RET). They require armv8.5-a and armv8.3-a architecture
Two space after period.
> + extensions respectively for the protection to be effective,
> + otherwise the used instructions are nops. User code can use PAC-RET
> + without libc support, but BTI requires a libc that is built with BTI
> + support, otherwise runtime objects linked into user code will not be
> + BTI compatible. It is recommended to use GCC 10 or newer when
> + building glibc with branch protection.
Should we extend why gcc 10 is required here? This statement without much
explanation might raise some questioning.
> +
> Deprecated and removed features, and other changes affecting compatibility:
>
> * The deprecated <sys/sysctl.h> header and the sysctl function have been
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 02/14] aarch64: configure test for BTI support
2020-07-06 18:12 ` Adhemerval Zanella
@ 2020-07-07 14:26 ` Szabolcs Nagy
2020-07-07 14:39 ` H.J. Lu
0 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-07 14:26 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
The 07/06/2020 15:12, Adhemerval Zanella wrote:
>
>
> On 06/07/2020 15:07, Szabolcs Nagy wrote:
> > The 07/06/2020 11:11, Adhemerval Zanella via Libc-alpha wrote:
> >>
> >>
> >> On 01/07/2020 11:38, Szabolcs Nagy wrote:
> >>> Check BTI support in the compiler and linker. The check also
> >>> requires READELF that understands the BTI GNU property note.
> >>> It is expected to succeed with gcc >=gcc-9 configured with
> >>> --enable-standard-branch-protection and binutils >=binutils-2.33.
> >>>
> >>> Note: passing -mbranch-protection=bti in CFLAGS when building glibc
> >>> may not be enough to get a glibc that supports BTI because crtbegin*
> >>> and crtend* provided by the compiler needs to be BTI compatible too.
> >>
> >> If I read correctly this scenario should be covered by the configure
> >> test, right?
> >
> > it is not covered because the config checks use -nostdlib
> > and -nostartfiles (otherwise objects from the libc that
> > the toolchain uses would leak into the checks).
> >
> > so if gcc startfiles are not bti compatible then configure
> > succeeds (but later on there will be link failures so the
> > build fails).
>
> Right, would be hard to try to fail early on configure for this
> case?
i don't know how to reliably link with gcc start files
but not with libc start files.
explicit link wiht crtbeginS.o may not be right, the
exact name depends on the compiler driver.
so i would have to provide dummy libc start files to be
able to do the check, but that can go bad in many ways
so the check may not be reliable.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 02/14] aarch64: configure test for BTI support
2020-07-07 14:26 ` Szabolcs Nagy
@ 2020-07-07 14:39 ` H.J. Lu
2020-07-07 16:58 ` Szabolcs Nagy
0 siblings, 1 reply; 41+ messages in thread
From: H.J. Lu @ 2020-07-07 14:39 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: Adhemerval Zanella, GNU C Library
On Tue, Jul 7, 2020 at 7:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 07/06/2020 15:12, Adhemerval Zanella wrote:
> >
> >
> > On 06/07/2020 15:07, Szabolcs Nagy wrote:
> > > The 07/06/2020 11:11, Adhemerval Zanella via Libc-alpha wrote:
> > >>
> > >>
> > >> On 01/07/2020 11:38, Szabolcs Nagy wrote:
> > >>> Check BTI support in the compiler and linker. The check also
> > >>> requires READELF that understands the BTI GNU property note.
> > >>> It is expected to succeed with gcc >=gcc-9 configured with
> > >>> --enable-standard-branch-protection and binutils >=binutils-2.33.
> > >>>
> > >>> Note: passing -mbranch-protection=bti in CFLAGS when building glibc
> > >>> may not be enough to get a glibc that supports BTI because crtbegin*
> > >>> and crtend* provided by the compiler needs to be BTI compatible too.
> > >>
> > >> If I read correctly this scenario should be covered by the configure
> > >> test, right?
> > >
> > > it is not covered because the config checks use -nostdlib
> > > and -nostartfiles (otherwise objects from the libc that
> > > the toolchain uses would leak into the checks).
> > >
> > > so if gcc startfiles are not bti compatible then configure
> > > succeeds (but later on there will be link failures so the
> > > build fails).
> >
> > Right, would be hard to try to fail early on configure for this
> > case?
>
> i don't know how to reliably link with gcc start files
> but not with libc start files.
>
> explicit link wiht crtbeginS.o may not be right, the
> exact name depends on the compiler driver.
>
> so i would have to provide dummy libc start files to be
> able to do the check, but that can go bad in many ways
> so the check may not be reliable.
For CET, I added --enable-cet to enable CET in GCC run-time,
including crtbeginS.o, ... Since CET enabled run-time is backward
compatible, it is safe to do so. In fact, --enable-cet is the default
starting from GCC 10.
--
H.J.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 02/14] aarch64: configure test for BTI support
2020-07-07 14:39 ` H.J. Lu
@ 2020-07-07 16:58 ` Szabolcs Nagy
2020-07-07 17:24 ` H.J. Lu
0 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-07 16:58 UTC (permalink / raw)
To: H.J. Lu; +Cc: Adhemerval Zanella, GNU C Library
The 07/07/2020 07:39, H.J. Lu wrote:
> On Tue, Jul 7, 2020 at 7:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > i don't know how to reliably link with gcc start files
> > but not with libc start files.
> >
> > explicit link wiht crtbeginS.o may not be right, the
> > exact name depends on the compiler driver.
> >
> > so i would have to provide dummy libc start files to be
> > able to do the check, but that can go bad in many ways
> > so the check may not be reliable.
>
> For CET, I added --enable-cet to enable CET in GCC run-time,
> including crtbeginS.o, ... Since CET enabled run-time is backward
> compatible, it is safe to do so. In fact, --enable-cet is the default
> starting from GCC 10.
makes sense.
on aarch64, bti needs a recent binutils otherwise
there are ugly linker warnings for the unknown gnu
properties so i don't think it's a good idea to
turn it on by default just yet.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 02/14] aarch64: configure test for BTI support
2020-07-07 16:58 ` Szabolcs Nagy
@ 2020-07-07 17:24 ` H.J. Lu
0 siblings, 0 replies; 41+ messages in thread
From: H.J. Lu @ 2020-07-07 17:24 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: Adhemerval Zanella, GNU C Library
On Tue, Jul 7, 2020 at 9:59 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 07/07/2020 07:39, H.J. Lu wrote:
> > On Tue, Jul 7, 2020 at 7:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > i don't know how to reliably link with gcc start files
> > > but not with libc start files.
> > >
> > > explicit link wiht crtbeginS.o may not be right, the
> > > exact name depends on the compiler driver.
> > >
> > > so i would have to provide dummy libc start files to be
> > > able to do the check, but that can go bad in many ways
> > > so the check may not be reliable.
> >
> > For CET, I added --enable-cet to enable CET in GCC run-time,
> > including crtbeginS.o, ... Since CET enabled run-time is backward
> > compatible, it is safe to do so. In fact, --enable-cet is the default
> > starting from GCC 10.
>
> makes sense.
>
> on aarch64, bti needs a recent binutils otherwise
> there are ugly linker warnings for the unknown gnu
> properties so i don't think it's a good idea to
> turn it on by default just yet.
config/cet.m4 has --enable-cet=auto. CET is enabled only
if binutuls supports CET.
--
H.J.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 14/14] aarch64: add NEWS entry about branch protection support
2020-07-06 18:41 ` Adhemerval Zanella
@ 2020-07-08 10:04 ` Szabolcs Nagy
0 siblings, 0 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-08 10:04 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
The 07/06/2020 15:41, Adhemerval Zanella wrote:
> On 01/07/2020 11:41, Szabolcs Nagy wrote:
> >
> > +* AArch64 now supports standard branch protection security hardening
> > + in glibc when it is built with a GCC that is configured with
> > + --enable-standard-branch-protection. This includes branch target
>
> Should we state that user can also set the required flags on compiler
> specification as well (CC='gcc -mbranch-protection=pac-ret+bti -O2)?
the gcc config option is the preferred way, explicit CC
setting may or may not work if the compiler internally
has to do something differently (such as building its
own runtime libs with bti support).
> > + identification (BTI) and pointer authentication for return addresses
> > + (PAC-RET). They require armv8.5-a and armv8.3-a architecture
>
> Two space after period.
fixed throughout.
> > + extensions respectively for the protection to be effective,
> > + otherwise the used instructions are nops. User code can use PAC-RET
> > + without libc support, but BTI requires a libc that is built with BTI
> > + support, otherwise runtime objects linked into user code will not be
> > + BTI compatible. It is recommended to use GCC 10 or newer when
> > + building glibc with branch protection.
>
> Should we extend why gcc 10 is required here? This statement without much
> explanation might raise some questioning.
i removed the last sentence. (there were nasty bugs in gcc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94514
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94515
which were fixed and backported, but instead of checking
if gcc-9 have them i thought it would be easier to just
recommend gcc 10 which always have the fixes. but i dont
think we need to go into details in the news entry.)
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 08/14] aarch64: enable BTI at runtime
2020-07-01 14:39 ` [PATCH v6 08/14] aarch64: enable BTI at runtime Szabolcs Nagy
2020-07-06 17:28 ` Adhemerval Zanella
@ 2020-07-11 15:58 ` Richard Henderson
2020-07-13 8:32 ` Szabolcs Nagy
2020-07-13 13:14 ` Szabolcs Nagy
1 sibling, 2 replies; 41+ messages in thread
From: Richard Henderson @ 2020-07-11 15:58 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha; +Cc: Sudakshina Das
On 7/1/20 7:39 AM, Szabolcs Nagy wrote:
> + 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)
Spend an extra cycle or two and honor the exact set of p_flags?
If I construct an executable but non-readable PT_LOAD segment, that should be
fine. RWX with BTI would undoubtedly be an unusual case, but perhaps not
implausible in the context of a JIT with a statically allocated buffer.
r~
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 08/14] aarch64: enable BTI at runtime
2020-07-11 15:58 ` Richard Henderson
@ 2020-07-13 8:32 ` Szabolcs Nagy
2020-07-13 13:14 ` Szabolcs Nagy
1 sibling, 0 replies; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-13 8:32 UTC (permalink / raw)
To: Richard Henderson; +Cc: libc-alpha, Sudakshina Das
The 07/11/2020 08:58, Richard Henderson wrote:
> On 7/1/20 7:39 AM, Szabolcs Nagy wrote:
> > + 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)
>
> Spend an extra cycle or two and honor the exact set of p_flags?
>
> If I construct an executable but non-readable PT_LOAD segment, that should be
> fine. RWX with BTI would undoubtedly be an unusual case, but perhaps not
> implausible in the context of a JIT with a statically allocated buffer.
hm makes sense, i'll fix it.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 08/14] aarch64: enable BTI at runtime
2020-07-11 15:58 ` Richard Henderson
2020-07-13 8:32 ` Szabolcs Nagy
@ 2020-07-13 13:14 ` Szabolcs Nagy
2020-07-13 13:28 ` Szabolcs Nagy
1 sibling, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-13 13:14 UTC (permalink / raw)
To: Richard Henderson; +Cc: libc-alpha, Sudakshina Das
[-- Attachment #1: Type: text/plain, Size: 827 bytes --]
The 07/11/2020 08:58, Richard Henderson wrote:
> On 7/1/20 7:39 AM, Szabolcs Nagy wrote:
> > + 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)
>
> Spend an extra cycle or two and honor the exact set of p_flags?
>
> If I construct an executable but non-readable PT_LOAD segment, that should be
> fine. RWX with BTI would undoubtedly be an unusual case, but perhaps not
> implausible in the context of a JIT with a statically allocated buffer.
i will commit the attached fix tomorrow
unless there are comments on it.
[-- Attachment #2: 0001-aarch64-Respect-p_flags-when-protecting-code-with-PR.patch --]
[-- Type: text/x-diff, Size: 1584 bytes --]
From ee5765a839f6a40a61960264ed46393dc5d6c534 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Mon, 13 Jul 2020 11:28:18 +0100
Subject: [PATCH] aarch64: Respect p_flags when protecting code with PROT_BTI
Use PROT_READ and PROT_WRITE according to the load segment p_flags
when adding PROT_BTI.
This is before processing relocations which may drop PROT_BTI in
case of textrels. Executable stacks are not protected via PROT_BTI
either. PROT_BTI is hardening in case memory corruption happened,
it's value is reduced if there is writable and executable memory
available so missing it on such memory is fine, but we should
respect the p_flags and should not drop PROT_WRITE.
---
sysdeps/aarch64/dl-bti.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
index 965ddcc732..01ffb69a4a 100644
--- a/sysdeps/aarch64/dl-bti.c
+++ b/sysdeps/aarch64/dl-bti.c
@@ -24,13 +24,20 @@ static int
enable_bti (struct link_map *map, const char *program)
{
const ElfW(Phdr) *phdr;
- unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
+ unsigned prot;
for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
{
void *start = (void *) (phdr->p_vaddr + map->l_addr);
size_t len = phdr->p_memsz;
+
+ prot = PROT_EXEC | PROT_BTI;
+ if (ph->p_flags & PF_R)
+ prot |= PROT_READ;
+ if (ph->p_flags & PF_W)
+ prot |= PROT_WRITE;
+
if (__mprotect (start, len, prot) < 0)
{
if (program)
--
2.17.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 08/14] aarch64: enable BTI at runtime
2020-07-13 13:14 ` Szabolcs Nagy
@ 2020-07-13 13:28 ` Szabolcs Nagy
2020-07-13 16:55 ` Richard Henderson
0 siblings, 1 reply; 41+ messages in thread
From: Szabolcs Nagy @ 2020-07-13 13:28 UTC (permalink / raw)
To: Richard Henderson; +Cc: Sudakshina Das, libc-alpha
The 07/13/2020 14:14, Szabolcs Nagy wrote:
> The 07/11/2020 08:58, Richard Henderson wrote:
> > On 7/1/20 7:39 AM, Szabolcs Nagy wrote:
> > > + 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)
> >
> > Spend an extra cycle or two and honor the exact set of p_flags?
> >
> > If I construct an executable but non-readable PT_LOAD segment, that should be
> > fine. RWX with BTI would undoubtedly be an unusual case, but perhaps not
> > implausible in the context of a JIT with a statically allocated buffer.
>
> i will commit the attached fix tomorrow
> unless there are comments on it.
>
> From ee5765a839f6a40a61960264ed46393dc5d6c534 Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Date: Mon, 13 Jul 2020 11:28:18 +0100
> Subject: [PATCH] aarch64: Respect p_flags when protecting code with PROT_BTI
>
> Use PROT_READ and PROT_WRITE according to the load segment p_flags
> when adding PROT_BTI.
>
> This is before processing relocations which may drop PROT_BTI in
> case of textrels. Executable stacks are not protected via PROT_BTI
> either. PROT_BTI is hardening in case memory corruption happened,
> it's value is reduced if there is writable and executable memory
> available so missing it on such memory is fine, but we should
> respect the p_flags and should not drop PROT_WRITE.
> ---
> sysdeps/aarch64/dl-bti.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
> index 965ddcc732..01ffb69a4a 100644
> --- a/sysdeps/aarch64/dl-bti.c
> +++ b/sysdeps/aarch64/dl-bti.c
> @@ -24,13 +24,20 @@ static int
> enable_bti (struct link_map *map, const char *program)
> {
> const ElfW(Phdr) *phdr;
> - unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
> + unsigned prot;
>
> for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
> if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
> {
> void *start = (void *) (phdr->p_vaddr + map->l_addr);
> size_t len = phdr->p_memsz;
> +
> + prot = PROT_EXEC | PROT_BTI;
> + if (ph->p_flags & PF_R)
should be phdr-> instead of ph->
and i will have to wait for the tests to finish too which
takes about 40h.
> + prot |= PROT_READ;
> + if (ph->p_flags & PF_W)
> + prot |= PROT_WRITE;
> +
> if (__mprotect (start, len, prot) < 0)
> {
> if (program)
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 08/14] aarch64: enable BTI at runtime
2020-07-13 13:28 ` Szabolcs Nagy
@ 2020-07-13 16:55 ` Richard Henderson
0 siblings, 0 replies; 41+ messages in thread
From: Richard Henderson @ 2020-07-13 16:55 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: Sudakshina Das, libc-alpha
On 7/13/20 6:28 AM, Szabolcs Nagy wrote:
> The 07/13/2020 14:14, Szabolcs Nagy wrote:
>> The 07/11/2020 08:58, Richard Henderson wrote:
>>> On 7/1/20 7:39 AM, Szabolcs Nagy wrote:
>>>> + 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)
>>>
>>> Spend an extra cycle or two and honor the exact set of p_flags?
>>>
>>> If I construct an executable but non-readable PT_LOAD segment, that should be
>>> fine. RWX with BTI would undoubtedly be an unusual case, but perhaps not
>>> implausible in the context of a JIT with a statically allocated buffer.
>>
>> i will commit the attached fix tomorrow
>> unless there are comments on it.
Thanks. LGTM, modulo the naming typo.
r~
>>
>
>> From ee5765a839f6a40a61960264ed46393dc5d6c534 Mon Sep 17 00:00:00 2001
>> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
>> Date: Mon, 13 Jul 2020 11:28:18 +0100
>> Subject: [PATCH] aarch64: Respect p_flags when protecting code with PROT_BTI
>>
>> Use PROT_READ and PROT_WRITE according to the load segment p_flags
>> when adding PROT_BTI.
>>
>> This is before processing relocations which may drop PROT_BTI in
>> case of textrels. Executable stacks are not protected via PROT_BTI
>> either. PROT_BTI is hardening in case memory corruption happened,
>> it's value is reduced if there is writable and executable memory
>> available so missing it on such memory is fine, but we should
>> respect the p_flags and should not drop PROT_WRITE.
>> ---
>> sysdeps/aarch64/dl-bti.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
>> index 965ddcc732..01ffb69a4a 100644
>> --- a/sysdeps/aarch64/dl-bti.c
>> +++ b/sysdeps/aarch64/dl-bti.c
>> @@ -24,13 +24,20 @@ static int
>> enable_bti (struct link_map *map, const char *program)
>> {
>> const ElfW(Phdr) *phdr;
>> - unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
>> + unsigned prot;
>>
>> for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
>> if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
>> {
>> void *start = (void *) (phdr->p_vaddr + map->l_addr);
>> size_t len = phdr->p_memsz;
>> +
>> + prot = PROT_EXEC | PROT_BTI;
>> + if (ph->p_flags & PF_R)
>
> should be phdr-> instead of ph->
>
> and i will have to wait for the tests to finish too which
> takes about 40h.
>
>> + prot |= PROT_READ;
>> + if (ph->p_flags & PF_W)
>> + prot |= PROT_WRITE;
>> +
>> if (__mprotect (start, len, prot) < 0)
>> {
>> if (program)
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2020-07-13 16:55 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01 14:37 [PATCH v6 00/14] aarch64: branch protection support Szabolcs Nagy
2020-07-01 14:37 ` [PATCH v6 01/14] Rewrite abi-note.S in C Szabolcs Nagy
2020-07-01 14:41 ` H.J. Lu
2020-07-01 17:31 ` Szabolcs Nagy
2020-07-01 17:43 ` H.J. Lu
2020-07-02 8:39 ` Szabolcs Nagy
2020-07-01 14:38 ` [PATCH v6 02/14] aarch64: configure test for BTI support Szabolcs Nagy
2020-07-06 14:11 ` Adhemerval Zanella
2020-07-06 18:07 ` Szabolcs Nagy
2020-07-06 18:12 ` Adhemerval Zanella
2020-07-07 14:26 ` Szabolcs Nagy
2020-07-07 14:39 ` H.J. Lu
2020-07-07 16:58 ` Szabolcs Nagy
2020-07-07 17:24 ` H.J. Lu
2020-07-01 14:38 ` [PATCH v6 03/14] aarch64: Rename place holder .S files to .c Szabolcs Nagy
2020-07-01 14:38 ` [PATCH v6 04/14] aarch64: Add BTI support to assembly files Szabolcs Nagy
2020-07-03 16:19 ` Szabolcs Nagy
2020-07-01 14:38 ` [PATCH v6 05/14] aarch64: fix swapcontext for BTI Szabolcs Nagy
2020-07-01 14:39 ` [PATCH v6 06/14] aarch64: fix RTLD_START " Szabolcs Nagy
2020-07-01 14:39 ` [PATCH v6 07/14] rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling Szabolcs Nagy
2020-07-06 16:11 ` Adhemerval Zanella
2020-07-01 14:39 ` [PATCH v6 08/14] aarch64: enable BTI at runtime Szabolcs Nagy
2020-07-06 17:28 ` Adhemerval Zanella
2020-07-11 15:58 ` Richard Henderson
2020-07-13 8:32 ` Szabolcs Nagy
2020-07-13 13:14 ` Szabolcs Nagy
2020-07-13 13:28 ` Szabolcs Nagy
2020-07-13 16:55 ` Richard Henderson
2020-07-01 14:39 ` [PATCH v6 09/14] aarch64: ensure objects are BTI compatible Szabolcs Nagy
2020-07-06 17:37 ` Adhemerval Zanella
2020-07-06 18:01 ` Szabolcs Nagy
2020-07-06 18:17 ` Adhemerval Zanella
2020-07-01 14:40 ` [PATCH v6 10/14] aarch64: configure check for pac-ret code generation Szabolcs Nagy
2020-07-01 14:40 ` [PATCH v6 11/14] aarch64: Add pac-ret support to assembly files Szabolcs Nagy
2020-07-01 14:40 ` [PATCH v6 12/14] aarch64: fix pac-ret support in _mcount Szabolcs Nagy
2020-07-06 18:33 ` Adhemerval Zanella
2020-07-01 14:40 ` [PATCH v6 13/14] aarch64: redefine RETURN_ADDRESS to strip PAC Szabolcs Nagy
2020-07-06 18:34 ` Adhemerval Zanella
2020-07-01 14:41 ` [PATCH v6 14/14] aarch64: add NEWS entry about branch protection support Szabolcs Nagy
2020-07-06 18:41 ` Adhemerval Zanella
2020-07-08 10:04 ` 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).