| Message ID | 7e800caa0485fec67b21ebab1e27bb23b2f0d971.1606898457.git.szabolcs.nagy@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] aarch64: align address for BTI protection [BZ #26988] | expand |
On 02/12/2020 05:55, Szabolcs Nagy via Libc-alpha wrote: > Re-mmap executable segments if possible instead of using mprotect > to add PROT_BTI. This allows using BTI protection with security > policies that prevent mprotect with PROT_EXEC. > > If the fd of the ELF module is not available because it was kernel > mapped then mprotect is used and failures are ignored. To protect > the main executable even when mprotect is filtered the linux kernel > will have to be changed to add PROT_BTI to it. > > The delayed failure reporting is mainly needed because currently > _dl_process_gnu_properties does not propagate failures such that > the required cleanups happen. Using the link_map_machine struct for > error propagation is not ideal, but this seemed to be the least > intrusive solution. > > Fixes bug 26831. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > v3: > - split the last patch in two. > - pushed to nsz/btifix-v3 branch. > > sysdeps/aarch64/dl-bti.c | 54 ++++++++++++++++++++++++++------------- > sysdeps/aarch64/dl-prop.h | 8 +++++- > sysdeps/aarch64/linkmap.h | 2 +- > 3 files changed, 44 insertions(+), 20 deletions(-) > > diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c > index 67d63c8a73..ff26c98ccf 100644 > --- a/sysdeps/aarch64/dl-bti.c > +++ b/sysdeps/aarch64/dl-bti.c > @@ -19,9 +19,17 @@ > #include <errno.h> > #include <libintl.h> > #include <ldsodefs.h> > +#include <sys/mman.h> > > -static void > -enable_bti (struct link_map *map, const char *program) > +/* See elf/dl-load.h. */ > +#ifndef MAP_COPY > +# define MAP_COPY (MAP_PRIVATE | MAP_DENYWRITE) > +#endif > + > +/* Enable BTI protection for MAP. */ > + > +void > +_dl_bti_protect (struct link_map *map, int fd) > { > const size_t pagesz = GLRO(dl_pagesize); > const ElfW(Phdr) *phdr; > @@ -41,19 +49,31 @@ enable_bti (struct link_map *map, const char *program) > if (phdr->p_flags & PF_W) > prot |= PROT_WRITE; > > - 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")); > - } > + if (fd == -1) > + /* Ignore failures for kernel mapped binaries. */ > + __mprotect (start, len, prot); > + else > + map->l_mach.bti_fail = __mmap (start, len, prot, > + MAP_FIXED|MAP_COPY|MAP_FILE, > + fd, off) == MAP_FAILED; > } > } > OK. I am not very found of ignore the mprotect failure here, but it has been discussed in multiple threads that we need kernel support to proper handle it. > -/* Enable BTI for MAP and its dependencies. */ > + > +static void > +bti_failed (struct link_map *l, const char *program) > +{ > + if (program) No implicit checks. > + _dl_fatal_printf ("%s: %s: failed to turn on BTI protection\n", > + program, l->l_name); > + else > + /* Note: the errno value is not available any more. */ > + _dl_signal_error (0, l->l_name, "dlopen", > + N_("failed to turn on BTI protection")); > +} > + > + > +/* Report BTI protection failures for MAP and its dependencies. */ > Ok. > void > _dl_bti_check (struct link_map *map, const char *program) > @@ -61,16 +81,14 @@ _dl_bti_check (struct link_map *map, const char *program) > if (!GLRO(dl_aarch64_cpu_features).bti) > return; > > - if (map->l_mach.bti) > - enable_bti (map, program); > + if (map->l_mach.bti_fail) > + bti_failed (map, program); > > unsigned int i = map->l_searchlist.r_nlist; > while (i-- > 0) > { > struct link_map *l = map->l_initfini[i]; > - if (l->l_init_called) > - continue; > - if (l->l_mach.bti) > - enable_bti (l, program); > + if (l->l_mach.bti_fail) > + bti_failed (l, program); > } > } Ok. > diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h > index 2016d1472e..e926e54984 100644 > --- a/sysdeps/aarch64/dl-prop.h > +++ b/sysdeps/aarch64/dl-prop.h > @@ -19,6 +19,8 @@ > #ifndef _DL_PROP_H > #define _DL_PROP_H > > +extern void _dl_bti_protect (struct link_map *, int) attribute_hidden; > + > extern void _dl_bti_check (struct link_map *, const char *) > attribute_hidden; > > @@ -43,6 +45,10 @@ static inline int > _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type, > uint32_t datasz, void *data) > { > + if (!GLRO(dl_aarch64_cpu_features).bti) > + /* Skip note processing. */ > + return 0; > + > if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND) > { > /* Stop if the property note is ill-formed. */ > @@ -51,7 +57,7 @@ _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type, > > unsigned int feature_1 = *(unsigned int *) data; > if (feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI) > - l->l_mach.bti = true; > + _dl_bti_protect (l, fd); > > /* Stop if we processed the property note. */ > return 0; Ok. > diff --git a/sysdeps/aarch64/linkmap.h b/sysdeps/aarch64/linkmap.h > index 847a03ace2..b3f7663b07 100644 > --- a/sysdeps/aarch64/linkmap.h > +++ b/sysdeps/aarch64/linkmap.h > @@ -22,5 +22,5 @@ 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. */ > + bool bti_fail; /* Failed to enable Branch Target Identification. */ > }; > Ok.
diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c index 67d63c8a73..ff26c98ccf 100644 --- a/sysdeps/aarch64/dl-bti.c +++ b/sysdeps/aarch64/dl-bti.c @@ -19,9 +19,17 @@ #include <errno.h> #include <libintl.h> #include <ldsodefs.h> +#include <sys/mman.h> -static void -enable_bti (struct link_map *map, const char *program) +/* See elf/dl-load.h. */ +#ifndef MAP_COPY +# define MAP_COPY (MAP_PRIVATE | MAP_DENYWRITE) +#endif + +/* Enable BTI protection for MAP. */ + +void +_dl_bti_protect (struct link_map *map, int fd) { const size_t pagesz = GLRO(dl_pagesize); const ElfW(Phdr) *phdr; @@ -41,19 +49,31 @@ enable_bti (struct link_map *map, const char *program) if (phdr->p_flags & PF_W) prot |= PROT_WRITE; - 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")); - } + if (fd == -1) + /* Ignore failures for kernel mapped binaries. */ + __mprotect (start, len, prot); + else + map->l_mach.bti_fail = __mmap (start, len, prot, + MAP_FIXED|MAP_COPY|MAP_FILE, + fd, off) == MAP_FAILED; } } -/* Enable BTI for MAP and its dependencies. */ + +static void +bti_failed (struct link_map *l, const char *program) +{ + if (program) + _dl_fatal_printf ("%s: %s: failed to turn on BTI protection\n", + program, l->l_name); + else + /* Note: the errno value is not available any more. */ + _dl_signal_error (0, l->l_name, "dlopen", + N_("failed to turn on BTI protection")); +} + + +/* Report BTI protection failures for MAP and its dependencies. */ void _dl_bti_check (struct link_map *map, const char *program) @@ -61,16 +81,14 @@ _dl_bti_check (struct link_map *map, const char *program) if (!GLRO(dl_aarch64_cpu_features).bti) return; - if (map->l_mach.bti) - enable_bti (map, program); + if (map->l_mach.bti_fail) + bti_failed (map, program); unsigned int i = map->l_searchlist.r_nlist; while (i-- > 0) { struct link_map *l = map->l_initfini[i]; - if (l->l_init_called) - continue; - if (l->l_mach.bti) - enable_bti (l, program); + if (l->l_mach.bti_fail) + bti_failed (l, program); } } diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h index 2016d1472e..e926e54984 100644 --- a/sysdeps/aarch64/dl-prop.h +++ b/sysdeps/aarch64/dl-prop.h @@ -19,6 +19,8 @@ #ifndef _DL_PROP_H #define _DL_PROP_H +extern void _dl_bti_protect (struct link_map *, int) attribute_hidden; + extern void _dl_bti_check (struct link_map *, const char *) attribute_hidden; @@ -43,6 +45,10 @@ static inline int _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type, uint32_t datasz, void *data) { + if (!GLRO(dl_aarch64_cpu_features).bti) + /* Skip note processing. */ + return 0; + if (type == GNU_PROPERTY_AARCH64_FEATURE_1_AND) { /* Stop if the property note is ill-formed. */ @@ -51,7 +57,7 @@ _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type, unsigned int feature_1 = *(unsigned int *) data; if (feature_1 & GNU_PROPERTY_AARCH64_FEATURE_1_BTI) - l->l_mach.bti = true; + _dl_bti_protect (l, fd); /* Stop if we processed the property note. */ return 0; diff --git a/sysdeps/aarch64/linkmap.h b/sysdeps/aarch64/linkmap.h index 847a03ace2..b3f7663b07 100644 --- a/sysdeps/aarch64/linkmap.h +++ b/sysdeps/aarch64/linkmap.h @@ -22,5 +22,5 @@ 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. */ + bool bti_fail; /* Failed to enable Branch Target Identification. */ };