[PATCH v7 00/14] aarch64: branch protection support

classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH v7 00/14] aarch64: branch protection support

Szabolcs Nagy-2
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-v7 branch.

v7:
- use sizeof in abi-note.S instead of int literals.
- move bti c after cfi_startproc.
- use size_t/void* instead of ElfW(Addr) in dl-bti.c.
- NEWS fixes.
- add reviewed-by, now only the first and last patches need review.

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                                          | 11 +++
 config.h.in                                   |  6 ++
 csu/{abi-note.S => abi-note.c}                | 25 +++--
 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, 575 insertions(+), 87 deletions(-)
 rename csu/{abi-note.S => abi-note.c} (89%)
 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 01/14] Rewrite abi-note.S in C.

Szabolcs Nagy-2
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 protection.
---
 csu/{abi-note.S => abi-note.c} | 25 +++++++++++++++----------
 sysdeps/arm/abi-note.S         |  8 --------
 2 files changed, 15 insertions(+), 18 deletions(-)
 rename csu/{abi-note.S => abi-note.c} (89%)
 delete mode 100644 sysdeps/arm/abi-note.S

diff --git a/csu/abi-note.S b/csu/abi-note.c
similarity index 89%
rename from csu/abi-note.S
rename to csu/abi-note.c
index 2b4b5f8824..8febf4cac0 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,16 @@ 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 = sizeof __abi_tag.name,
+    .n_descsz = sizeof __abi_tag.desc,
+    .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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 02/14] aarch64: configure test for BTI support

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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.

Reviewed-by: Adhemerval Zanella  <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 03/14] aarch64: Rename place holder .S files to .c

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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  <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 04/14] aarch64: Add BTI support to assembly files

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
From: Sudakshina Das <[hidden email]>

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 <[hidden email]>
Reviewed-by: Adhemerval Zanella <[hidden email]>
---
 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..0eeb0bb2f1 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -41,6 +41,35 @@
 
 #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); \
@@ -48,6 +77,7 @@
   .align 4; \
   C_LABEL(name) \
   cfi_startproc; \
+  BTI_C; \
   CALL_MCOUNT
 
 /* Define an entry point visible from C.  */
@@ -57,6 +87,7 @@
   .p2align align; \
   C_LABEL(name) \
   cfi_startproc; \
+  BTI_C; \
   CALL_MCOUNT
 
 /* Define an entry point visible from C with a specified alignment and
@@ -68,11 +99,12 @@
   .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) \
   cfi_startproc; \
+  BTI_C; \
   CALL_MCOUNT
 
 #undef END
--
2.17.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 05/14] aarch64: fix swapcontext for BTI

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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  <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 06/14] aarch64: fix RTLD_START for BTI

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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  <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 07/14] rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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 <[hidden email]>
Reviewed-by: Adhemerval Zanella  <[hidden email]>
---
 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..e39980fb19 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 08/14] aarch64: enable BTI at runtime

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
From: Sudakshina Das <[hidden email]>

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 <[hidden email]>
Reviewed-by: Adhemerval Zanella  <[hidden email]>
---
 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..965ddcc732
--- /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))
+      {
+ void *start = (void *) (phdr->p_vaddr + map->l_addr);
+ size_t len = phdr->p_memsz;
+ if (__mprotect (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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 09/14] aarch64: ensure objects are BTI compatible

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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.

Reviewed-by: Adhemerval Zanella <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 10/14] aarch64: configure check for pac-ret code generation

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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 <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 11/14] aarch64: Add pac-ret support to assembly files

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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 <[hidden email]>
---
 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 0eeb0bb2f1..cd88023163 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 12/14] aarch64: fix pac-ret support in _mcount

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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.

Reviewed-by: Adhemerval Zanella  <[hidden email]>
---
 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 cd88023163..f442506952 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 13/14] aarch64: redefine RETURN_ADDRESS to strip PAC

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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.

Reviewed-by: Adhemerval Zanella  <[hidden email]>
---
 sysdeps/aarch64/sysdep.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index f442506952..ae269146e3 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v7 14/14] aarch64: add NEWS entry about branch protection support

Szabolcs Nagy-2
In reply to this post by Szabolcs Nagy-2
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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/NEWS b/NEWS
index d7282b4ad5..5083f5eacf 100644
--- a/NEWS
+++ b/NEWS
@@ -67,6 +67,17 @@ Major new features:
   They should be used instead of sys_errlist and sys_nerr, both are
   thread and async-signal safe.  These functions are GNU extensions.
 
+* 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.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * The deprecated <sys/sysctl.h> header and the sysctl function have been
--
2.17.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 01/14] Rewrite abi-note.S in C.

Sourceware - libc-alpha mailing list
In reply to this post by Szabolcs Nagy-2
* Szabolcs Nagy:

> 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 protection.
> ---
>  csu/{abi-note.S => abi-note.c} | 25 +++++++++++++++----------
>  sysdeps/arm/abi-note.S         |  8 --------
>  2 files changed, 15 insertions(+), 18 deletions(-)
>  rename csu/{abi-note.S => abi-note.c} (89%)
>  delete mode 100644 sysdeps/arm/abi-note.S

This looks okay to me.

Thanks,
Florian

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 07/14] rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling

Sourceware - libc-alpha mailing list
In reply to this post by Szabolcs Nagy-2
On Wed, Jul 8, 2020 at 5:14 AM Szabolcs Nagy <[hidden email]> 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 <[hidden email]>
> Reviewed-by: Adhemerval Zanella  <[hidden email]>

The previous patch:

https://sourceware.org/pipermail/libc-alpha/2020-July/115636.html

has been acked by

https://sourceware.org/pipermail/libc-alpha/2020-July/115831.html


>  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..e39980fb19 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;
> +

A space is added.

Does this patch require other patches? If not, it can go in now.

Thanks.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 07/14] rtld: Clean up PT_NOTE and add PT_GNU_PROPERTY handling

Szabolcs Nagy-2
The 07/08/2020 06:23, H.J. Lu wrote:
> Does this patch require other patches? If not, it can go in now.

committed now.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 14/14] aarch64: add NEWS entry about branch protection support

Sourceware - libc-alpha mailing list
In reply to this post by Szabolcs Nagy-2


On 08/07/2020 09:14, 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.

LGTM, thanks. I think I have acked patches, is there still a missing one?
Otherwise I think the patchset it ok to be pushed upstream.

> ---
>  NEWS | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index d7282b4ad5..5083f5eacf 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -67,6 +67,17 @@ Major new features:
>    They should be used instead of sys_errlist and sys_nerr, both are
>    thread and async-signal safe.  These functions are GNU extensions.
>  
> +* 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.
> +
>  Deprecated and removed features, and other changes affecting compatibility:
>  
>  * The deprecated <sys/sysctl.h> header and the sysctl function have been
>

Ok.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v7 14/14] aarch64: add NEWS entry about branch protection support

Szabolcs Nagy-2
The 07/08/2020 10:48, Adhemerval Zanella wrote:
> On 08/07/2020 09:14, 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.
>
> LGTM, thanks. I think I have acked patches, is there still a missing one?
> Otherwise I think the patchset it ok to be pushed upstream.

thanks, pushed the series.
12