[09/12] aarch64: support BTI enabled binaries
Commit Message
Comments
On 30/04/2020 14:44, Szabolcs Nagy wrote:
> From 45c6bce5a691ecec9bba52785bd1f3a4cbc76fd4 Mon Sep 17 00:00:00 2001
> From: Sudakshina Das <sudi.das@arm.com>
> Date: Tue, 17 Mar 2020 15:54:12 +0000
> Subject: [PATCH 09/12] aarch64: support BTI enabled binaries
>
> Binaries can opt-in to using BTI via an ELF property marking.
> The dynamic linker has to then mprotect the executable segments
> with PROT_BTI. In case of static linked executables or in case
> of the dynamic linker itself, PROT_BTI protection is done by the
> operating system.
>
> On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
> the properties of a binary because PT_NOTE can be unreliable with
> old linkers.
>
> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> ---
> elf/dl-load.c | 2 +
> elf/rtld.c | 2 +
> sysdeps/aarch64/Makefile | 4 +
> sysdeps/aarch64/dl-bti.c | 54 ++++++
> sysdeps/aarch64/dl-prop.h | 170 ++++++++++++++++++
> sysdeps/aarch64/linkmap.h | 1 +
> sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
> sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 ++++
> .../unix/sysv/linux/aarch64/cpu-features.c | 3 +
> .../unix/sysv/linux/aarch64/cpu-features.h | 1 +
> 10 files changed, 269 insertions(+)
> create mode 100644 sysdeps/aarch64/dl-bti.c
> create mode 100644 sysdeps/aarch64/dl-prop.h
> create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index a6b80f9395..0930250619 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1145,6 +1145,8 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> l->l_relro_size = ph->p_memsz;
> break;
>
> + case PT_GNU_PROPERTY:
> + /* Fall through. PT_GNU_PROPERTY holds property notes. */
> case PT_NOTE:
> if (_dl_process_pt_note (l, ph, fd, fbp))
> {
This will print the same error message for a failure in _dl_process_pt_note
("cannot process note segment"). Wouldn't be better to use a more specific
error message, like "cannot process GNU property segment"?
> diff --git a/elf/rtld.c b/elf/rtld.c
> index b2ea21c98b..88b8e74de0 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1505,6 +1505,8 @@ of this helper program; chances are you did not intend to run this program.\n\
> main_map->l_relro_size = ph->p_memsz;
> break;
>
> + case PT_GNU_PROPERTY:
> + /* Fall through. PT_GNU_PROPERTY holds property notes. */
> case PT_NOTE:
> if (_rtld_process_pt_note (main_map, ph))
> _dl_error_printf ("\
As before.
> diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> index 9cb141004d..5ae8b082b0 100644
> --- a/sysdeps/aarch64/Makefile
> +++ b/sysdeps/aarch64/Makefile
> @@ -1,5 +1,9 @@
> long-double-fcts = yes
>
> +ifeq ($(subdir),elf)
> +sysdep-dl-routines += dl-bti
> +endif
> +
> ifeq ($(subdir),elf)
> sysdep-dl-routines += tlsdesc dl-tlsdesc
> gen-as-const-headers += dl-link.sym
Ok.
> diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
> new file mode 100644
> index 0000000000..9ce697527d
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-bti.c
> @@ -0,0 +1,54 @@
> +/* AArch64 BTI initializers function.
'functions' maybe?
> + Copyright (C) 2020 Free Software Foundation, Inc.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <unistd.h>
> +#include <errno.h>
> +#include <libintl.h>
> +#include <ldsodefs.h>
> +
> +static int
> +enable_bti (struct link_map *map, const char *program)
> +{
> + const ElfW(Phdr) *phdr;
> + unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
> +
> + for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
> + if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
> + {
> + ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
> + ElfW(Addr) len = phdr->p_memsz;
> + if (__mprotect ((void *)start, len, prot) < 0)
Space after cast.
> + {
> + if (program)
> + _dl_fatal_printf ("%s: mprotect failed to turn on BTI\n",
> + map->l_name);
> + else
> + _dl_signal_error (EINVAL, map->l_name, "dlopen",
> + N_("mprotect failed to turn on BTI"));
> + }
> + }
> + return 0;
> +}
> +
> +/* Enable BTI for L if required. */
> +
> +void
> +_dl_bti_check (struct link_map *l, const char *program)
> +{
> + if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti_guarded)
> + enable_bti (l, program);
> +}
No implicit checks (since there are defined both as int).
> diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
> new file mode 100644
> index 0000000000..6662e4ab14
> --- /dev/null
> +++ b/sysdeps/aarch64/dl-prop.h
> @@ -0,0 +1,170 @@
> +/* Support for GNU properties. AArch64 version.
> + Copyright (C) 2018-2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#ifndef _DL_PROP_H
> +#define _DL_PROP_H
> +
> +#include <not-cancel.h>
> +
> +extern void _dl_bti_check (struct link_map *, const char *)
> + attribute_hidden;
> +
> +static inline void __attribute__ ((always_inline))
> +_rtld_main_check (struct link_map *m, const char *program)
> +{
> + _dl_bti_check (m, program);
> +}
> +
> +static inline void __attribute__ ((always_inline))
> +_dl_open_check (struct link_map *m)
> +{
> + _dl_bti_check (m, 0);
It should be NULL here instead of 0 (glibc code guideline).
> +}
> +
> +static inline void __attribute__ ((unused))
> +_dl_process_aarch64_property (struct link_map *l,
> + const ElfW(Nhdr) *note,
> + const ElfW(Addr) size,
> + const ElfW(Addr) align)
> +{
> + /* The NT_GNU_PROPERTY_TYPE_0 note must be aliged to 4 bytes in
s/aliged/aligned
> + 32-bit objects and to 8 bytes in 64-bit objects. Skip notes
> + with incorrect alignment. */
> + if (align != (__ELF_NATIVE_CLASS / 8))
> + return;
> +
> + const ElfW(Addr) start = (ElfW(Addr)) note;
> +
> + unsigned int feature_1 = 0;
> + unsigned int last_type = 0;
> +
> + while ((ElfW(Addr)) (note + 1) - start < size)
> + {
> + /* Find the NT_GNU_PROPERTY_TYPE_0 note. */
> + if (note->n_namesz == 4
> + && note->n_type == NT_GNU_PROPERTY_TYPE_0
> + && memcmp (note + 1, "GNU", 4) == 0)
> + {
> + /* Check for invalid property. */
> + if (note->n_descsz < 8
> + || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
> + return;
> +
> + /* Start and end of property array. */
> + unsigned char *ptr = (unsigned char *) (note + 1) + 4;
> + unsigned char *ptr_end = ptr + note->n_descsz;
> +
> + do
> + {
> + unsigned int type = *(unsigned int *) ptr;
> + unsigned int datasz = *(unsigned int *) (ptr + 4);
> +
> + /* Property type must be in ascending order. */
> + if (type < last_type)
> + return;
> +
> + ptr += 8;
> + if ((ptr + datasz) > ptr_end)
> + return;
> +
> + last_type = type;
The logic to parse the PT_GNU_PROPERTY is quite similar to the one
at sysdeps/x86/dl-prop.h to parse PT_NOTE. Would it be possible to
maybe try to consolidate the logic somewhere to avoid this code
duplication?
> +
> + if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
> + {
> + /* The size of GNU_PROPERTY_AARCH64_FEATURE_1_AND is 4
> + bytes. When seeing GNU_PROPERTY_AARCH64_FEATURE_1_AND,
> + we stop the search regardless if its size is correct
> + or not. There is no point to continue if this note
> + is ill-formed. */
> + if (datasz != 4)
> + return;
> +
> + feature_1 = *(unsigned int *) ptr;
> + if ((feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI))
> + l->l_mach.bti_guarded = true;
> +
> + /* Stop if we found the property note. */
> + return;
> + }
> + else if (type > GNU_PROPERTY_AARCH64_FEATURE_1_AND)
> + {
> + /* Stop since property type is in ascending order. */
> + return;
> + }
> +
> + /* Check the next property item. */
> + ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> + }
> + while ((ptr_end - ptr) >= 8);
> + }
> +
> + /* NB: Note sections like .note.ABI-tag and .note.gnu.build-id are
> + aligned to 4 bytes in 64-bit ELF objects. */
> + note = ((const void *) note
> + + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
> + align));
> + }
> +}
> +
> +#ifdef FILEBUF_SIZE
> +static inline int __attribute__ ((unused))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
> + int fd, struct filebuf *fbp)
> +{
> + if (ph->p_type != PT_GNU_PROPERTY)
> + return 0;
> +
> + const ElfW(Nhdr) *note;
> + ElfW(Nhdr) *note_malloced = NULL;
> + ElfW(Addr) size = ph->p_filesz;
> +
> + if (ph->p_offset + size <= (size_t) fbp->len)
> + note = (const void *) (fbp->buf + ph->p_offset);
> + else
> + {
> + if (size < __MAX_ALLOCA_CUTOFF)
> + note = alloca (size);
> + else
> + note = note_malloced = malloc (size);
> + if (note == NULL)
> + return -1;
> + if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
> + {
> + if (note_malloced)
> + free (note_malloced);
> + return -1;
I wonder if we should use a scratch_buffer here instead.
> + }
> + }
> + _dl_process_aarch64_property (l, note, ph->p_filesz, ph->p_align);
> + if (note_malloced)
> + free (note_malloced);
> + return 0;
> +}
> +#endif
> +
> +static inline int __attribute__ ((unused))
> +_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> + if (ph->p_type != PT_GNU_PROPERTY)
> + return 0;
Not sure this is the right design to use the same function to handle
both PT_NOTE and PT_GNU_PROPERTY.
> + const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
> + _dl_process_aarch64_property (l, note, ph->p_memsz, ph->p_align);
> + return 0;
> +}
> +
> +#endif /* _DL_PROP_H */
> diff --git a/sysdeps/aarch64/linkmap.h b/sysdeps/aarch64/linkmap.h
> index 943a9ee9e4..cc196512d7 100644
> --- a/sysdeps/aarch64/linkmap.h
> +++ b/sysdeps/aarch64/linkmap.h
> @@ -20,4 +20,5 @@ struct link_map_machine
> {
> ElfW(Addr) plt; /* Address of .plt */
> void *tlsdesc_table; /* Address of TLS descriptor hash table. */
> + int bti_guarded; /* Branch Target Identification mechanism enabled. */
Maybe bool here?
> };
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> index 4ee14b4208..af90d8a626 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> @@ -72,3 +72,4 @@
> #define HWCAP2_BF16 (1 << 14)
> #define HWCAP2_DGH (1 << 15)
> #define HWCAP2_RNG (1 << 16)
> +#define HWCAP2_BTI (1 << 17)
This it not yet upstream on Linus tree (6e7f2eacf098), but follows
the arm64/for-next/bti branch (8ef8f360cf30be12).
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> new file mode 100644
> index 0000000000..ecae046344
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> @@ -0,0 +1,31 @@
> +/* Definitions for POSIX memory map interface. Linux/AArch64 version.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#ifndef _SYS_MMAN_H
> +# error "Never use <bits/mman.h> directly; include <sys/mman.h> instead."
> +#endif
> +
> +/* AArch64 specific definitions, should be in sync with
> + arch/arm64/include/uapi/asm/mman.h. */
> +
> +#define PROT_BTI 0x10
Linux specific flags should be protected by __USE_MISC.
> +
> +#include <bits/mman-map-flags-generic.h>
> +
> +/* Include generic Linux declarations. */
> +#include <bits/mman-linux.h>
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> index 896c588fee..c2385fb498 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -83,4 +83,7 @@ init_cpu_features (struct cpu_features *cpu_features)
>
> if ((dczid & DCZID_DZP_MASK) == 0)
> cpu_features->zva_size = 4 << (dczid & DCZID_BS_MASK);
> +
> + /* Check if BTI is enabled. */
> + cpu_features->bti = (GLRO (dl_hwcap2) & HWCAP2_BTI);
> }
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> index 1389cea1b3..88983eb723 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> @@ -64,6 +64,7 @@ struct cpu_features
> {
> uint64_t midr_el1;
> unsigned zva_size;
> + int bti;
Maybe bool here?
> };
>
> #endif /* _CPU_FEATURES_AARCH64_H */
> --
> 2.17.1
The 05/07/2020 18:07, Adhemerval Zanella wrote:
> On 30/04/2020 14:44, Szabolcs Nagy wrote:
> > From 45c6bce5a691ecec9bba52785bd1f3a4cbc76fd4 Mon Sep 17 00:00:00 2001
> > From: Sudakshina Das <sudi.das@arm.com>
> > Date: Tue, 17 Mar 2020 15:54:12 +0000
> > Subject: [PATCH 09/12] aarch64: support BTI enabled binaries
> >
> > Binaries can opt-in to using BTI via an ELF property marking.
> > The dynamic linker has to then mprotect the executable segments
> > with PROT_BTI. In case of static linked executables or in case
> > of the dynamic linker itself, PROT_BTI protection is done by the
> > operating system.
> >
> > On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
> > the properties of a binary because PT_NOTE can be unreliable with
> > old linkers.
> >
> > Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> > ---
> > elf/dl-load.c | 2 +
> > elf/rtld.c | 2 +
> > sysdeps/aarch64/Makefile | 4 +
> > sysdeps/aarch64/dl-bti.c | 54 ++++++
> > sysdeps/aarch64/dl-prop.h | 170 ++++++++++++++++++
> > sysdeps/aarch64/linkmap.h | 1 +
> > sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
> > sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 ++++
> > .../unix/sysv/linux/aarch64/cpu-features.c | 3 +
> > .../unix/sysv/linux/aarch64/cpu-features.h | 1 +
> > 10 files changed, 269 insertions(+)
> > create mode 100644 sysdeps/aarch64/dl-bti.c
> > create mode 100644 sysdeps/aarch64/dl-prop.h
> > create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> >
> > diff --git a/elf/dl-load.c b/elf/dl-load.c
> > index a6b80f9395..0930250619 100644
> > --- a/elf/dl-load.c
> > +++ b/elf/dl-load.c
> > @@ -1145,6 +1145,8 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> > l->l_relro_size = ph->p_memsz;
> > break;
> >
> > + case PT_GNU_PROPERTY:
> > + /* Fall through. PT_GNU_PROPERTY holds property notes. */
> > case PT_NOTE:
> > if (_dl_process_pt_note (l, ph, fd, fbp))
> > {
>
> This will print the same error message for a failure in _dl_process_pt_note
> ("cannot process note segment"). Wouldn't be better to use a more specific
> error message, like "cannot process GNU property segment"?
i was struggling to deal with this, i'm happy
to create a new hook for pt_gnu_property
(that's actually cleaner in aarch64, but x86
will have to continue to look at PT_NOTE for
the same).
it requires more generic changes though and
related code repetitions.
> > + do
> > + {
> > + unsigned int type = *(unsigned int *) ptr;
> > + unsigned int datasz = *(unsigned int *) (ptr + 4);
> > +
> > + /* Property type must be in ascending order. */
> > + if (type < last_type)
> > + return;
> > +
> > + ptr += 8;
> > + if ((ptr + datasz) > ptr_end)
> > + return;
> > +
> > + last_type = type;
>
> The logic to parse the PT_GNU_PROPERTY is quite similar to the one
> at sysdeps/x86/dl-prop.h to parse PT_NOTE. Would it be possible to
> maybe try to consolidate the logic somewhere to avoid this code
> duplication?
yes it's similar but not the same.
x86 tries to deal with multiple property notes
which does not happen on aarch64.
i can try to refactor the code and see if
that works.
> > + if (ph->p_offset + size <= (size_t) fbp->len)
> > + note = (const void *) (fbp->buf + ph->p_offset);
> > + else
> > + {
> > + if (size < __MAX_ALLOCA_CUTOFF)
> > + note = alloca (size);
> > + else
> > + note = note_malloced = malloc (size);
> > + if (note == NULL)
> > + return -1;
> > + if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
> > + {
> > + if (note_malloced)
> > + free (note_malloced);
> > + return -1;
>
> I wonder if we should use a scratch_buffer here instead.
this logic is copied from x86, on aarch64 local buffer
should work with current linkers (since they will only
add at most one gnu property note to PT_GNU_PROPERTY),
but we don't know what happens in the future so the
malloc fallback is probably required.
i think ideally the segment is mmaped into memory and
we can just use that, but i assumed the logic is there
for a reason on x86.
> > +++ b/sysdeps/aarch64/linkmap.h
> > @@ -20,4 +20,5 @@ struct link_map_machine
> > {
> > ElfW(Addr) plt; /* Address of .plt */
> > void *tlsdesc_table; /* Address of TLS descriptor hash table. */
> > + int bti_guarded; /* Branch Target Identification mechanism enabled. */
>
> Maybe bool here?
ok.
> > +++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> > @@ -72,3 +72,4 @@
> > #define HWCAP2_BF16 (1 << 14)
> > #define HWCAP2_DGH (1 << 15)
> > #define HWCAP2_RNG (1 << 16)
> > +#define HWCAP2_BTI (1 << 17)
>
> This it not yet upstream on Linus tree (6e7f2eacf098), but follows
> the arm64/for-next/bti branch (8ef8f360cf30be12).
yes, now renamed to bti-user (because the kernel
code can also use bti protection) it is scheduled
for next linux, this patchset depends on that work,
but they have to be tested together.
> > +/* AArch64 specific definitions, should be in sync with
> > + arch/arm64/include/uapi/asm/mman.h. */
> > +
> > +#define PROT_BTI 0x10
>
> Linux specific flags should be protected by __USE_MISC.
in posix the PROT_ prefix is reserved for sys/mman.h
so there is no namespace issue with this.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
see also PROT_SAO on powerpc.
> > +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> > @@ -64,6 +64,7 @@ struct cpu_features
> > {
> > uint64_t midr_el1;
> > unsigned zva_size;
> > + int bti;
>
> Maybe bool here?
ok.
On 11/05/2020 08:04, Szabolcs Nagy wrote:
> The 05/07/2020 18:07, Adhemerval Zanella wrote:
>> On 30/04/2020 14:44, Szabolcs Nagy wrote:
>>> From 45c6bce5a691ecec9bba52785bd1f3a4cbc76fd4 Mon Sep 17 00:00:00 2001
>>> From: Sudakshina Das <sudi.das@arm.com>
>>> Date: Tue, 17 Mar 2020 15:54:12 +0000
>>> Subject: [PATCH 09/12] aarch64: support BTI enabled binaries
>>>
>>> Binaries can opt-in to using BTI via an ELF property marking.
>>> The dynamic linker has to then mprotect the executable segments
>>> with PROT_BTI. In case of static linked executables or in case
>>> of the dynamic linker itself, PROT_BTI protection is done by the
>>> operating system.
>>>
>>> On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
>>> the properties of a binary because PT_NOTE can be unreliable with
>>> old linkers.
>>>
>>> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>> ---
>>> elf/dl-load.c | 2 +
>>> elf/rtld.c | 2 +
>>> sysdeps/aarch64/Makefile | 4 +
>>> sysdeps/aarch64/dl-bti.c | 54 ++++++
>>> sysdeps/aarch64/dl-prop.h | 170 ++++++++++++++++++
>>> sysdeps/aarch64/linkmap.h | 1 +
>>> sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
>>> sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 ++++
>>> .../unix/sysv/linux/aarch64/cpu-features.c | 3 +
>>> .../unix/sysv/linux/aarch64/cpu-features.h | 1 +
>>> 10 files changed, 269 insertions(+)
>>> create mode 100644 sysdeps/aarch64/dl-bti.c
>>> create mode 100644 sysdeps/aarch64/dl-prop.h
>>> create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>>>
>>> diff --git a/elf/dl-load.c b/elf/dl-load.c
>>> index a6b80f9395..0930250619 100644
>>> --- a/elf/dl-load.c
>>> +++ b/elf/dl-load.c
>>> @@ -1145,6 +1145,8 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>>> l->l_relro_size = ph->p_memsz;
>>> break;
>>>
>>> + case PT_GNU_PROPERTY:
>>> + /* Fall through. PT_GNU_PROPERTY holds property notes. */
>>> case PT_NOTE:
>>> if (_dl_process_pt_note (l, ph, fd, fbp))
>>> {
>>
>> This will print the same error message for a failure in _dl_process_pt_note
>> ("cannot process note segment"). Wouldn't be better to use a more specific
>> error message, like "cannot process GNU property segment"?
>
> i was struggling to deal with this, i'm happy
> to create a new hook for pt_gnu_property
> (that's actually cleaner in aarch64, but x86
> will have to continue to look at PT_NOTE for
> the same).
>
> it requires more generic changes though and
> related code repetitions.
I think it might be worth, specially since the _dl_process_pt_note
stills used the __MAX_ALLOCA_CUTOFF (which I think we should replace
with a scratch_buffer).
>
>>> + do
>>> + {
>>> + unsigned int type = *(unsigned int *) ptr;
>>> + unsigned int datasz = *(unsigned int *) (ptr + 4);
>>> +
>>> + /* Property type must be in ascending order. */
>>> + if (type < last_type)
>>> + return;
>>> +
>>> + ptr += 8;
>>> + if ((ptr + datasz) > ptr_end)
>>> + return;
>>> +
>>> + last_type = type;
>>
>> The logic to parse the PT_GNU_PROPERTY is quite similar to the one
>> at sysdeps/x86/dl-prop.h to parse PT_NOTE. Would it be possible to
>> maybe try to consolidate the logic somewhere to avoid this code
>> duplication?
>
> yes it's similar but not the same.
>
> x86 tries to deal with multiple property notes
> which does not happen on aarch64.
>
> i can try to refactor the code and see if
> that works.
Maybe parametrize the logic of number of property notes?
>
>>> + if (ph->p_offset + size <= (size_t) fbp->len)
>>> + note = (const void *) (fbp->buf + ph->p_offset);
>>> + else
>>> + {
>>> + if (size < __MAX_ALLOCA_CUTOFF)
>>> + note = alloca (size);
>>> + else
>>> + note = note_malloced = malloc (size);
>>> + if (note == NULL)
>>> + return -1;
>>> + if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
>>> + {
>>> + if (note_malloced)
>>> + free (note_malloced);
>>> + return -1;
>>
>> I wonder if we should use a scratch_buffer here instead.
>
> this logic is copied from x86, on aarch64 local buffer
> should work with current linkers (since they will only
> add at most one gnu property note to PT_GNU_PROPERTY),
> but we don't know what happens in the future so the
> malloc fallback is probably required.
>
> i think ideally the segment is mmaped into memory and
> we can just use that, but i assumed the logic is there
> for a reason on x86.
Would mmap/unmap the segment a better strategy than use a small stack
allocation for most of cases with a malloc fallback (which I think
won't be used unless a ill formatted note)?
>
>>> +++ b/sysdeps/aarch64/linkmap.h
>>> @@ -20,4 +20,5 @@ struct link_map_machine
>>> {
>>> ElfW(Addr) plt; /* Address of .plt */
>>> void *tlsdesc_table; /* Address of TLS descriptor hash table. */
>>> + int bti_guarded; /* Branch Target Identification mechanism enabled. */
>>
>> Maybe bool here?
>
> ok.
>
>>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
>>> @@ -72,3 +72,4 @@
>>> #define HWCAP2_BF16 (1 << 14)
>>> #define HWCAP2_DGH (1 << 15)
>>> #define HWCAP2_RNG (1 << 16)
>>> +#define HWCAP2_BTI (1 << 17)
>>
>> This it not yet upstream on Linus tree (6e7f2eacf098), but follows
>> the arm64/for-next/bti branch (8ef8f360cf30be12).
>
> yes, now renamed to bti-user (because the kernel
> code can also use bti protection) it is scheduled
> for next linux, this patchset depends on that work,
> but they have to be tested together.
Ack, we just need to avoid another BZ#25971.
>
>>> +/* AArch64 specific definitions, should be in sync with
>>> + arch/arm64/include/uapi/asm/mman.h. */
>>> +
>>> +#define PROT_BTI 0x10
>>
>> Linux specific flags should be protected by __USE_MISC.
>
> in posix the PROT_ prefix is reserved for sys/mman.h
> so there is no namespace issue with this.
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
>
> see also PROT_SAO on powerpc.
>
MMAP_ as well, but we still protect Linux ones with __USE_MISC.
The conform/data/sys/mman.h-data does not trigger a namespace issue
for neither MMAP_ nor PROT_, so I am not sure which would be the
best policy here.
>>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>>> @@ -64,6 +64,7 @@ struct cpu_features
>>> {
>>> uint64_t midr_el1;
>>> unsigned zva_size;
>>> + int bti;
>>
>> Maybe bool here?
>
> ok.
>
From 45c6bce5a691ecec9bba52785bd1f3a4cbc76fd4 Mon Sep 17 00:00:00 2001
From: Sudakshina Das <sudi.das@arm.com>
Date: Tue, 17 Mar 2020 15:54:12 +0000
Subject: [PATCH 09/12] aarch64: support BTI enabled binaries
Binaries can opt-in to using BTI via an ELF property marking.
The dynamic linker has to then mprotect the executable segments
with PROT_BTI. In case of static linked executables or in case
of the dynamic linker itself, PROT_BTI protection is done by the
operating system.
On AArch64 glibc uses PT_GNU_PROPERTY instead of PT_NOTE to check
the properties of a binary because PT_NOTE can be unreliable with
old linkers.
Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
elf/dl-load.c | 2 +
elf/rtld.c | 2 +
sysdeps/aarch64/Makefile | 4 +
sysdeps/aarch64/dl-bti.c | 54 ++++++
sysdeps/aarch64/dl-prop.h | 170 ++++++++++++++++++
sysdeps/aarch64/linkmap.h | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/mman.h | 31 ++++
.../unix/sysv/linux/aarch64/cpu-features.c | 3 +
.../unix/sysv/linux/aarch64/cpu-features.h | 1 +
10 files changed, 269 insertions(+)
create mode 100644 sysdeps/aarch64/dl-bti.c
create mode 100644 sysdeps/aarch64/dl-prop.h
create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/mman.h
@@ -1145,6 +1145,8 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
l->l_relro_size = ph->p_memsz;
break;
+ case PT_GNU_PROPERTY:
+ /* Fall through. PT_GNU_PROPERTY holds property notes. */
case PT_NOTE:
if (_dl_process_pt_note (l, ph, fd, fbp))
{
@@ -1505,6 +1505,8 @@ of this helper program; chances are you did not intend to run this program.\n\
main_map->l_relro_size = ph->p_memsz;
break;
+ case PT_GNU_PROPERTY:
+ /* Fall through. PT_GNU_PROPERTY holds property notes. */
case PT_NOTE:
if (_rtld_process_pt_note (main_map, ph))
_dl_error_printf ("\
@@ -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
new file mode 100644
@@ -0,0 +1,54 @@
+/* AArch64 BTI initializers function.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <unistd.h>
+#include <errno.h>
+#include <libintl.h>
+#include <ldsodefs.h>
+
+static int
+enable_bti (struct link_map *map, const char *program)
+{
+ const ElfW(Phdr) *phdr;
+ unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
+
+ for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
+ if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
+ {
+ ElfW(Addr) start = phdr->p_vaddr + map->l_addr;
+ ElfW(Addr) len = phdr->p_memsz;
+ if (__mprotect ((void *)start, len, prot) < 0)
+ {
+ if (program)
+ _dl_fatal_printf ("%s: mprotect failed to turn on BTI\n",
+ map->l_name);
+ else
+ _dl_signal_error (EINVAL, map->l_name, "dlopen",
+ N_("mprotect failed to turn on BTI"));
+ }
+ }
+ return 0;
+}
+
+/* Enable BTI for L if required. */
+
+void
+_dl_bti_check (struct link_map *l, const char *program)
+{
+ if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti_guarded)
+ enable_bti (l, program);
+}
new file mode 100644
@@ -0,0 +1,170 @@
+/* Support for GNU properties. AArch64 version.
+ Copyright (C) 2018-2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _DL_PROP_H
+#define _DL_PROP_H
+
+#include <not-cancel.h>
+
+extern void _dl_bti_check (struct link_map *, const char *)
+ attribute_hidden;
+
+static inline void __attribute__ ((always_inline))
+_rtld_main_check (struct link_map *m, const char *program)
+{
+ _dl_bti_check (m, program);
+}
+
+static inline void __attribute__ ((always_inline))
+_dl_open_check (struct link_map *m)
+{
+ _dl_bti_check (m, 0);
+}
+
+static inline void __attribute__ ((unused))
+_dl_process_aarch64_property (struct link_map *l,
+ const ElfW(Nhdr) *note,
+ const ElfW(Addr) size,
+ const ElfW(Addr) align)
+{
+ /* The NT_GNU_PROPERTY_TYPE_0 note must be aliged to 4 bytes in
+ 32-bit objects and to 8 bytes in 64-bit objects. Skip notes
+ with incorrect alignment. */
+ if (align != (__ELF_NATIVE_CLASS / 8))
+ return;
+
+ const ElfW(Addr) start = (ElfW(Addr)) note;
+
+ unsigned int feature_1 = 0;
+ unsigned int last_type = 0;
+
+ while ((ElfW(Addr)) (note + 1) - start < size)
+ {
+ /* Find the NT_GNU_PROPERTY_TYPE_0 note. */
+ if (note->n_namesz == 4
+ && note->n_type == NT_GNU_PROPERTY_TYPE_0
+ && memcmp (note + 1, "GNU", 4) == 0)
+ {
+ /* Check for invalid property. */
+ if (note->n_descsz < 8
+ || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
+ return;
+
+ /* Start and end of property array. */
+ unsigned char *ptr = (unsigned char *) (note + 1) + 4;
+ unsigned char *ptr_end = ptr + note->n_descsz;
+
+ do
+ {
+ unsigned int type = *(unsigned int *) ptr;
+ unsigned int datasz = *(unsigned int *) (ptr + 4);
+
+ /* Property type must be in ascending order. */
+ if (type < last_type)
+ return;
+
+ ptr += 8;
+ if ((ptr + datasz) > ptr_end)
+ return;
+
+ last_type = type;
+
+ if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
+ {
+ /* The size of GNU_PROPERTY_AARCH64_FEATURE_1_AND is 4
+ bytes. When seeing GNU_PROPERTY_AARCH64_FEATURE_1_AND,
+ we stop the search regardless if its size is correct
+ or not. There is no point to continue if this note
+ is ill-formed. */
+ if (datasz != 4)
+ return;
+
+ feature_1 = *(unsigned int *) ptr;
+ if ((feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI))
+ l->l_mach.bti_guarded = true;
+
+ /* Stop if we found the property note. */
+ return;
+ }
+ else if (type > GNU_PROPERTY_AARCH64_FEATURE_1_AND)
+ {
+ /* Stop since property type is in ascending order. */
+ return;
+ }
+
+ /* Check the next property item. */
+ ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
+ }
+ while ((ptr_end - ptr) >= 8);
+ }
+
+ /* NB: Note sections like .note.ABI-tag and .note.gnu.build-id are
+ aligned to 4 bytes in 64-bit ELF objects. */
+ note = ((const void *) note
+ + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
+ align));
+ }
+}
+
+#ifdef FILEBUF_SIZE
+static inline int __attribute__ ((unused))
+_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph,
+ int fd, struct filebuf *fbp)
+{
+ if (ph->p_type != PT_GNU_PROPERTY)
+ return 0;
+
+ const ElfW(Nhdr) *note;
+ ElfW(Nhdr) *note_malloced = NULL;
+ ElfW(Addr) size = ph->p_filesz;
+
+ if (ph->p_offset + size <= (size_t) fbp->len)
+ note = (const void *) (fbp->buf + ph->p_offset);
+ else
+ {
+ if (size < __MAX_ALLOCA_CUTOFF)
+ note = alloca (size);
+ else
+ note = note_malloced = malloc (size);
+ if (note == NULL)
+ return -1;
+ if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size)
+ {
+ if (note_malloced)
+ free (note_malloced);
+ return -1;
+ }
+ }
+ _dl_process_aarch64_property (l, note, ph->p_filesz, ph->p_align);
+ if (note_malloced)
+ free (note_malloced);
+ return 0;
+}
+#endif
+
+static inline int __attribute__ ((unused))
+_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
+{
+ if (ph->p_type != PT_GNU_PROPERTY)
+ return 0;
+ const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
+ _dl_process_aarch64_property (l, note, ph->p_memsz, ph->p_align);
+ return 0;
+}
+
+#endif /* _DL_PROP_H */
@@ -20,4 +20,5 @@ struct link_map_machine
{
ElfW(Addr) plt; /* Address of .plt */
void *tlsdesc_table; /* Address of TLS descriptor hash table. */
+ int bti_guarded; /* Branch Target Identification mechanism enabled. */
};
@@ -72,3 +72,4 @@
#define HWCAP2_BF16 (1 << 14)
#define HWCAP2_DGH (1 << 15)
#define HWCAP2_RNG (1 << 16)
+#define HWCAP2_BTI (1 << 17)
new file mode 100644
@@ -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>
@@ -83,4 +83,7 @@ init_cpu_features (struct cpu_features *cpu_features)
if ((dczid & DCZID_DZP_MASK) == 0)
cpu_features->zva_size = 4 << (dczid & DCZID_BS_MASK);
+
+ /* Check if BTI is enabled. */
+ cpu_features->bti = (GLRO (dl_hwcap2) & HWCAP2_BTI);
}
@@ -64,6 +64,7 @@ struct cpu_features
{
uint64_t midr_el1;
unsigned zva_size;
+ int bti;
};
#endif /* _CPU_FEATURES_AARCH64_H */
--
2.17.1