diff mbox series

[v3,09/13] aarch64: enable BTI at runtime

Message ID 7b32d3a81141aad6c52187d959ecf82d125a513c.1589552055.git.szabolcs.nagy@arm.com
State New
Headers show
Series aarch64: branch protection support | expand

Commit Message

Szabolcs Nagy May 15, 2020, 2:40 p.m. UTC
From: Sudakshina Das <sudi.das@arm.com>

Binaries can opt-in to using BTI via an ELF object file marking.
The dynamic linker has to then mprotect the executable segments
with PROT_BTI. In case of static linked executables or in case
of the dynamic linker itself, PROT_BTI protection is done by the
operating system.

On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
the properties of a binary because PT_NOTE can be unreliable with
old linkers (old linkers just append the notes of input objects
together and add them to the output without checking them for
consistency which means multiple incompatible GNU property notes
can be present in PT_NOTE). A new _dl_process_pt_gnu_property
hook is introduced in dl-prop.h and to keep it maintainable the
rtld and dlopen code paths use the same function (if the main
map needs special treatment, that should be inferred by the hook
from the link map). Unlike the _dt_process_pt_note hook this one
is called after segments are mapped to avoid unbounded allocation
and additional read syscall. Otherwise the AArch64 logic follows
the x86 logic for handling GNU properties (but the code is not
shared because x86 needs to manage internal CET state and look
out for multiple property notes).

BTI property is handled in the loader even if glibc is not built
with BTI support, so in theory user code can be BTI protected
independently of glibc. In practice though user binaries are not
marked with the BTI property if glibc has no support because the
static linked libc objects (crt files, libc_nonshared.a) are
unmarked.

This patch relies on Linux userspace API that is scheduled to be
merged in Linux 5.8 and now it is in the for-next/bti-user branch
of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git.

Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
 elf/dl-load.c                                 |  13 ++
 elf/rtld.c                                    |   6 +
 sysdeps/aarch64/Makefile                      |   4 +
 sysdeps/aarch64/dl-bti.c                      |  54 +++++++
 sysdeps/aarch64/dl-prop.h                     | 145 ++++++++++++++++++
 sysdeps/aarch64/linkmap.h                     |   3 +
 sysdeps/generic/dl-prop.h                     |  16 +-
 sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h  |   1 +
 sysdeps/unix/sysv/linux/aarch64/bits/mman.h   |  31 ++++
 .../unix/sysv/linux/aarch64/cpu-features.c    |   3 +
 .../unix/sysv/linux/aarch64/cpu-features.h    |   2 +
 sysdeps/x86/dl-prop.h                         |   6 +
 12 files changed, 279 insertions(+), 5 deletions(-)
 create mode 100644 sysdeps/aarch64/dl-bti.c
 create mode 100644 sysdeps/aarch64/dl-prop.h
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h

Comments

Adhemerval Zanella May 25, 2020, 7:53 p.m. UTC | #1
On 15/05/2020 11:40, Szabolcs Nagy wrote:
> From: Sudakshina Das <sudi.das@arm.com>
> 
> Binaries can opt-in to using BTI via an ELF object file marking.
> The dynamic linker has to then mprotect the executable segments
> with PROT_BTI. In case of static linked executables or in case
> of the dynamic linker itself, PROT_BTI protection is done by the
> operating system.
> 
> On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
> the properties of a binary because PT_NOTE can be unreliable with
> old linkers (old linkers just append the notes of input objects
> together and add them to the output without checking them for
> consistency which means multiple incompatible GNU property notes
> can be present in PT_NOTE). A new _dl_process_pt_gnu_property
> hook is introduced in dl-prop.h and to keep it maintainable the
> rtld and dlopen code paths use the same function (if the main
> map needs special treatment, that should be inferred by the hook
> from the link map). Unlike the _dt_process_pt_note hook this one
> is called after segments are mapped to avoid unbounded allocation
> and additional read syscall. Otherwise the AArch64 logic follows
> the x86 logic for handling GNU properties (but the code is not
> shared because x86 needs to manage internal CET state and look
> out for multiple property notes).
> 
> BTI property is handled in the loader even if glibc is not built
> with BTI support, so in theory user code can be BTI protected
> independently of glibc. In practice though user binaries are not
> marked with the BTI property if glibc has no support because the
> static linked libc objects (crt files, libc_nonshared.a) are
> unmarked.
> 
> This patch relies on Linux userspace API that is scheduled to be
> merged in Linux 5.8 and now it is in the for-next/bti-user branch
> of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git.
> 
> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>

LGTM with a just a nit below.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> ---
>  elf/dl-load.c                                 |  13 ++
>  elf/rtld.c                                    |   6 +
>  sysdeps/aarch64/Makefile                      |   4 +
>  sysdeps/aarch64/dl-bti.c                      |  54 +++++++
>  sysdeps/aarch64/dl-prop.h                     | 145 ++++++++++++++++++
>  sysdeps/aarch64/linkmap.h                     |   3 +
>  sysdeps/generic/dl-prop.h                     |  16 +-
>  sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h  |   1 +
>  sysdeps/unix/sysv/linux/aarch64/bits/mman.h   |  31 ++++
>  .../unix/sysv/linux/aarch64/cpu-features.c    |   3 +
>  .../unix/sysv/linux/aarch64/cpu-features.h    |   2 +
>  sysdeps/x86/dl-prop.h                         |   6 +
>  12 files changed, 279 insertions(+), 5 deletions(-)
>  create mode 100644 sysdeps/aarch64/dl-bti.c
>  create mode 100644 sysdeps/aarch64/dl-prop.h
>  create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 06f2ba7264..9c37ec1098 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1188,6 +1188,19 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>  				  maplength, has_holes, loader);
>      if (__glibc_unlikely (errstring != NULL))
>        goto call_lose;
> +
> +    /* Process program headers again after load segments are mapped.  */

Maybe add a brief explanation of why it is done after load segment mapping?

> +    for (ph = phdr; ph < &phdr[l->l_phnum]; ++ph)
> +      switch (ph->p_type)
> +	{
> +	case PT_GNU_PROPERTY:
> +	  if (_dl_process_pt_gnu_property (l, ph))
> +	    {
> +	      errstring = N_("cannot process GNU property segment");
> +	      goto call_lose;
> +	    }
> +	  break;
> +	}
>    }
>  
>    if (l->l_ld == 0)

Ok.

> diff --git a/elf/rtld.c b/elf/rtld.c
> index 5ccc3c2dbb..97a0bbf4dc 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1506,6 +1506,12 @@ of this helper program; chances are you did not intend to run this program.\n\
>  	main_map->l_relro_size = ph->p_memsz;
>  	break;
>  
> +      case PT_GNU_PROPERTY:
> +	if (_dl_process_pt_gnu_property (main_map, ph))
> +	  _dl_error_printf (
> +"ERROR: '%s': cannot process GNU property segment.\n", _dl_argv[0]);
> +	break;
> +
>        case PT_NOTE:
>  	if (_rtld_process_pt_note (main_map, ph))
>  	  _dl_error_printf ("\

Ok.

> diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> index 9cb141004d..5ae8b082b0 100644
> --- a/sysdeps/aarch64/Makefile
> +++ b/sysdeps/aarch64/Makefile
> @@ -1,5 +1,9 @@
>  long-double-fcts = yes
>  
> +ifeq ($(subdir),elf)
> +sysdep-dl-routines += dl-bti
> +endif
> +
>  ifeq ($(subdir),elf)
>  sysdep-dl-routines += tlsdesc dl-tlsdesc
>  gen-as-const-headers += dl-link.sym

Ok.

> diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
> new file mode 100644
> index 0000000000..6003686601
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-bti.c
> @@ -0,0 +1,54 @@
> +/* AArch64 BTI functions.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <unistd.h>
> +#include <errno.h>
> +#include <libintl.h>
> +#include <ldsodefs.h>
> +
> +static int
> +enable_bti (struct link_map *map, const char *program)
> +{
> +  const ElfW(Phdr) *phdr;
> +  unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
> +
> +  for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
> +    if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
> +      {
> +	ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
> +	ElfW(Addr) len = phdr->p_memsz;
> +	if (__mprotect ((void *) start, len, prot) < 0)
> +	  {
> +	    if (program)
> +	      _dl_fatal_printf ("%s: mprotect failed to turn on BTI\n",
> +				map->l_name);
> +	    else
> +	      _dl_signal_error (EINVAL, map->l_name, "dlopen",
> +				N_("mprotect failed to turn on BTI"));

Is EINVAL the only possible error here (EACCES or ENOMEM might be still
possible)?

> +	  }
> +      }
> +  return 0;
> +}
> +
> +/* Enable BTI for L if required.  */
> +
> +void
> +_dl_bti_check (struct link_map *l, const char *program)
> +{
> +  if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti)
> +    enable_bti (l, program);
> +}

Ok.

> diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
> new file mode 100644
> index 0000000000..b6f8a88667
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-prop.h
> @@ -0,0 +1,145 @@
> +/* Support for GNU properties.  AArch64 version.
> +   Copyright (C) 2018-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DL_PROP_H
> +#define _DL_PROP_H
> +
> +#include <not-cancel.h>
> +
> +extern void _dl_bti_check (struct link_map *, const char *)
> +    attribute_hidden;
> +
> +static inline void __attribute__ ((always_inline))
> +_rtld_main_check (struct link_map *m, const char *program)
> +{
> +  _dl_bti_check (m, program);
> +}
> +
> +static inline void __attribute__ ((always_inline))
> +_dl_open_check (struct link_map *m)
> +{
> +  _dl_bti_check (m, NULL);
> +}
> +
> +static inline void __attribute__ ((unused))
> +_dl_process_aarch64_property (struct link_map *l,
> +			      const ElfW(Nhdr) *note,
> +			      const ElfW(Addr) size,
> +			      const ElfW(Addr) align)
> +{
> +  /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
> +     32-bit objects and to 8 bytes in 64-bit objects.  Skip notes
> +     with incorrect alignment.  */
> +  if (align != (__ELF_NATIVE_CLASS / 8))
> +    return;
> +
> +  const ElfW(Addr) start = (ElfW(Addr)) note;
> +
> +  unsigned int feature_1 = 0;
> +  unsigned int last_type = 0;
> +
> +  while ((ElfW(Addr)) (note + 1) - start < size)
> +    {
> +      /* Find the NT_GNU_PROPERTY_TYPE_0 note.  */
> +      if (note->n_namesz == 4
> +	  && note->n_type == NT_GNU_PROPERTY_TYPE_0
> +	  && memcmp (note + 1, "GNU", 4) == 0)
> +	{
> +	  /* Check for invalid property.  */
> +	  if (note->n_descsz < 8
> +	      || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
> +	    return;
> +
> +	  /* Start and end of property array.  */
> +	  unsigned char *ptr = (unsigned char *) (note + 1) + 4;
> +	  unsigned char *ptr_end = ptr + note->n_descsz;
> +
> +	  do
> +	    {
> +	      unsigned int type = *(unsigned int *) ptr;
> +	      unsigned int datasz = *(unsigned int *) (ptr + 4);
> +
> +	      /* Property type must be in ascending order.  */
> +	      if (type < last_type)
> +		return;
> +
> +	      ptr += 8;
> +	      if ((ptr + datasz) > ptr_end)
> +		return;
> +
> +	      last_type = type;
> +
> +	      if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
> +		{
> +		  /* The size of GNU_PROPERTY_AARCH64_FEATURE_1_AND is 4
> +		     bytes.  When seeing GNU_PROPERTY_AARCH64_FEATURE_1_AND,
> +		     we stop the search regardless if its size is correct
> +		     or not.  There is no point to continue if this note
> +		     is ill-formed.  */
> +		  if (datasz != 4)
> +		    return;
> +
> +		  feature_1 = *(unsigned int *) ptr;
> +		  if ((feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI))
> +		    l->l_mach.bti = true;
> +
> +		  /* Stop if we found the property note.  */
> +		  return;
> +		}
> +	      else if (type > GNU_PROPERTY_AARCH64_FEATURE_1_AND)
> +		{
> +		  /* Stop since property type is in ascending order.  */
> +		  return;
> +		}
> +
> +	      /* Check the next property item.  */
> +	      ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> +	    }
> +	  while ((ptr_end - ptr) >= 8);
> +	}
> +
> +      note = ((const void *) note
> +	      + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
> +				      align));
> +    }
> +}
> +
> +#ifdef FILEBUF_SIZE
> +static inline int __attribute__ ((always_inline))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
> +		     int fd, struct filebuf *fbp)
> +{
> +  return 0;
> +}
> +#endif
> +
> +static inline int __attribute__ ((always_inline))
> +_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> +  return 0;
> +}
> +
> +static inline int
> +_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> +  const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
> +  _dl_process_aarch64_property (l, note, ph->p_memsz, ph->p_align);
> +  return 0;
> +}
> +
> +#endif /* _DL_PROP_H */

Ok.

> diff --git a/sysdeps/aarch64/linkmap.h b/sysdeps/aarch64/linkmap.h
> index 943a9ee9e4..847a03ace2 100644
> --- a/sysdeps/aarch64/linkmap.h
> +++ b/sysdeps/aarch64/linkmap.h
> @@ -16,8 +16,11 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +#include <stdbool.h>
> +
>  struct link_map_machine
>  {
>    ElfW(Addr) plt;	  /* Address of .plt */
>    void *tlsdesc_table;	  /* Address of TLS descriptor hash table.  */
> +  bool bti;		  /* Branch Target Identification is enabled.  */
>  };

Ok.

> diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
> index 6b0f2aa95a..4192049739 100644
> --- a/sysdeps/generic/dl-prop.h
> +++ b/sysdeps/generic/dl-prop.h
> @@ -20,11 +20,11 @@
>  #define _DL_PROP_H
>  
>  /* The following functions are used by the dynamic loader and the
> -   dlopen machinery to process PT_NOTE entries in the binary or
> -   shared object.  The notes can be used to change the behaviour of
> -   the loader, and as such offer a flexible mechanism for hooking in
> -   various checks related to ABI tags or implementing "flag day" ABI
> -   transitions.  */
> +   dlopen machinery to process PT_NOTE and PT_GNU_PROPERTY entries in
> +   the binary or shared object.  The notes can be used to change the
> +   behaviour of the loader, and as such offer a flexible mechanism
> +   for hooking in various checks related to ABI tags or implementing
> +   "flag day" ABI transitions.  */
>  
>  static inline void __attribute__ ((always_inline))
>  _rtld_main_check (struct link_map *m, const char *program)
> @@ -51,4 +51,10 @@ _rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
>    return 0;
>  }
>  
> +static inline int __attribute__ ((always_inline))
> +_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> +  return 0;
> +}
> +
>  #endif /* _DL_PROP_H */

Ok.

> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> index 4ee14b4208..af90d8a626 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> @@ -72,3 +72,4 @@
>  #define HWCAP2_BF16		(1 << 14)
>  #define HWCAP2_DGH		(1 << 15)
>  #define HWCAP2_RNG		(1 << 16)
> +#define HWCAP2_BTI		(1 << 17)

Ok.

> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> new file mode 100644
> index 0000000000..ecae046344
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> @@ -0,0 +1,31 @@
> +/* Definitions for POSIX memory map interface.  Linux/AArch64 version.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_MMAN_H
> +# error "Never use <bits/mman.h> directly; include <sys/mman.h> instead."
> +#endif
> +
> +/* AArch64 specific definitions, should be in sync with
> +   arch/arm64/include/uapi/asm/mman.h.  */
> +
> +#define PROT_BTI	0x10
> +
> +#include <bits/mman-map-flags-generic.h>
> +
> +/* Include generic Linux declarations.  */
> +#include <bits/mman-linux.h>

Ok (although I am still not sure if this should be inside a __USE_MISC).

> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> index 896c588fee..b9ab827aca 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -83,4 +83,7 @@ init_cpu_features (struct cpu_features *cpu_features)
>  
>    if ((dczid & DCZID_DZP_MASK) == 0)
>      cpu_features->zva_size = 4 << (dczid & DCZID_BS_MASK);
> +
> +  /* Check if BTI is supported.  */
> +  cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
>  }

Ok.

> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> index 1389cea1b3..a81f186ec2 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> @@ -20,6 +20,7 @@
>  #define _CPU_FEATURES_AARCH64_H
>  
>  #include <stdint.h>
> +#include <stdbool.h>
>  
>  #define MIDR_PARTNUM_SHIFT	4
>  #define MIDR_PARTNUM_MASK	(0xfff << MIDR_PARTNUM_SHIFT)
> @@ -64,6 +65,7 @@ struct cpu_features
>  {
>    uint64_t midr_el1;
>    unsigned zva_size;
> +  bool bti;
>  };
>  
>  #endif /* _CPU_FEATURES_AARCH64_H  */

Ok.

> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index 516f88ea80..8649314f9d 100644
> --- a/sysdeps/x86/dl-prop.h
> +++ b/sysdeps/x86/dl-prop.h
> @@ -191,4 +191,10 @@ _rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
>    return 0;
>  }
>  
> +static inline int __attribute__ ((always_inline))
> +_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> +  return 0;
> +}
> +
>  #endif /* _DL_PROP_H */
> 

Ok.
Szabolcs Nagy May 26, 2020, 11:20 a.m. UTC | #2
The 05/25/2020 16:53, Adhemerval Zanella wrote:
> On 15/05/2020 11:40, Szabolcs Nagy wrote:
> > From: Sudakshina Das <sudi.das@arm.com>
> > 
> > Binaries can opt-in to using BTI via an ELF object file marking.
> > The dynamic linker has to then mprotect the executable segments
> > with PROT_BTI. In case of static linked executables or in case
> > of the dynamic linker itself, PROT_BTI protection is done by the
> > operating system.
> > 
> > On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
> > the properties of a binary because PT_NOTE can be unreliable with
> > old linkers (old linkers just append the notes of input objects
> > together and add them to the output without checking them for
> > consistency which means multiple incompatible GNU property notes
> > can be present in PT_NOTE). A new _dl_process_pt_gnu_property
> > hook is introduced in dl-prop.h and to keep it maintainable the
> > rtld and dlopen code paths use the same function (if the main
> > map needs special treatment, that should be inferred by the hook
> > from the link map). Unlike the _dt_process_pt_note hook this one
> > is called after segments are mapped to avoid unbounded allocation
> > and additional read syscall. Otherwise the AArch64 logic follows
> > the x86 logic for handling GNU properties (but the code is not
> > shared because x86 needs to manage internal CET state and look
> > out for multiple property notes).
> > 
> > BTI property is handled in the loader even if glibc is not built
> > with BTI support, so in theory user code can be BTI protected
> > independently of glibc. In practice though user binaries are not
> > marked with the BTI property if glibc has no support because the
> > static linked libc objects (crt files, libc_nonshared.a) are
> > unmarked.
> > 
> > This patch relies on Linux userspace API that is scheduled to be
> > merged in Linux 5.8 and now it is in the for-next/bti-user branch
> > of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git.
> > 
> > Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> 
> LGTM with a just a nit below.
> 
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

thanks for the review.

> > @@ -1188,6 +1188,19 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> >  				  maplength, has_holes, loader);
> >      if (__glibc_unlikely (errstring != NULL))
> >        goto call_lose;
> > +
> > +    /* Process program headers again after load segments are mapped.  */
> 
> Maybe add a brief explanation of why it is done after load segment mapping?
> 
> > +    for (ph = phdr; ph < &phdr[l->l_phnum]; ++ph)
> > +      switch (ph->p_type)
> > +	{
> > +	case PT_GNU_PROPERTY:
> > +	  if (_dl_process_pt_gnu_property (l, ph))
> > +	    {
> > +	      errstring = N_("cannot process GNU property segment");
> > +	      goto call_lose;
> > +	    }
> > +	  break;
> > +	}

btw i think the _dl_process_pt_note callback should
be done here too and x86 fixed up accordingly so it
does not need unbounded allocation + pread_nocancel
to process the notes.

> > +static int
> > +enable_bti (struct link_map *map, const char *program)
> > +{
> > +  const ElfW(Phdr) *phdr;
> > +  unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
> > +
> > +  for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
> > +    if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
> > +      {
> > +	ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
> > +	ElfW(Addr) len = phdr->p_memsz;
> > +	if (__mprotect ((void *) start, len, prot) < 0)
> > +	  {
> > +	    if (program)
> > +	      _dl_fatal_printf ("%s: mprotect failed to turn on BTI\n",
> > +				map->l_name);
> > +	    else
> > +	      _dl_signal_error (EINVAL, map->l_name, "dlopen",
> > +				N_("mprotect failed to turn on BTI"));
> 
> Is EINVAL the only possible error here (EACCES or ENOMEM might be still
> possible)?

no, i think passing errno makes more sense here.
i'll fix it.
diff mbox series

Patch

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 06f2ba7264..9c37ec1098 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1188,6 +1188,19 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 				  maplength, has_holes, loader);
     if (__glibc_unlikely (errstring != NULL))
       goto call_lose;
+
+    /* Process program headers again after load segments are mapped.  */
+    for (ph = phdr; ph < &phdr[l->l_phnum]; ++ph)
+      switch (ph->p_type)
+	{
+	case PT_GNU_PROPERTY:
+	  if (_dl_process_pt_gnu_property (l, ph))
+	    {
+	      errstring = N_("cannot process GNU property segment");
+	      goto call_lose;
+	    }
+	  break;
+	}
   }
 
   if (l->l_ld == 0)
diff --git a/elf/rtld.c b/elf/rtld.c
index 5ccc3c2dbb..97a0bbf4dc 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1506,6 +1506,12 @@  of this helper program; chances are you did not intend to run this program.\n\
 	main_map->l_relro_size = ph->p_memsz;
 	break;
 
+      case PT_GNU_PROPERTY:
+	if (_dl_process_pt_gnu_property (main_map, ph))
+	  _dl_error_printf (
+"ERROR: '%s': cannot process GNU property segment.\n", _dl_argv[0]);
+	break;
+
       case PT_NOTE:
 	if (_rtld_process_pt_note (main_map, ph))
 	  _dl_error_printf ("\
diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
index 9cb141004d..5ae8b082b0 100644
--- a/sysdeps/aarch64/Makefile
+++ b/sysdeps/aarch64/Makefile
@@ -1,5 +1,9 @@ 
 long-double-fcts = yes
 
+ifeq ($(subdir),elf)
+sysdep-dl-routines += dl-bti
+endif
+
 ifeq ($(subdir),elf)
 sysdep-dl-routines += tlsdesc dl-tlsdesc
 gen-as-const-headers += dl-link.sym
diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
new file mode 100644
index 0000000000..6003686601
--- /dev/null
+++ b/sysdeps/aarch64/dl-bti.c
@@ -0,0 +1,54 @@ 
+/* AArch64 BTI functions.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+#include <errno.h>
+#include <libintl.h>
+#include <ldsodefs.h>
+
+static int
+enable_bti (struct link_map *map, const char *program)
+{
+  const ElfW(Phdr) *phdr;
+  unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
+
+  for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
+    if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
+      {
+	ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
+	ElfW(Addr) len = phdr->p_memsz;
+	if (__mprotect ((void *) start, len, prot) < 0)
+	  {
+	    if (program)
+	      _dl_fatal_printf ("%s: mprotect failed to turn on BTI\n",
+				map->l_name);
+	    else
+	      _dl_signal_error (EINVAL, map->l_name, "dlopen",
+				N_("mprotect failed to turn on BTI"));
+	  }
+      }
+  return 0;
+}
+
+/* Enable BTI for L if required.  */
+
+void
+_dl_bti_check (struct link_map *l, const char *program)
+{
+  if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti)
+    enable_bti (l, program);
+}
diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
new file mode 100644
index 0000000000..b6f8a88667
--- /dev/null
+++ b/sysdeps/aarch64/dl-prop.h
@@ -0,0 +1,145 @@ 
+/* Support for GNU properties.  AArch64 version.
+   Copyright (C) 2018-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _DL_PROP_H
+#define _DL_PROP_H
+
+#include <not-cancel.h>
+
+extern void _dl_bti_check (struct link_map *, const char *)
+    attribute_hidden;
+
+static inline void __attribute__ ((always_inline))
+_rtld_main_check (struct link_map *m, const char *program)
+{
+  _dl_bti_check (m, program);
+}
+
+static inline void __attribute__ ((always_inline))
+_dl_open_check (struct link_map *m)
+{
+  _dl_bti_check (m, NULL);
+}
+
+static inline void __attribute__ ((unused))
+_dl_process_aarch64_property (struct link_map *l,
+			      const ElfW(Nhdr) *note,
+			      const ElfW(Addr) size,
+			      const ElfW(Addr) align)
+{
+  /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
+     32-bit objects and to 8 bytes in 64-bit objects.  Skip notes
+     with incorrect alignment.  */
+  if (align != (__ELF_NATIVE_CLASS / 8))
+    return;
+
+  const ElfW(Addr) start = (ElfW(Addr)) note;
+
+  unsigned int feature_1 = 0;
+  unsigned int last_type = 0;
+
+  while ((ElfW(Addr)) (note + 1) - start < size)
+    {
+      /* Find the NT_GNU_PROPERTY_TYPE_0 note.  */
+      if (note->n_namesz == 4
+	  && note->n_type == NT_GNU_PROPERTY_TYPE_0
+	  && memcmp (note + 1, "GNU", 4) == 0)
+	{
+	  /* Check for invalid property.  */
+	  if (note->n_descsz < 8
+	      || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
+	    return;
+
+	  /* Start and end of property array.  */
+	  unsigned char *ptr = (unsigned char *) (note + 1) + 4;
+	  unsigned char *ptr_end = ptr + note->n_descsz;
+
+	  do
+	    {
+	      unsigned int type = *(unsigned int *) ptr;
+	      unsigned int datasz = *(unsigned int *) (ptr + 4);
+
+	      /* Property type must be in ascending order.  */
+	      if (type < last_type)
+		return;
+
+	      ptr += 8;
+	      if ((ptr + datasz) > ptr_end)
+		return;
+
+	      last_type = type;
+
+	      if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
+		{
+		  /* The size of GNU_PROPERTY_AARCH64_FEATURE_1_AND is 4
+		     bytes.  When seeing GNU_PROPERTY_AARCH64_FEATURE_1_AND,
+		     we stop the search regardless if its size is correct
+		     or not.  There is no point to continue if this note
+		     is ill-formed.  */
+		  if (datasz != 4)
+		    return;
+
+		  feature_1 = *(unsigned int *) ptr;
+		  if ((feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI))
+		    l->l_mach.bti = true;
+
+		  /* Stop if we found the property note.  */
+		  return;
+		}
+	      else if (type > GNU_PROPERTY_AARCH64_FEATURE_1_AND)
+		{
+		  /* Stop since property type is in ascending order.  */
+		  return;
+		}
+
+	      /* Check the next property item.  */
+	      ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
+	    }
+	  while ((ptr_end - ptr) >= 8);
+	}
+
+      note = ((const void *) note
+	      + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
+				      align));
+    }
+}
+
+#ifdef FILEBUF_SIZE
+static inline int __attribute__ ((always_inline))
+_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
+		     int fd, struct filebuf *fbp)
+{
+  return 0;
+}
+#endif
+
+static inline int __attribute__ ((always_inline))
+_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
+{
+  return 0;
+}
+
+static inline int
+_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
+{
+  const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
+  _dl_process_aarch64_property (l, note, ph->p_memsz, ph->p_align);
+  return 0;
+}
+
+#endif /* _DL_PROP_H */
diff --git a/sysdeps/aarch64/linkmap.h b/sysdeps/aarch64/linkmap.h
index 943a9ee9e4..847a03ace2 100644
--- a/sysdeps/aarch64/linkmap.h
+++ b/sysdeps/aarch64/linkmap.h
@@ -16,8 +16,11 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <stdbool.h>
+
 struct link_map_machine
 {
   ElfW(Addr) plt;	  /* Address of .plt */
   void *tlsdesc_table;	  /* Address of TLS descriptor hash table.  */
+  bool bti;		  /* Branch Target Identification is enabled.  */
 };
diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
index 6b0f2aa95a..4192049739 100644
--- a/sysdeps/generic/dl-prop.h
+++ b/sysdeps/generic/dl-prop.h
@@ -20,11 +20,11 @@ 
 #define _DL_PROP_H
 
 /* The following functions are used by the dynamic loader and the
-   dlopen machinery to process PT_NOTE entries in the binary or
-   shared object.  The notes can be used to change the behaviour of
-   the loader, and as such offer a flexible mechanism for hooking in
-   various checks related to ABI tags or implementing "flag day" ABI
-   transitions.  */
+   dlopen machinery to process PT_NOTE and PT_GNU_PROPERTY entries in
+   the binary or shared object.  The notes can be used to change the
+   behaviour of the loader, and as such offer a flexible mechanism
+   for hooking in various checks related to ABI tags or implementing
+   "flag day" ABI transitions.  */
 
 static inline void __attribute__ ((always_inline))
 _rtld_main_check (struct link_map *m, const char *program)
@@ -51,4 +51,10 @@  _rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
   return 0;
 }
 
+static inline int __attribute__ ((always_inline))
+_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
+{
+  return 0;
+}
+
 #endif /* _DL_PROP_H */
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
index 4ee14b4208..af90d8a626 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
@@ -72,3 +72,4 @@ 
 #define HWCAP2_BF16		(1 << 14)
 #define HWCAP2_DGH		(1 << 15)
 #define HWCAP2_RNG		(1 << 16)
+#define HWCAP2_BTI		(1 << 17)
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
new file mode 100644
index 0000000000..ecae046344
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
@@ -0,0 +1,31 @@ 
+/* Definitions for POSIX memory map interface.  Linux/AArch64 version.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SYS_MMAN_H
+# error "Never use <bits/mman.h> directly; include <sys/mman.h> instead."
+#endif
+
+/* AArch64 specific definitions, should be in sync with
+   arch/arm64/include/uapi/asm/mman.h.  */
+
+#define PROT_BTI	0x10
+
+#include <bits/mman-map-flags-generic.h>
+
+/* Include generic Linux declarations.  */
+#include <bits/mman-linux.h>
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
index 896c588fee..b9ab827aca 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
@@ -83,4 +83,7 @@  init_cpu_features (struct cpu_features *cpu_features)
 
   if ((dczid & DCZID_DZP_MASK) == 0)
     cpu_features->zva_size = 4 << (dczid & DCZID_BS_MASK);
+
+  /* Check if BTI is supported.  */
+  cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
 }
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
index 1389cea1b3..a81f186ec2 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
@@ -20,6 +20,7 @@ 
 #define _CPU_FEATURES_AARCH64_H
 
 #include <stdint.h>
+#include <stdbool.h>
 
 #define MIDR_PARTNUM_SHIFT	4
 #define MIDR_PARTNUM_MASK	(0xfff << MIDR_PARTNUM_SHIFT)
@@ -64,6 +65,7 @@  struct cpu_features
 {
   uint64_t midr_el1;
   unsigned zva_size;
+  bool bti;
 };
 
 #endif /* _CPU_FEATURES_AARCH64_H  */
diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index 516f88ea80..8649314f9d 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -191,4 +191,10 @@  _rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
   return 0;
 }
 
+static inline int __attribute__ ((always_inline))
+_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
+{
+  return 0;
+}
+
 #endif /* _DL_PROP_H */