diff mbox series

[v5,07/14] rtld: Handle PT_GNU_PROPERTY

Message ID 2fd3fdf54dcc9cf2c40808bf6b9b95a69038f929.1592834304.git.szabolcs.nagy@arm.com
State Superseded
Headers
Series aarch64: branch protection support |

Commit Message

Szabolcs Nagy June 22, 2020, 2 p.m. UTC 
  Add generic code to handle PT_GNU_PROPERTY notes. Unlike
_dl_process_pt_note, _dl_process_pt_gnu_property is generic,
has no failure mode (invalid content is ignored) and always
called after PT_LOAD segments are mapped. Currently only one
NT_GNU_PROPERTY_TYPE_0 note is handled, which contains target
specific properties: the _dl_process_gnu_property target hook
is called for each property.

Otherwise it follows the existing x86 note processing logic.
---
 elf/dl-load.c              | 81 ++++++++++++++++++++++++++++++++++++++
 elf/rtld.c                 |  4 ++
 sysdeps/generic/dl-prop.h  | 19 ++++++---
 sysdeps/generic/ldsodefs.h |  4 ++
 sysdeps/x86/dl-prop.h      |  7 ++++
 5 files changed, 110 insertions(+), 5 deletions(-)

Comments

H.J. Lu June 22, 2020, 8:41 p.m. UTC | #1 
On Mon, Jun 22, 2020 at 7:04 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> Add generic code to handle PT_GNU_PROPERTY notes. Unlike
> _dl_process_pt_note, _dl_process_pt_gnu_property is generic,
> has no failure mode (invalid content is ignored) and always
> called after PT_LOAD segments are mapped. Currently only one
> NT_GNU_PROPERTY_TYPE_0 note is handled, which contains target
> specific properties: the _dl_process_gnu_property target hook
> is called for each property.
>
> Otherwise it follows the existing x86 note processing logic.
> ---
>  elf/dl-load.c              | 81 ++++++++++++++++++++++++++++++++++++++
>  elf/rtld.c                 |  4 ++
>  sysdeps/generic/dl-prop.h  | 19 ++++++---
>  sysdeps/generic/ldsodefs.h |  4 ++
>  sysdeps/x86/dl-prop.h      |  7 ++++
>  5 files changed, 110 insertions(+), 5 deletions(-)
>
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 06f2ba7264..66bd0ca0a3 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -853,6 +853,77 @@ lose (int code, int fd, const char *name, char *realname, struct link_map *l,
>  }
>
>
> +/* Process PT_GNU_PROPERTY program header PH in module L after
> +   PT_LOAD segments are mapped.  Only one NT_GNU_PROPERTY_TYPE_0
> +   note is handled which contains processor specific properties.  */
> +
> +void
> +_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
> +{
> +  const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
> +  const ElfW(Addr) size = ph->p_memsz;
> +  const ElfW(Addr) align = ph->p_align;
> +
> +  /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
> +     32-bit objects and to 8 bytes in 64-bit objects.  Skip notes
> +     with incorrect alignment.  */
> +  if (align != (__ELF_NATIVE_CLASS / 8))
> +    return;
> +
> +  const ElfW(Addr) start = (ElfW(Addr)) note;
> +  unsigned int last_type = 0;
> +
> +  while ((ElfW(Addr)) (note + 1) - start < size)
> +    {
> +      /* Find the NT_GNU_PROPERTY_TYPE_0 note.  */
> +      if (note->n_namesz == 4
> +         && note->n_type == NT_GNU_PROPERTY_TYPE_0
> +         && memcmp (note + 1, "GNU", 4) == 0)
> +       {
> +         /* Check for invalid property.  */
> +         if (note->n_descsz < 8
> +             || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
> +           return;
> +
> +         /* Start and end of property array.  */
> +         unsigned char *ptr = (unsigned char *) (note + 1) + 4;
> +         unsigned char *ptr_end = ptr + note->n_descsz;
> +
> +         do
> +           {
> +             unsigned int type = *(unsigned int *) ptr;
> +             unsigned int datasz = *(unsigned int *) (ptr + 4);
> +
> +             /* Property type must be in ascending order.  */
> +             if (type < last_type)
> +               return;
> +
> +             ptr += 8;
> +             if ((ptr + datasz) > ptr_end)
> +               return;
> +
> +             last_type = type;
> +
> +             /* Target specific property processing.  */
> +             if (_dl_process_gnu_property(l, type, datasz, ptr) == 0)
> +               return;
> +
> +             /* Check the next property item.  */
> +             ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> +           }
> +         while ((ptr_end - ptr) >= 8);
> +
> +         /* Only handle one NT_GNU_PROPERTY_TYPE_0.  */
> +         return;
> +       }
> +
> +      note = ((const void *) note
> +             + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
> +                                     align));
> +    }
> +}
> +
> +
>  /* Map in the shared object NAME, actually located in REALNAME, and already
>     opened on FD.  */
>
> @@ -1188,6 +1259,16 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>                                   maplength, has_holes, loader);
>      if (__glibc_unlikely (errstring != NULL))
>        goto call_lose;
> +
> +    /* Process program headers again after load segments are mapped in
> +       case processing requires accessing those segments.  */
> +    for (ph = phdr; ph < &phdr[l->l_phnum]; ++ph)
> +      switch (ph->p_type)
> +       {
> +       case PT_GNU_PROPERTY:
> +         _dl_process_pt_gnu_property (l, ph);
> +         break;
> +       }
>    }
>

This patch is immediately changed by the next patch.  You should
combine them into
one patch and add

Co-Authored-By: H.J. Lu <hjl.tools@gmail.com>

Thanks.
Szabolcs Nagy June 25, 2020, 2:58 p.m. UTC | #2 
The 06/22/2020 13:41, H.J. Lu wrote:
> On Mon, Jun 22, 2020 at 7:04 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > Add generic code to handle PT_GNU_PROPERTY notes. Unlike
> > _dl_process_pt_note, _dl_process_pt_gnu_property is generic,
> > has no failure mode (invalid content is ignored) and always
> > called after PT_LOAD segments are mapped. Currently only one
> > NT_GNU_PROPERTY_TYPE_0 note is handled, which contains target
> > specific properties: the _dl_process_gnu_property target hook
> > is called for each property.
> 
> This patch is immediately changed by the next patch.  You should
> combine them into
> one patch and add
> 
> Co-Authored-By: H.J. Lu <hjl.tools@gmail.com>

in general i don't think a patch should be merged with a
previous patch just because it modifies something in it.

but in this case i'm fine either way so attached the
merged patch. (the content is unchanged only the
commit message updated to reflect the merge)

is this OK?
H.J. Lu June 25, 2020, 3:25 p.m. UTC | #3 
On Thu, Jun 25, 2020 at 7:58 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 06/22/2020 13:41, H.J. Lu wrote:
> > On Mon, Jun 22, 2020 at 7:04 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > Add generic code to handle PT_GNU_PROPERTY notes. Unlike
> > > _dl_process_pt_note, _dl_process_pt_gnu_property is generic,
> > > has no failure mode (invalid content is ignored) and always
> > > called after PT_LOAD segments are mapped. Currently only one
> > > NT_GNU_PROPERTY_TYPE_0 note is handled, which contains target
> > > specific properties: the _dl_process_gnu_property target hook
> > > is called for each property.
> >
> > This patch is immediately changed by the next patch.  You should
> > combine them into
> > one patch and add
> >
> > Co-Authored-By: H.J. Lu <hjl.tools@gmail.com>
>
> in general i don't think a patch should be merged with a
> previous patch just because it modifies something in it.
>
> but in this case i'm fine either way so attached the
> merged patch. (the content is unchanged only the
> commit message updated to reflect the merge)
>
> is this OK?

LGTM.  I am working on a follow-up patch to implement
_dl_process_gnu_property.

Thanks.
Szabolcs Nagy June 25, 2020, 3:43 p.m. UTC | #4 
The 06/25/2020 08:25, H.J. Lu wrote:
> On Thu, Jun 25, 2020 at 7:58 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > The 06/22/2020 13:41, H.J. Lu wrote:
> > > On Mon, Jun 22, 2020 at 7:04 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > Add generic code to handle PT_GNU_PROPERTY notes. Unlike
> > > > _dl_process_pt_note, _dl_process_pt_gnu_property is generic,
> > > > has no failure mode (invalid content is ignored) and always
> > > > called after PT_LOAD segments are mapped. Currently only one
> > > > NT_GNU_PROPERTY_TYPE_0 note is handled, which contains target
> > > > specific properties: the _dl_process_gnu_property target hook
> > > > is called for each property.
> > >
> > > This patch is immediately changed by the next patch.  You should
> > > combine them into
> > > one patch and add
> > >
> > > Co-Authored-By: H.J. Lu <hjl.tools@gmail.com>
> >
> > in general i don't think a patch should be merged with a
> > previous patch just because it modifies something in it.
> >
> > but in this case i'm fine either way so attached the
> > merged patch. (the content is unchanged only the
> > commit message updated to reflect the merge)
> >
> > is this OK?
> 
> LGTM.  I am working on a follow-up patch to implement
> _dl_process_gnu_property.

OK, i will wait a bit in case somebody has a comment
e.g. about the backward iteration over phdr.

if there are no comments i'll commit this tomorrow.
H.J. Lu June 25, 2020, 7:14 p.m. UTC | #5 
On Thu, Jun 25, 2020 at 8:43 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 06/25/2020 08:25, H.J. Lu wrote:
> > On Thu, Jun 25, 2020 at 7:58 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > The 06/22/2020 13:41, H.J. Lu wrote:
> > > > On Mon, Jun 22, 2020 at 7:04 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > Add generic code to handle PT_GNU_PROPERTY notes. Unlike
> > > > > _dl_process_pt_note, _dl_process_pt_gnu_property is generic,
> > > > > has no failure mode (invalid content is ignored) and always
> > > > > called after PT_LOAD segments are mapped. Currently only one
> > > > > NT_GNU_PROPERTY_TYPE_0 note is handled, which contains target
> > > > > specific properties: the _dl_process_gnu_property target hook
> > > > > is called for each property.
> > > >
> > > > This patch is immediately changed by the next patch.  You should
> > > > combine them into
> > > > one patch and add
> > > >
> > > > Co-Authored-By: H.J. Lu <hjl.tools@gmail.com>
> > >
> > > in general i don't think a patch should be merged with a
> > > previous patch just because it modifies something in it.
> > >
> > > but in this case i'm fine either way so attached the
> > > merged patch. (the content is unchanged only the
> > > commit message updated to reflect the merge)
> > >
> > > is this OK?
> >
> > LGTM.  I am working on a follow-up patch to implement
> > _dl_process_gnu_property.
>
> OK, i will wait a bit in case somebody has a comment
> e.g. about the backward iteration over phdr.
>
> if there are no comments i'll commit this tomorrow.

Here is a patch to implement _dl_process_gnu_property for x86.
I added _dl_process_pt_gnu_property_start to set the l_cet field
to lc_none before scanning GNU property to indicate that CET
status is no longer unknown.
diff mbox series

Patch

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 06f2ba7264..66bd0ca0a3 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -853,6 +853,77 @@  lose (int code, int fd, const char *name, char *realname, struct link_map *l,
 }
 
 
+/* Process PT_GNU_PROPERTY program header PH in module L after
+   PT_LOAD segments are mapped.  Only one NT_GNU_PROPERTY_TYPE_0
+   note is handled which contains processor specific properties.  */
+
+void
+_dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
+{
+  const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
+  const ElfW(Addr) size = ph->p_memsz;
+  const ElfW(Addr) align = ph->p_align;
+
+  /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
+     32-bit objects and to 8 bytes in 64-bit objects.  Skip notes
+     with incorrect alignment.  */
+  if (align != (__ELF_NATIVE_CLASS / 8))
+    return;
+
+  const ElfW(Addr) start = (ElfW(Addr)) note;
+  unsigned int last_type = 0;
+
+  while ((ElfW(Addr)) (note + 1) - start < size)
+    {
+      /* Find the NT_GNU_PROPERTY_TYPE_0 note.  */
+      if (note->n_namesz == 4
+	  && note->n_type == NT_GNU_PROPERTY_TYPE_0
+	  && memcmp (note + 1, "GNU", 4) == 0)
+	{
+	  /* Check for invalid property.  */
+	  if (note->n_descsz < 8
+	      || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
+	    return;
+
+	  /* Start and end of property array.  */
+	  unsigned char *ptr = (unsigned char *) (note + 1) + 4;
+	  unsigned char *ptr_end = ptr + note->n_descsz;
+
+	  do
+	    {
+	      unsigned int type = *(unsigned int *) ptr;
+	      unsigned int datasz = *(unsigned int *) (ptr + 4);
+
+	      /* Property type must be in ascending order.  */
+	      if (type < last_type)
+		return;
+
+	      ptr += 8;
+	      if ((ptr + datasz) > ptr_end)
+		return;
+
+	      last_type = type;
+
+	      /* Target specific property processing.  */
+	      if (_dl_process_gnu_property(l, type, datasz, ptr) == 0)
+		return;
+
+	      /* Check the next property item.  */
+	      ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
+	    }
+	  while ((ptr_end - ptr) >= 8);
+
+	  /* Only handle one NT_GNU_PROPERTY_TYPE_0.  */
+	  return;
+	}
+
+      note = ((const void *) note
+	      + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
+				      align));
+    }
+}
+
+
 /* Map in the shared object NAME, actually located in REALNAME, and already
    opened on FD.  */
 
@@ -1188,6 +1259,16 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 				  maplength, has_holes, loader);
     if (__glibc_unlikely (errstring != NULL))
       goto call_lose;
+
+    /* Process program headers again after load segments are mapped in
+       case processing requires accessing those segments.  */
+    for (ph = phdr; ph < &phdr[l->l_phnum]; ++ph)
+      switch (ph->p_type)
+	{
+	case PT_GNU_PROPERTY:
+	  _dl_process_pt_gnu_property (l, ph);
+	  break;
+	}
   }
 
   if (l->l_ld == 0)
diff --git a/elf/rtld.c b/elf/rtld.c
index 882b070cc0..3ad2bf5079 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1508,6 +1508,10 @@  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:
+	_dl_process_pt_gnu_property (main_map, ph);
+	break;
+
       case PT_NOTE:
 	if (_rtld_process_pt_note (main_map, ph))
 	  _dl_error_printf ("\
diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
index 6b0f2aa95a..ceb6f623ee 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,13 @@  _rtld_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,
+   processing of the properties continues until this returns 0.  */
+static inline int __attribute__ ((always_inline))
+_dl_process_gnu_property (struct link_map *l, uint32_t type, uint32_t datasz,
+			  void *data)
+{
+  return 0;
+}
+
 #endif /* _DL_PROP_H */
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index d08b97a5ef..c525ffa12c 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -910,6 +910,10 @@  extern void _dl_setup_hash (struct link_map *map) attribute_hidden;
 extern void _dl_rtld_di_serinfo (struct link_map *loader,
 				 Dl_serinfo *si, bool counting);
 
+/* Process PT_GNU_PROPERTY program header PH in module L after
+   PT_LOAD segments are mapped.  */
+void _dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph);
+
 
 /* Search loaded objects' symbol tables for a definition of the symbol
    referred to by UNDEF.  *SYM is the symbol table entry containing the
diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index 516f88ea80..4a8ebc573e 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -191,4 +191,11 @@  _rtld_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph)
   return 0;
 }
 
+static inline int __attribute__ ((always_inline))
+_dl_process_gnu_property (struct link_map *l, uint32_t type, uint32_t datasz,
+			  void *data)
+{
+  return 0;
+}
+
 #endif /* _DL_PROP_H */