[PATCH v5 07/12] aarch64: enable BTI at runtime
Szabolcs Nagy
szabolcs.nagy@arm.com
Fri Jun 19 12:31:58 GMT 2020
The 06/18/2020 13:16, H.J. Lu wrote:
> On Thu, Jun 18, 2020 at 8:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 06/12/2020 08:18, H.J. Lu wrote:
> > > On Fri, Jun 12, 2020 at 8:08 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > The 06/12/2020 06:43, H.J. Lu wrote:
> > > > > Can you make _dl_process_pt_gnu_property more generic so that it
> > > > > can be shared with x86? If there is PT_GNU_PROPERTY, we don't
> > > > > need to check for multiple property notes. Thanks.
> > > >
> > > > it can use a hook for NT_GNU_PROPERTY_TYPE_0 notes
> > > > that is called for each property, e.g.
> > > >
> > > > int
> > > > _dl_process_gnu_property_note0(uint32_t type, uint32_t datasz, const void *data);
> >
> > i also had to pass the link_map.
> >
> > > > however this only helps on x86 if PT_GNU_PROPERTY
> > > > processing is before PT_NOTE processing otherwise
> > > > you cannot skip handling the notes.
> > >
> > > On x86-64, there are
> > >
> > > Program Headers:
> > > Type Offset VirtAddr PhysAddr
> > > FileSiz MemSiz Flags Align
> > ...
> > > NOTE 0x0000000000000338 0x0000000000000338 0x0000000000000338
> > > 0x0000000000000020 0x0000000000000020 R 0x8
> > > NOTE 0x0000000000000358 0x0000000000000358 0x0000000000000358
> > > 0x0000000000000044 0x0000000000000044 R 0x4
> > > GNU_PROPERTY 0x0000000000000338 0x0000000000000338 0x0000000000000338
> > > 0x0000000000000020 0x0000000000000020 R 0x8
> > ...
> > > Only the PT_NOTE segment with the correct alignment may contain property.
> > > In the first pass, we record such PT_NOTE segment, but don't process it.
> > > After all segments have been processed, we process the saved PT_NOTE segment
> > > if there is no GNU_PROPERTY segment.
> >
> > i refactored the code but didn't change the x86 side.
> >
> > note: in rtld there is only one pass (load segments
> > are already mapped) while dlopen does the property
> > handling in the second pass over program headers.
> >
> > attaching v5 of the patch:
> >
> > - moved _dl_process_pt_gnu_property to generic code
> > (into dl-load.c), this assumes PT_GNU_PROPERTY
> > is a gnu platform abi and the integer types used
> > for accessing it work on all targets.
> >
> > - made _dl_process_pt_gnu_property return type void
> > and remove all failure handling (invalid notes
> > are ignored).
> >
> > - only handle one NT_GNU_PROPERTY_TYPE_0.
> >
> > - added _dl_process_gnu_property target hook.
> >
> > - removed reviewed-by because of significant changes.
> >
> > tested on aarch64.
> >
>
> Can you incorporate this patch into yours?
>
> 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. Since PT_GNU_PROPERTY is processed
> after PT_LOAD segments are mapped, we can process PT_NOTE together with
> PT_GNU_PROPERTY. We can remove the old _dl_process_pt_note and rename
> _rtld_process_pt_note to _dl_process_pt_note.
>
> NOTE: We scan program headers backward so that PT_NOTE can be skipped
> if PT_GNU_PROPERTY exits.
this makes sense to me and it can be a follow up
patch on top of my PT_GNU_PROPERTY patch, it
does not have to be integrated. (it's not ideal
to intergate it with the bti enablement, so i
would have to split that patch to do the property
things and bti things separately)
i think you didn't change the order of the phdr
processing, i'd expect a backward iteration in both
rtld.c and dl-load.c for that to work.
>
> Thanks.
>
> --
> H.J.
> From 53fb2346572a456f1a07a3a14c6b0859b556fab8 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 18 Jun 2020 10:28:39 -0700
> Subject: [PATCH] Rename _rtld_process_pt_note to _dl_process_pt_note
>
> 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. Since PT_GNU_PROPERTY is processed
> after PT_LOAD segments are mapped, we can process PT_NOTE together with
> PT_GNU_PROPERTY. We can remove the old _dl_process_pt_note and rename
> _rtld_process_pt_note to _dl_process_pt_note.
>
> NOTE: We scan program headers backward so that PT_NOTE can be skipped
> if PT_GNU_PROPERTY exits.
> ---
> elf/dl-load.c | 15 ++++++-------
> elf/rtld.c | 4 +---
> sysdeps/aarch64/dl-prop.h | 16 ++------------
> sysdeps/generic/dl-prop.h | 14 ++-----------
> sysdeps/x86/dl-prop.h | 44 ++-------------------------------------
> 5 files changed, 13 insertions(+), 80 deletions(-)
>
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 66bd0ca0a3..b04dff559c 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1216,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))
> @@ -1261,10 +1253,15 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> goto call_lose;
>
> /* Process program headers again after load segments are mapped in
> - case processing requires accessing those segments. */
> + 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; ph < &phdr[l->l_phnum]; ++ph)
> switch (ph->p_type)
> {
> + case PT_NOTE:
> + _dl_process_pt_note (l, ph);
> + break;
> case PT_GNU_PROPERTY:
> _dl_process_pt_gnu_property (l, ph);
> break;
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 3ad2bf5079..ea68fea3e6 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1513,9 +1513,7 @@ of this helper program; chances are you did not intend to run this program.\n\
> break;
>
> 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);
> break;
> }
>
> diff --git a/sysdeps/aarch64/dl-prop.h b/sysdeps/aarch64/dl-prop.h
> index 70edc52145..b0785bda83 100644
> --- a/sysdeps/aarch64/dl-prop.h
> +++ b/sysdeps/aarch64/dl-prop.h
> @@ -19,8 +19,6 @@
> #ifndef _DL_PROP_H
> #define _DL_PROP_H
>
> -#include <not-cancel.h>
> -
> extern void _dl_bti_check (struct link_map *, const char *)
> attribute_hidden;
>
> @@ -36,19 +34,9 @@ _dl_open_check (struct link_map *m)
> _dl_bti_check (m, NULL);
> }
>
> -#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)
> +static inline void __attribute__ ((always_inline))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> {
> - return 0;
> }
>
> static inline int
> diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
> index ceb6f623ee..f1cf576fe3 100644
> --- a/sysdeps/generic/dl-prop.h
> +++ b/sysdeps/generic/dl-prop.h
> @@ -36,19 +36,9 @@ _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)
> -{
> - return 0;
> -}
> -#endif
> -
> -static inline int __attribute__ ((always_inline))
> -_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> +static inline void __attribute__ ((always_inline))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> {
> - return 0;
> }
>
> /* Called for each property in the NT_GNU_PROPERTY_TYPE_0 note of L,
> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index 4a8ebc573e..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,49 +144,11 @@ _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)
> -{
> -# 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;
> -}
> -#endif
> -
> -static inline int __attribute__ ((unused))
> -_rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> +static inline void __attribute__ ((unused))
> +_dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
> {
> 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;
> }
>
> static inline int __attribute__ ((always_inline))
> --
> 2.26.2
>
--
More information about the Libc-alpha
mailing list