elf: Add elf checks for main executable

Message ID 20211119150329.2200675-1-adhemerval.zanella@linaro.org
State Superseded
Delegated to: H.J. Lu
Headers
Series elf: Add elf checks for main executable |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Adhemerval Zanella Netto Nov. 19, 2021, 3:03 p.m. UTC
  The ELF header integrity check is only done on open_verify(), i.e,
for objects explicitly loaded.  For main executable (issued with
execve() for the binary) only kernel checks are done, which does
not check EI_ABIVERSION.

To enable it, the loader needs to find where the ELF header is placed
at program start, however Linux auxiliary vectors only provides
the program header table (AT_EHDR).  To avoid require upstream
kernel support, the ELF header is implicitly obtained from the PT_LOAD
values by checking for a segment with offset 0 and memory size
different than 0 (For Linux it is start the ELF file issued by
execve()).

Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
---
 elf/Makefile                           |   8 +-
 elf/dl-check-err.h                     |  14 ++
 elf/dl-check.c                         | 151 ++++++++++++++++++
 elf/dl-check.h                         |  45 ++++++
 elf/dl-load.c                          | 114 ++------------
 elf/rtld.c                             |   8 +
 elf/tst-elf-check.c                    | 209 +++++++++++++++++++++++++
 sysdeps/generic/dl-elf-check.h         |  28 ++++
 sysdeps/unix/sysv/linux/dl-elf-check.h |  32 ++++
 9 files changed, 507 insertions(+), 102 deletions(-)
 create mode 100644 elf/dl-check-err.h
 create mode 100644 elf/dl-check.c
 create mode 100644 elf/dl-check.h
 create mode 100644 elf/tst-elf-check.c
 create mode 100644 sysdeps/generic/dl-elf-check.h
 create mode 100644 sysdeps/unix/sysv/linux/dl-elf-check.h
  

Comments

H.J. Lu Nov. 19, 2021, 3:33 p.m. UTC | #1
On Fri, Nov 19, 2021 at 7:03 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> The ELF header integrity check is only done on open_verify(), i.e,
> for objects explicitly loaded.  For main executable (issued with
> execve() for the binary) only kernel checks are done, which does
> not check EI_ABIVERSION.
>
> To enable it, the loader needs to find where the ELF header is placed
> at program start, however Linux auxiliary vectors only provides

Is it possible to check __ehdr_start?

> the program header table (AT_EHDR).  To avoid require upstream
> kernel support, the ELF header is implicitly obtained from the PT_LOAD
> values by checking for a segment with offset 0 and memory size
> different than 0 (For Linux it is start the ELF file issued by
> execve()).
>
> Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
> ---
>  elf/Makefile                           |   8 +-
>  elf/dl-check-err.h                     |  14 ++
>  elf/dl-check.c                         | 151 ++++++++++++++++++
>  elf/dl-check.h                         |  45 ++++++
>  elf/dl-load.c                          | 114 ++------------
>  elf/rtld.c                             |   8 +
>  elf/tst-elf-check.c                    | 209 +++++++++++++++++++++++++
>  sysdeps/generic/dl-elf-check.h         |  28 ++++
>  sysdeps/unix/sysv/linux/dl-elf-check.h |  32 ++++
>  9 files changed, 507 insertions(+), 102 deletions(-)
>  create mode 100644 elf/dl-check-err.h
>  create mode 100644 elf/dl-check.c
>  create mode 100644 elf/dl-check.h
>  create mode 100644 elf/tst-elf-check.c
>  create mode 100644 sysdeps/generic/dl-elf-check.h
>  create mode 100644 sysdeps/unix/sysv/linux/dl-elf-check.h
>
> diff --git a/elf/Makefile b/elf/Makefile
> index 4723c159cb..f09fc5c6ec 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -36,7 +36,8 @@ dl-routines   = $(addprefix dl-,load lookup object reloc deps \
>                                   exception sort-maps lookup-direct \
>                                   call-libc-early-init write \
>                                   thread_gscope_wait tls_init_tp \
> -                                 debug-symbols minimal-malloc)
> +                                 debug-symbols minimal-malloc \
> +                                 check)
>  ifeq (yes,$(use-ldconfig))
>  dl-routines += dl-cache
>  endif
> @@ -238,7 +239,8 @@ tests-internal += loadtest unload unload2 circleload1 \
>          tst-ptrguard1 tst-stackguard1 \
>          tst-create_format1 tst-tls-surplus tst-dl-hwcaps_split
>  tests-container += tst-pldd tst-dlopen-tlsmodid-container \
> -  tst-dlopen-self-container tst-preload-pthread-libc
> +  tst-dlopen-self-container tst-preload-pthread-libc \
> +  tst-elf-check
>  test-srcs = tst-pathopt
>  selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
>  ifneq ($(selinux-enabled),1)
> @@ -494,6 +496,8 @@ tests-special += $(objpfx)order-cmp.out $(objpfx)tst-array1-cmp.out \
>                  $(objpfx)tst-unused-dep-cmp.out
>  endif
>
> +tst-elf-check-ARGS = -- $(host-test-program-cmd)
> +
>  ifndef avoid-generated
>  # DSO sorting tests:
>  # The dso-ordering-test.py script generates testcase source files in $(objpfx),
> diff --git a/elf/dl-check-err.h b/elf/dl-check-err.h
> new file mode 100644
> index 0000000000..6ca5246eb8
> --- /dev/null
> +++ b/elf/dl-check-err.h
> @@ -0,0 +1,14 @@
> +_S(DL_ELFHDR_OK, "")
> +_S(DL_ELFHDR_ERR_ELFMAG,     N_("invalid ELF header"))
> +_S(DL_ELFHDR_ERR_CLASS32,    N_("wrong ELF class: ELFCLASS32"))
> +_S(DL_ELFHDR_ERR_CLASS64,    N_("wrong ELF class: ELFCLASS64"))
> +_S(DL_ELFHDR_ERR_BENDIAN,    N_("ELF file data encoding not big-endian"))
> +_S(DL_ELFHDR_ERR_LENDIAN,    N_("ELF file data encoding not little-endian"))
> +_S(DL_ELFHDR_ERR_EIVERSION,  N_("ELF file version ident does not match current one"))
> +_S(DL_ELFHDR_ERR_OSABI,      N_("ELF file OS ABI invalid"))
> +_S(DL_ELFHDR_ERR_ABIVERSION, N_("ELF file ABI version invalid"))
> +_S(DL_ELFHDR_ERR_PAD,        N_("nonzero padding in e_ident"))
> +_S(DL_ELFHDR_ERR_VERSION,    N_("ELF file version does not match current one"))
> +_S(DL_ELFHDR_ERR_TYPE,       N_("only ET_DYN and ET_EXEC can be loaded"))
> +_S(DL_ELFHDR_ERR_PHENTSIZE,  N_("ELF file's phentsize not the expected size"))
> +_S(DL_ELFHDR_ERR_INTERNAL,   N_("internal error"))
> diff --git a/elf/dl-check.c b/elf/dl-check.c
> new file mode 100644
> index 0000000000..ef1720df2a
> --- /dev/null
> +++ b/elf/dl-check.c
> @@ -0,0 +1,151 @@
> +/* ELF header consistency and ABI checks.
> +   Copyright (C) 1995-2021 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/>.  */
> +
> +#include <array_length.h>
> +#include <dl-check.h>
> +#include <endian.h>
> +#include <ldsodefs.h>
> +#include <libintl.h>
> +
> +int
> +_dl_elfhdr_check (const ElfW(Ehdr) *ehdr)
> +{
> +#define ELF32_CLASS ELFCLASS32
> +#define ELF64_CLASS ELFCLASS64
> +#if BYTE_ORDER == BIG_ENDIAN
> +# define byteorder ELFDATA2MSB
> +#elif BYTE_ORDER == LITTLE_ENDIAN
> +# define byteorder ELFDATA2LSB
> +#else
> +# error "Unknown BYTE_ORDER " BYTE_ORDER
> +# define byteorder ELFDATANONE
> +#endif
> +  MORE_ELF_HEADER_DATA;
> +  static const unsigned char expected[EI_NIDENT] =
> +  {
> +    [EI_MAG0] = ELFMAG0,
> +    [EI_MAG1] = ELFMAG1,
> +    [EI_MAG2] = ELFMAG2,
> +    [EI_MAG3] = ELFMAG3,
> +    [EI_CLASS] = ELFW(CLASS),
> +    [EI_DATA] = byteorder,
> +    [EI_VERSION] = EV_CURRENT,
> +    [EI_OSABI] = ELFOSABI_SYSV,
> +    [EI_ABIVERSION] = 0
> +  };
> +
> +  /* See whether the ELF header is what we expect.  */
> +  if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
> +                                           EI_ABIVERSION)
> +                       || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> +                                                 ehdr->e_ident[EI_ABIVERSION])
> +                       || memcmp (&ehdr->e_ident[EI_PAD],
> +                                  &expected[EI_PAD],
> +                                  EI_NIDENT - EI_PAD) != 0))
> +    {
> +      /* Something is wrong.  */
> +      const Elf32_Word *magp = (const void *) ehdr->e_ident;
> +      if (*magp !=
> +#if BYTE_ORDER == LITTLE_ENDIAN
> +         ((ELFMAG0 << (EI_MAG0 * 8))
> +          | (ELFMAG1 << (EI_MAG1 * 8))
> +          | (ELFMAG2 << (EI_MAG2 * 8))
> +          | (ELFMAG3 << (EI_MAG3 * 8)))
> +#else
> +         ((ELFMAG0 << (EI_MAG3 * 8))
> +          | (ELFMAG1 << (EI_MAG2 * 8))
> +          | (ELFMAG2 << (EI_MAG1 * 8))
> +          | (ELFMAG3 << (EI_MAG0 * 8)))
> +#endif
> +         )
> +       return DL_ELFHDR_ERR_ELFMAG;
> +      else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
> +       return ELFW(CLASS) == ELFCLASS32
> +              ? DL_ELFHDR_ERR_CLASS64
> +              : DL_ELFHDR_ERR_CLASS32;
> +      else if (ehdr->e_ident[EI_DATA] != byteorder)
> +       {
> +         if (BYTE_ORDER == BIG_ENDIAN)
> +           return DL_ELFHDR_ERR_BENDIAN;
> +         else
> +           return DL_ELFHDR_ERR_LENDIAN;
> +       }
> +      else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
> +       return DL_ELFHDR_ERR_EIVERSION;
> +      /* XXX We should be able so set system specific versions which are
> +        allowed here.  */
> +      else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
> +       return DL_ELFHDR_ERR_OSABI;
> +      else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> +                                     ehdr->e_ident[EI_ABIVERSION]))
> +       return DL_ELFHDR_ERR_ABIVERSION;
> +      else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
> +                      EI_NIDENT - EI_PAD) != 0)
> +       return DL_ELFHDR_ERR_PAD;
> +      else
> +       return DL_ELFHDR_ERR_INTERNAL;
> +    }
> +
> +  if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
> +    return DL_ELFHDR_ERR_VERSION;
> +  else if (__glibc_unlikely (ehdr->e_type != ET_DYN
> +                            && ehdr->e_type != ET_EXEC))
> +    return DL_ELFHDR_ERR_TYPE;
> +  else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
> +    return DL_ELFHDR_ERR_PHENTSIZE;
> +
> +  return DL_ELFHDR_OK;
> +}
> +
> +static const union elfhdr_errstr_t
> +{
> +  struct
> +  {
> +#define _S(n, s) char str##n[sizeof (s)];
> +#include "dl-check-err.h"
> +#undef _S
> +  };
> +  char str[0];
> +} elfhdr_errstr =
> +{
> +  {
> +#define _S(n, s) s,
> +#include "dl-check-err.h"
> +#undef _S
> +  }
> +};
> +
> +static const unsigned short elfhder_erridx[] =
> +{
> +#define _S(n, s) [n] = offsetof(union elfhdr_errstr_t, str##n),
> +#include "dl-check-err.h"
> +#undef _S
> +};
> +
> +const char *
> +_dl_elfhdr_errstr (int err)
> +{
> +#if 0
> +  if (err >= 0 && err < array_length (elfhdr_errstr))
> +    return elfhdr_errstr[err];
> +  return NULL;
> +#endif
> +  if (err < 0 || err >= array_length (elfhder_erridx))
> +    err = 0;
> +  return elfhdr_errstr.str + elfhder_erridx[err];
> +}
> diff --git a/elf/dl-check.h b/elf/dl-check.h
> new file mode 100644
> index 0000000000..5104a353ed
> --- /dev/null
> +++ b/elf/dl-check.h
> @@ -0,0 +1,45 @@
> +/* ELF header consistency and ABI checks.
> +   Copyright (C) 1995-2021 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_OPENCHECK_H
> +#define _DL_OPENCHECK_H
> +
> +#include <link.h>
> +
> +enum
> + {
> +   DL_ELFHDR_OK,
> +   DL_ELFHDR_ERR_ELFMAG,     /* Invalid ELFMAGX value.  */
> +   DL_ELFHDR_ERR_CLASS32,    /* Mismatched EI_CLASS.  */
> +   DL_ELFHDR_ERR_CLASS64,    /* Mismatched EI_CLASS.  */
> +   DL_ELFHDR_ERR_BENDIAN,    /* Mismatched EI_DATA (not big-endian).  */
> +   DL_ELFHDR_ERR_LENDIAN,    /* Mismatched EI_DATA (not little-endian).  */
> +   DL_ELFHDR_ERR_EIVERSION,  /* Invalid EI_VERSION.  */
> +   DL_ELFHDR_ERR_OSABI,      /* Invalid EI_OSABI.  */
> +   DL_ELFHDR_ERR_ABIVERSION, /* Invalid ABI vrsion.  */
> +   DL_ELFHDR_ERR_PAD,        /* Invalid EI_PAD value.  */
> +   DL_ELFHDR_ERR_VERSION,    /* Invalid e_version.  */
> +   DL_ELFHDR_ERR_TYPE,       /* Invalid e_type.  */
> +   DL_ELFHDR_ERR_PHENTSIZE,  /* Invalid e_phentsize.  */
> +   DL_ELFHDR_ERR_INTERNAL    /* Internal error.  */
> + };
> +
> +int _dl_elfhdr_check (const ElfW(Ehdr) *ehdr) attribute_hidden;
> +const char *_dl_elfhdr_errstr (int err) attribute_hidden;
> +
> +#endif
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index bf8957e73c..45266c3501 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -73,19 +73,9 @@ struct filebuf
>  #include <dl-machine-reject-phdr.h>
>  #include <dl-sysdep-open.h>
>  #include <dl-prop.h>
> +#include <dl-check.h>
>  #include <not-cancel.h>
>
> -#include <endian.h>
> -#if BYTE_ORDER == BIG_ENDIAN
> -# define byteorder ELFDATA2MSB
> -#elif BYTE_ORDER == LITTLE_ENDIAN
> -# define byteorder ELFDATA2LSB
> -#else
> -# error "Unknown BYTE_ORDER " BYTE_ORDER
> -# define byteorder ELFDATANONE
> -#endif
> -
> -#define STRING(x) __STRING (x)
>
>
>  int __stack_prot attribute_hidden attribute_relro
> @@ -1598,25 +1588,6 @@ open_verify (const char *name, int fd,
>    /* This is the expected ELF header.  */
>  #define ELF32_CLASS ELFCLASS32
>  #define ELF64_CLASS ELFCLASS64
> -#ifndef VALID_ELF_HEADER
> -# define VALID_ELF_HEADER(hdr,exp,size)        (memcmp (hdr, exp, size) == 0)
> -# define VALID_ELF_OSABI(osabi)                (osabi == ELFOSABI_SYSV)
> -# define VALID_ELF_ABIVERSION(osabi,ver) (ver == 0)
> -#elif defined MORE_ELF_HEADER_DATA
> -  MORE_ELF_HEADER_DATA;
> -#endif
> -  static const unsigned char expected[EI_NIDENT] =
> -  {
> -    [EI_MAG0] = ELFMAG0,
> -    [EI_MAG1] = ELFMAG1,
> -    [EI_MAG2] = ELFMAG2,
> -    [EI_MAG3] = ELFMAG3,
> -    [EI_CLASS] = ELFW(CLASS),
> -    [EI_DATA] = byteorder,
> -    [EI_VERSION] = EV_CURRENT,
> -    [EI_OSABI] = ELFOSABI_SYSV,
> -    [EI_ABIVERSION] = 0
> -  };
>    static const struct
>    {
>      ElfW(Word) vendorlen;
> @@ -1709,83 +1680,26 @@ open_verify (const char *name, int fd,
>         }
>
>        /* See whether the ELF header is what we expect.  */
> -      if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
> -                                               EI_ABIVERSION)
> -                           || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> -                                                     ehdr->e_ident[EI_ABIVERSION])
> -                           || memcmp (&ehdr->e_ident[EI_PAD],
> -                                      &expected[EI_PAD],
> -                                      EI_NIDENT - EI_PAD) != 0))
> +      int err = _dl_elfhdr_check (ehdr);
> +      switch (err)
>         {
> -         /* Something is wrong.  */
> -         const Elf32_Word *magp = (const void *) ehdr->e_ident;
> -         if (*magp !=
> -#if BYTE_ORDER == LITTLE_ENDIAN
> -             ((ELFMAG0 << (EI_MAG0 * 8))
> -              | (ELFMAG1 << (EI_MAG1 * 8))
> -              | (ELFMAG2 << (EI_MAG2 * 8))
> -              | (ELFMAG3 << (EI_MAG3 * 8)))
> -#else
> -             ((ELFMAG0 << (EI_MAG3 * 8))
> -              | (ELFMAG1 << (EI_MAG2 * 8))
> -              | (ELFMAG2 << (EI_MAG1 * 8))
> -              | (ELFMAG3 << (EI_MAG0 * 8)))
> -#endif
> -             )
> -           errstring = N_("invalid ELF header");
> -         else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
> -           {
> -             /* This is not a fatal error.  On architectures where
> -                32-bit and 64-bit binaries can be run this might
> -                happen.  */
> -             *found_other_class = true;
> -             goto close_and_out;
> -           }
> -         else if (ehdr->e_ident[EI_DATA] != byteorder)
> -           {
> -             if (BYTE_ORDER == BIG_ENDIAN)
> -               errstring = N_("ELF file data encoding not big-endian");
> -             else
> -               errstring = N_("ELF file data encoding not little-endian");
> -           }
> -         else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
> -           errstring
> -             = N_("ELF file version ident does not match current one");
> -         /* XXX We should be able so set system specific versions which are
> -            allowed here.  */
> -         else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
> -           errstring = N_("ELF file OS ABI invalid");
> -         else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> -                                         ehdr->e_ident[EI_ABIVERSION]))
> -           errstring = N_("ELF file ABI version invalid");
> -         else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
> -                          EI_NIDENT - EI_PAD) != 0)
> -           errstring = N_("nonzero padding in e_ident");
> -         else
> -           /* Otherwise we don't know what went wrong.  */
> -           errstring = N_("internal error");
> +       case DL_ELFHDR_OK:
> +         break;
>
> -         goto lose;
> -       }
> +       case DL_ELFHDR_ERR_CLASS32:
> +       case DL_ELFHDR_ERR_CLASS64:
> +         /* This is not a fatal error.  On architectures where 32-bit and
> +            64-bit binaries can be run this might happen.  */
> +         *found_other_class = true;
> +         goto close_and_out;
>
> -      if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
> -       {
> -         errstring = N_("ELF file version does not match current one");
> +       default:
> +         errstring = _dl_elfhdr_errstr (err);
>           goto lose;
>         }
> +
>        if (! __glibc_likely (elf_machine_matches_host (ehdr)))
>         goto close_and_out;
> -      else if (__glibc_unlikely (ehdr->e_type != ET_DYN
> -                                && ehdr->e_type != ET_EXEC))
> -       {
> -         errstring = N_("only ET_DYN and ET_EXEC can be loaded");
> -         goto lose;
> -       }
> -      else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
> -       {
> -         errstring = N_("ELF file's phentsize not the expected size");
> -         goto lose;
> -       }
>
>        maplength = ehdr->e_phnum * sizeof (ElfW(Phdr));
>        if (ehdr->e_phoff + maplength <= (size_t) fbp->len)
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 847141e21d..89b3157f31 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -50,6 +50,7 @@
>  #include <gnu/lib-names.h>
>  #include <dl-tunables.h>
>  #include <get-dynamic-info.h>
> +#include <dl-elf-check.h>
>
>  #include <assert.h>
>
> @@ -1112,6 +1113,7 @@ dl_main (const ElfW(Phdr) *phdr,
>          ElfW(Addr) *user_entry,
>          ElfW(auxv_t) *auxv)
>  {
> +  const ElfW(Ehdr) *ehdr = NULL;
>    const ElfW(Phdr) *ph;
>    struct link_map *main_map;
>    size_t file_size;
> @@ -1518,6 +1520,9 @@ dl_main (const ElfW(Phdr) *phdr,
>           ElfW(Addr) mapstart;
>           ElfW(Addr) allocend;
>
> +         if (ph->p_offset == 0 && ph->p_memsz > 0)
> +           ehdr = (void *) ph->p_vaddr;
> +
>           /* Remember where the main program starts in memory.  */
>           mapstart = (main_map->l_addr
>                       + (ph->p_vaddr & ~(GLRO(dl_pagesize) - 1)));
> @@ -1577,6 +1582,9 @@ dl_main (const ElfW(Phdr) *phdr,
>         break;
>        }
>
> +  if (ehdr != NULL)
> +    _dl_check_ehdr (ehdr);
> +
>    /* Adjust the address of the TLS initialization image in case
>       the executable is actually an ET_DYN object.  */
>    if (main_map->l_tls_initimage != NULL)
> diff --git a/elf/tst-elf-check.c b/elf/tst-elf-check.c
> new file mode 100644
> index 0000000000..175ba6fb5a
> --- /dev/null
> +++ b/elf/tst-elf-check.c
> @@ -0,0 +1,209 @@
> +/* Check ELF header error paths.
> +   Copyright (C) 2021 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/>.  */
> +
> +#include <elf.h>
> +#include <link.h>
> +#include <libc-abis.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <support/capture_subprocess.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <support/temp_file.h>
> +
> +static char *spargv[6];
> +static char *tmpbin;
> +
> +static void
> +do_prepare (int argc, char *argv[])
> +{
> +  int fdin = xopen (argv[0], O_RDONLY | O_LARGEFILE, 0);
> +  struct stat64 st;
> +  xfstat (fdin, &st);
> +  int fdout = create_temp_file ("tst-elf-check-", &tmpbin);
> +  xfchmod (fdout, S_IXUSR | S_IRUSR | S_IWUSR);
> +  TEST_VERIFY_EXIT (fdout >= 0);
> +  xcopy_file_range (fdin, NULL, fdout, NULL, st.st_size, 0);
> +  xclose (fdin);
> +  xclose (fdout);
> +}
> +
> +static void
> +run_test_expect_failure (void (*modify)(ElfW(Ehdr) *), const char *errmsg)
> +{
> +  int fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
> +  ElfW(Ehdr) orig_hdr;
> +  if (read (fd, &orig_hdr, sizeof (orig_hdr)) != sizeof (orig_hdr))
> +    FAIL_EXIT1 ("read (%s): %m\n", tmpbin);
> +  ElfW(Ehdr) hdr = orig_hdr;
> +  modify (&hdr);
> +  if (lseek (fd, 0, SEEK_SET) != 0)
> +    FAIL_EXIT1 ("lseek: %m");
> +  xwrite (fd, &hdr, sizeof (hdr));
> +  xclose (fd);
> +
> +  struct support_capture_subprocess proc =
> +      support_capture_subprogram (spargv[0], spargv);
> +  support_capture_subprocess_check (&proc, "tst-elf-check", 127,
> +                                   sc_allow_stderr);
> +  TEST_VERIFY (strstr (proc.err.buffer, errmsg) != NULL);
> +  support_capture_subprocess_free (&proc);
> +
> +  /* Restore previous header.  */
> +  fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
> +  xwrite (fd, &orig_hdr, sizeof (orig_hdr));
> +  xclose (fd);
> +}
> +
> +static void
> +modify_mag (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_MAG0] = EI_MAG3;
> +  ehdr->e_ident[EI_MAG1] = EI_MAG2;
> +  ehdr->e_ident[EI_MAG2] = EI_MAG1;
> +  ehdr->e_ident[EI_MAG3] = EI_MAG0;
> +}
> +
> +static void
> +modify_class (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_CLASS] = ELFCLASSNONE;
> +}
> +
> +static void
> +modify_endian (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_DATA] = ELFDATANUM;
> +}
> +
> +static void
> +modify_eiversion (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_VERSION] = EV_CURRENT + 1;
> +}
> +
> +static void
> +modify_osabi (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_OSABI] = ELFOSABI_STANDALONE;
> +}
> +
> +static void
> +modify_abiversion (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
> +}
> +
> +static void
> +modify_pad (ElfW(Ehdr) *ehdr)
> +{
> +  memset (&ehdr->e_ident[EI_PAD], 0xff, EI_NIDENT - EI_PAD);
> +}
> +
> +static void
> +modify_version (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_version = EV_NONE;
> +}
> +
> +static void
> +modify_type (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_type = ET_NONE;
> +}
> +
> +static void
> +modify_phentsize (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_phentsize = sizeof (ElfW(Phdr)) + 1;
> +}
> +
> +static void
> +do_test_kernel (void)
> +{
> +  run_test_expect_failure (modify_mag,
> +                          "invalid ELF header");
> +  run_test_expect_failure (modify_type,
> +                          "only ET_DYN and ET_EXEC can be loaded");
> +  run_test_expect_failure (modify_phentsize,
> +                          "ELF file's phentsize not the expected size");
> +  run_test_expect_failure (modify_class,
> +                          "wrong ELF class");
> +}
> +
> +static void
> +do_test_common (void)
> +{
> +  run_test_expect_failure (modify_endian,
> +                          "ELF file data encoding not");
> +  run_test_expect_failure (modify_eiversion,
> +                          "ELF file version ident does not match current one");
> +  run_test_expect_failure (modify_pad,
> +                          "nonzero padding in e_ident");
> +  run_test_expect_failure (modify_osabi,
> +                          "ELF file OS ABI invalid");
> +  run_test_expect_failure (modify_abiversion,
> +                          "ELF file ABI version invalid");
> +  run_test_expect_failure (modify_version,
> +                          "ELF file version does not match current one");
> +}
> +
> +static int
> +do_test (int argc, char *argv[])
> +{
> +  /* We must have one or four parameters:
> +     + argv[0]:   the application name
> +     + argv[1]:   path for ld.so        optional
> +     + argv[2]:   "--library-path"      optional
> +     + argv[3]:   the library path      optional
> +     + argv[4/1]: the application name  */
> +
> +  bool hardpath = argc == 2;
> +
> +  int i;
> +  for (i = 0; i < argc - 2; i++)
> +    spargv[i] = argv[i+1];
> +  spargv[i++] = tmpbin;
> +  spargv[i++] = (char *) "--direct";
> +  spargv[i] = NULL;
> +
> +  /* Some fields are checked by the kernel results in a execve failure, so skip
> +     them for --enable-hardcoded-path-in-tests.  */
> +  if (!hardpath)
> +    do_test_kernel ();
> +  do_test_common ();
> +
> +  /* Also run the tests without issuing the loader.  */
> +  if (hardpath)
> +    return 0;
> +
> +  spargv[0] = tmpbin;
> +  spargv[1] = (char *) "--direct";
> +  spargv[2] = NULL;
> +
> +  do_test_common ();
> +
> +  return 0;
> +}
> +
> +#define PREPARE do_prepare
> +#define TEST_FUNCTION_ARGV do_test
> +#include <support/test-driver.c>
> diff --git a/sysdeps/generic/dl-elf-check.h b/sysdeps/generic/dl-elf-check.h
> new file mode 100644
> index 0000000000..48eb82e9e7
> --- /dev/null
> +++ b/sysdeps/generic/dl-elf-check.h
> @@ -0,0 +1,28 @@
> +/* ELF header consistency and ABI checks.
> +   Copyright (C) 2021 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_ELF_CHECK_H
> +#define _DL_ELF_CHECK_H
> +
> +/* Called from the loader just after the program headers are processed.  */
> +static inline void
> +_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
> +{
> +}
> +
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/dl-elf-check.h b/sysdeps/unix/sysv/linux/dl-elf-check.h
> new file mode 100644
> index 0000000000..9e4925c090
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/dl-elf-check.h
> @@ -0,0 +1,32 @@
> +/* ELF header consistency and ABI checks.
> +   Copyright (C) 2021 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_ELF_CHECK_H
> +#define _DL_ELF_CHECK_H
> +
> +#include <dl-check.h>
> +
> +static inline void
> +_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
> +{
> +  int err = _dl_elfhdr_check (ehdr);
> +  if (err != DL_ELFHDR_OK)
> +    _dl_fatal_printf ("program loading error: %s\n", _dl_elfhdr_errstr (err));
> +}
> +
> +#endif
> --
> 2.32.0
>
  
H.J. Lu Nov. 19, 2021, 4:05 p.m. UTC | #2
On Fri, Nov 19, 2021 at 7:33 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Nov 19, 2021 at 7:03 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> >
> > The ELF header integrity check is only done on open_verify(), i.e,
> > for objects explicitly loaded.  For main executable (issued with
> > execve() for the binary) only kernel checks are done, which does
> > not check EI_ABIVERSION.
> >
> > To enable it, the loader needs to find where the ELF header is placed
> > at program start, however Linux auxiliary vectors only provides
>
> Is it possible to check __ehdr_start?

Probably not.   I added this to binutils master branch:

$ elfedit
...
 --output-abiversion [0-255]
                              Set output ABIVERSION

Please use it for both executable and shared library tests.

> > the program header table (AT_EHDR).  To avoid require upstream
> > kernel support, the ELF header is implicitly obtained from the PT_LOAD
> > values by checking for a segment with offset 0 and memory size
> > different than 0 (For Linux it is start the ELF file issued by
> > execve()).
> >
> > Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
> > ---
> >  elf/Makefile                           |   8 +-
> >  elf/dl-check-err.h                     |  14 ++
> >  elf/dl-check.c                         | 151 ++++++++++++++++++
> >  elf/dl-check.h                         |  45 ++++++
> >  elf/dl-load.c                          | 114 ++------------
> >  elf/rtld.c                             |   8 +
> >  elf/tst-elf-check.c                    | 209 +++++++++++++++++++++++++
> >  sysdeps/generic/dl-elf-check.h         |  28 ++++
> >  sysdeps/unix/sysv/linux/dl-elf-check.h |  32 ++++
> >  9 files changed, 507 insertions(+), 102 deletions(-)
> >  create mode 100644 elf/dl-check-err.h
> >  create mode 100644 elf/dl-check.c
> >  create mode 100644 elf/dl-check.h
> >  create mode 100644 elf/tst-elf-check.c
> >  create mode 100644 sysdeps/generic/dl-elf-check.h
> >  create mode 100644 sysdeps/unix/sysv/linux/dl-elf-check.h
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index 4723c159cb..f09fc5c6ec 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -36,7 +36,8 @@ dl-routines   = $(addprefix dl-,load lookup object reloc deps \
> >                                   exception sort-maps lookup-direct \
> >                                   call-libc-early-init write \
> >                                   thread_gscope_wait tls_init_tp \
> > -                                 debug-symbols minimal-malloc)
> > +                                 debug-symbols minimal-malloc \
> > +                                 check)
> >  ifeq (yes,$(use-ldconfig))
> >  dl-routines += dl-cache
> >  endif
> > @@ -238,7 +239,8 @@ tests-internal += loadtest unload unload2 circleload1 \
> >          tst-ptrguard1 tst-stackguard1 \
> >          tst-create_format1 tst-tls-surplus tst-dl-hwcaps_split
> >  tests-container += tst-pldd tst-dlopen-tlsmodid-container \
> > -  tst-dlopen-self-container tst-preload-pthread-libc
> > +  tst-dlopen-self-container tst-preload-pthread-libc \
> > +  tst-elf-check
> >  test-srcs = tst-pathopt
> >  selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
> >  ifneq ($(selinux-enabled),1)
> > @@ -494,6 +496,8 @@ tests-special += $(objpfx)order-cmp.out $(objpfx)tst-array1-cmp.out \
> >                  $(objpfx)tst-unused-dep-cmp.out
> >  endif
> >
> > +tst-elf-check-ARGS = -- $(host-test-program-cmd)
> > +
> >  ifndef avoid-generated
> >  # DSO sorting tests:
> >  # The dso-ordering-test.py script generates testcase source files in $(objpfx),
> > diff --git a/elf/dl-check-err.h b/elf/dl-check-err.h
> > new file mode 100644
> > index 0000000000..6ca5246eb8
> > --- /dev/null
> > +++ b/elf/dl-check-err.h
> > @@ -0,0 +1,14 @@
> > +_S(DL_ELFHDR_OK, "")
> > +_S(DL_ELFHDR_ERR_ELFMAG,     N_("invalid ELF header"))
> > +_S(DL_ELFHDR_ERR_CLASS32,    N_("wrong ELF class: ELFCLASS32"))
> > +_S(DL_ELFHDR_ERR_CLASS64,    N_("wrong ELF class: ELFCLASS64"))
> > +_S(DL_ELFHDR_ERR_BENDIAN,    N_("ELF file data encoding not big-endian"))
> > +_S(DL_ELFHDR_ERR_LENDIAN,    N_("ELF file data encoding not little-endian"))
> > +_S(DL_ELFHDR_ERR_EIVERSION,  N_("ELF file version ident does not match current one"))
> > +_S(DL_ELFHDR_ERR_OSABI,      N_("ELF file OS ABI invalid"))
> > +_S(DL_ELFHDR_ERR_ABIVERSION, N_("ELF file ABI version invalid"))
> > +_S(DL_ELFHDR_ERR_PAD,        N_("nonzero padding in e_ident"))
> > +_S(DL_ELFHDR_ERR_VERSION,    N_("ELF file version does not match current one"))
> > +_S(DL_ELFHDR_ERR_TYPE,       N_("only ET_DYN and ET_EXEC can be loaded"))
> > +_S(DL_ELFHDR_ERR_PHENTSIZE,  N_("ELF file's phentsize not the expected size"))
> > +_S(DL_ELFHDR_ERR_INTERNAL,   N_("internal error"))
> > diff --git a/elf/dl-check.c b/elf/dl-check.c
> > new file mode 100644
> > index 0000000000..ef1720df2a
> > --- /dev/null
> > +++ b/elf/dl-check.c
> > @@ -0,0 +1,151 @@
> > +/* ELF header consistency and ABI checks.
> > +   Copyright (C) 1995-2021 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/>.  */
> > +
> > +#include <array_length.h>
> > +#include <dl-check.h>
> > +#include <endian.h>
> > +#include <ldsodefs.h>
> > +#include <libintl.h>
> > +
> > +int
> > +_dl_elfhdr_check (const ElfW(Ehdr) *ehdr)
> > +{
> > +#define ELF32_CLASS ELFCLASS32
> > +#define ELF64_CLASS ELFCLASS64
> > +#if BYTE_ORDER == BIG_ENDIAN
> > +# define byteorder ELFDATA2MSB
> > +#elif BYTE_ORDER == LITTLE_ENDIAN
> > +# define byteorder ELFDATA2LSB
> > +#else
> > +# error "Unknown BYTE_ORDER " BYTE_ORDER
> > +# define byteorder ELFDATANONE
> > +#endif
> > +  MORE_ELF_HEADER_DATA;
> > +  static const unsigned char expected[EI_NIDENT] =
> > +  {
> > +    [EI_MAG0] = ELFMAG0,
> > +    [EI_MAG1] = ELFMAG1,
> > +    [EI_MAG2] = ELFMAG2,
> > +    [EI_MAG3] = ELFMAG3,
> > +    [EI_CLASS] = ELFW(CLASS),
> > +    [EI_DATA] = byteorder,
> > +    [EI_VERSION] = EV_CURRENT,
> > +    [EI_OSABI] = ELFOSABI_SYSV,
> > +    [EI_ABIVERSION] = 0
> > +  };
> > +
> > +  /* See whether the ELF header is what we expect.  */
> > +  if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
> > +                                           EI_ABIVERSION)
> > +                       || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> > +                                                 ehdr->e_ident[EI_ABIVERSION])
> > +                       || memcmp (&ehdr->e_ident[EI_PAD],
> > +                                  &expected[EI_PAD],
> > +                                  EI_NIDENT - EI_PAD) != 0))
> > +    {
> > +      /* Something is wrong.  */
> > +      const Elf32_Word *magp = (const void *) ehdr->e_ident;
> > +      if (*magp !=
> > +#if BYTE_ORDER == LITTLE_ENDIAN
> > +         ((ELFMAG0 << (EI_MAG0 * 8))
> > +          | (ELFMAG1 << (EI_MAG1 * 8))
> > +          | (ELFMAG2 << (EI_MAG2 * 8))
> > +          | (ELFMAG3 << (EI_MAG3 * 8)))
> > +#else
> > +         ((ELFMAG0 << (EI_MAG3 * 8))
> > +          | (ELFMAG1 << (EI_MAG2 * 8))
> > +          | (ELFMAG2 << (EI_MAG1 * 8))
> > +          | (ELFMAG3 << (EI_MAG0 * 8)))
> > +#endif
> > +         )
> > +       return DL_ELFHDR_ERR_ELFMAG;
> > +      else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
> > +       return ELFW(CLASS) == ELFCLASS32
> > +              ? DL_ELFHDR_ERR_CLASS64
> > +              : DL_ELFHDR_ERR_CLASS32;
> > +      else if (ehdr->e_ident[EI_DATA] != byteorder)
> > +       {
> > +         if (BYTE_ORDER == BIG_ENDIAN)
> > +           return DL_ELFHDR_ERR_BENDIAN;
> > +         else
> > +           return DL_ELFHDR_ERR_LENDIAN;
> > +       }
> > +      else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
> > +       return DL_ELFHDR_ERR_EIVERSION;
> > +      /* XXX We should be able so set system specific versions which are
> > +        allowed here.  */
> > +      else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
> > +       return DL_ELFHDR_ERR_OSABI;
> > +      else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> > +                                     ehdr->e_ident[EI_ABIVERSION]))
> > +       return DL_ELFHDR_ERR_ABIVERSION;
> > +      else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
> > +                      EI_NIDENT - EI_PAD) != 0)
> > +       return DL_ELFHDR_ERR_PAD;
> > +      else
> > +       return DL_ELFHDR_ERR_INTERNAL;
> > +    }
> > +
> > +  if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
> > +    return DL_ELFHDR_ERR_VERSION;
> > +  else if (__glibc_unlikely (ehdr->e_type != ET_DYN
> > +                            && ehdr->e_type != ET_EXEC))
> > +    return DL_ELFHDR_ERR_TYPE;
> > +  else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
> > +    return DL_ELFHDR_ERR_PHENTSIZE;
> > +
> > +  return DL_ELFHDR_OK;
> > +}
> > +
> > +static const union elfhdr_errstr_t
> > +{
> > +  struct
> > +  {
> > +#define _S(n, s) char str##n[sizeof (s)];
> > +#include "dl-check-err.h"
> > +#undef _S
> > +  };
> > +  char str[0];
> > +} elfhdr_errstr =
> > +{
> > +  {
> > +#define _S(n, s) s,
> > +#include "dl-check-err.h"
> > +#undef _S
> > +  }
> > +};
> > +
> > +static const unsigned short elfhder_erridx[] =
> > +{
> > +#define _S(n, s) [n] = offsetof(union elfhdr_errstr_t, str##n),
> > +#include "dl-check-err.h"
> > +#undef _S
> > +};
> > +
> > +const char *
> > +_dl_elfhdr_errstr (int err)
> > +{
> > +#if 0
> > +  if (err >= 0 && err < array_length (elfhdr_errstr))
> > +    return elfhdr_errstr[err];
> > +  return NULL;
> > +#endif
> > +  if (err < 0 || err >= array_length (elfhder_erridx))
> > +    err = 0;
> > +  return elfhdr_errstr.str + elfhder_erridx[err];
> > +}
> > diff --git a/elf/dl-check.h b/elf/dl-check.h
> > new file mode 100644
> > index 0000000000..5104a353ed
> > --- /dev/null
> > +++ b/elf/dl-check.h
> > @@ -0,0 +1,45 @@
> > +/* ELF header consistency and ABI checks.
> > +   Copyright (C) 1995-2021 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_OPENCHECK_H
> > +#define _DL_OPENCHECK_H
> > +
> > +#include <link.h>
> > +
> > +enum
> > + {
> > +   DL_ELFHDR_OK,
> > +   DL_ELFHDR_ERR_ELFMAG,     /* Invalid ELFMAGX value.  */
> > +   DL_ELFHDR_ERR_CLASS32,    /* Mismatched EI_CLASS.  */
> > +   DL_ELFHDR_ERR_CLASS64,    /* Mismatched EI_CLASS.  */
> > +   DL_ELFHDR_ERR_BENDIAN,    /* Mismatched EI_DATA (not big-endian).  */
> > +   DL_ELFHDR_ERR_LENDIAN,    /* Mismatched EI_DATA (not little-endian).  */
> > +   DL_ELFHDR_ERR_EIVERSION,  /* Invalid EI_VERSION.  */
> > +   DL_ELFHDR_ERR_OSABI,      /* Invalid EI_OSABI.  */
> > +   DL_ELFHDR_ERR_ABIVERSION, /* Invalid ABI vrsion.  */
> > +   DL_ELFHDR_ERR_PAD,        /* Invalid EI_PAD value.  */
> > +   DL_ELFHDR_ERR_VERSION,    /* Invalid e_version.  */
> > +   DL_ELFHDR_ERR_TYPE,       /* Invalid e_type.  */
> > +   DL_ELFHDR_ERR_PHENTSIZE,  /* Invalid e_phentsize.  */
> > +   DL_ELFHDR_ERR_INTERNAL    /* Internal error.  */
> > + };
> > +
> > +int _dl_elfhdr_check (const ElfW(Ehdr) *ehdr) attribute_hidden;
> > +const char *_dl_elfhdr_errstr (int err) attribute_hidden;
> > +
> > +#endif
> > diff --git a/elf/dl-load.c b/elf/dl-load.c
> > index bf8957e73c..45266c3501 100644
> > --- a/elf/dl-load.c
> > +++ b/elf/dl-load.c
> > @@ -73,19 +73,9 @@ struct filebuf
> >  #include <dl-machine-reject-phdr.h>
> >  #include <dl-sysdep-open.h>
> >  #include <dl-prop.h>
> > +#include <dl-check.h>
> >  #include <not-cancel.h>
> >
> > -#include <endian.h>
> > -#if BYTE_ORDER == BIG_ENDIAN
> > -# define byteorder ELFDATA2MSB
> > -#elif BYTE_ORDER == LITTLE_ENDIAN
> > -# define byteorder ELFDATA2LSB
> > -#else
> > -# error "Unknown BYTE_ORDER " BYTE_ORDER
> > -# define byteorder ELFDATANONE
> > -#endif
> > -
> > -#define STRING(x) __STRING (x)
> >
> >
> >  int __stack_prot attribute_hidden attribute_relro
> > @@ -1598,25 +1588,6 @@ open_verify (const char *name, int fd,
> >    /* This is the expected ELF header.  */
> >  #define ELF32_CLASS ELFCLASS32
> >  #define ELF64_CLASS ELFCLASS64
> > -#ifndef VALID_ELF_HEADER
> > -# define VALID_ELF_HEADER(hdr,exp,size)        (memcmp (hdr, exp, size) == 0)
> > -# define VALID_ELF_OSABI(osabi)                (osabi == ELFOSABI_SYSV)
> > -# define VALID_ELF_ABIVERSION(osabi,ver) (ver == 0)
> > -#elif defined MORE_ELF_HEADER_DATA
> > -  MORE_ELF_HEADER_DATA;
> > -#endif
> > -  static const unsigned char expected[EI_NIDENT] =
> > -  {
> > -    [EI_MAG0] = ELFMAG0,
> > -    [EI_MAG1] = ELFMAG1,
> > -    [EI_MAG2] = ELFMAG2,
> > -    [EI_MAG3] = ELFMAG3,
> > -    [EI_CLASS] = ELFW(CLASS),
> > -    [EI_DATA] = byteorder,
> > -    [EI_VERSION] = EV_CURRENT,
> > -    [EI_OSABI] = ELFOSABI_SYSV,
> > -    [EI_ABIVERSION] = 0
> > -  };
> >    static const struct
> >    {
> >      ElfW(Word) vendorlen;
> > @@ -1709,83 +1680,26 @@ open_verify (const char *name, int fd,
> >         }
> >
> >        /* See whether the ELF header is what we expect.  */
> > -      if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
> > -                                               EI_ABIVERSION)
> > -                           || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> > -                                                     ehdr->e_ident[EI_ABIVERSION])
> > -                           || memcmp (&ehdr->e_ident[EI_PAD],
> > -                                      &expected[EI_PAD],
> > -                                      EI_NIDENT - EI_PAD) != 0))
> > +      int err = _dl_elfhdr_check (ehdr);
> > +      switch (err)
> >         {
> > -         /* Something is wrong.  */
> > -         const Elf32_Word *magp = (const void *) ehdr->e_ident;
> > -         if (*magp !=
> > -#if BYTE_ORDER == LITTLE_ENDIAN
> > -             ((ELFMAG0 << (EI_MAG0 * 8))
> > -              | (ELFMAG1 << (EI_MAG1 * 8))
> > -              | (ELFMAG2 << (EI_MAG2 * 8))
> > -              | (ELFMAG3 << (EI_MAG3 * 8)))
> > -#else
> > -             ((ELFMAG0 << (EI_MAG3 * 8))
> > -              | (ELFMAG1 << (EI_MAG2 * 8))
> > -              | (ELFMAG2 << (EI_MAG1 * 8))
> > -              | (ELFMAG3 << (EI_MAG0 * 8)))
> > -#endif
> > -             )
> > -           errstring = N_("invalid ELF header");
> > -         else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
> > -           {
> > -             /* This is not a fatal error.  On architectures where
> > -                32-bit and 64-bit binaries can be run this might
> > -                happen.  */
> > -             *found_other_class = true;
> > -             goto close_and_out;
> > -           }
> > -         else if (ehdr->e_ident[EI_DATA] != byteorder)
> > -           {
> > -             if (BYTE_ORDER == BIG_ENDIAN)
> > -               errstring = N_("ELF file data encoding not big-endian");
> > -             else
> > -               errstring = N_("ELF file data encoding not little-endian");
> > -           }
> > -         else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
> > -           errstring
> > -             = N_("ELF file version ident does not match current one");
> > -         /* XXX We should be able so set system specific versions which are
> > -            allowed here.  */
> > -         else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
> > -           errstring = N_("ELF file OS ABI invalid");
> > -         else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> > -                                         ehdr->e_ident[EI_ABIVERSION]))
> > -           errstring = N_("ELF file ABI version invalid");
> > -         else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
> > -                          EI_NIDENT - EI_PAD) != 0)
> > -           errstring = N_("nonzero padding in e_ident");
> > -         else
> > -           /* Otherwise we don't know what went wrong.  */
> > -           errstring = N_("internal error");
> > +       case DL_ELFHDR_OK:
> > +         break;
> >
> > -         goto lose;
> > -       }
> > +       case DL_ELFHDR_ERR_CLASS32:
> > +       case DL_ELFHDR_ERR_CLASS64:
> > +         /* This is not a fatal error.  On architectures where 32-bit and
> > +            64-bit binaries can be run this might happen.  */
> > +         *found_other_class = true;
> > +         goto close_and_out;
> >
> > -      if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
> > -       {
> > -         errstring = N_("ELF file version does not match current one");
> > +       default:
> > +         errstring = _dl_elfhdr_errstr (err);
> >           goto lose;
> >         }
> > +
> >        if (! __glibc_likely (elf_machine_matches_host (ehdr)))
> >         goto close_and_out;
> > -      else if (__glibc_unlikely (ehdr->e_type != ET_DYN
> > -                                && ehdr->e_type != ET_EXEC))
> > -       {
> > -         errstring = N_("only ET_DYN and ET_EXEC can be loaded");
> > -         goto lose;
> > -       }
> > -      else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
> > -       {
> > -         errstring = N_("ELF file's phentsize not the expected size");
> > -         goto lose;
> > -       }
> >
> >        maplength = ehdr->e_phnum * sizeof (ElfW(Phdr));
> >        if (ehdr->e_phoff + maplength <= (size_t) fbp->len)
> > diff --git a/elf/rtld.c b/elf/rtld.c
> > index 847141e21d..89b3157f31 100644
> > --- a/elf/rtld.c
> > +++ b/elf/rtld.c
> > @@ -50,6 +50,7 @@
> >  #include <gnu/lib-names.h>
> >  #include <dl-tunables.h>
> >  #include <get-dynamic-info.h>
> > +#include <dl-elf-check.h>
> >
> >  #include <assert.h>
> >
> > @@ -1112,6 +1113,7 @@ dl_main (const ElfW(Phdr) *phdr,
> >          ElfW(Addr) *user_entry,
> >          ElfW(auxv_t) *auxv)
> >  {
> > +  const ElfW(Ehdr) *ehdr = NULL;
> >    const ElfW(Phdr) *ph;
> >    struct link_map *main_map;
> >    size_t file_size;
> > @@ -1518,6 +1520,9 @@ dl_main (const ElfW(Phdr) *phdr,
> >           ElfW(Addr) mapstart;
> >           ElfW(Addr) allocend;
> >
> > +         if (ph->p_offset == 0 && ph->p_memsz > 0)
> > +           ehdr = (void *) ph->p_vaddr;
> > +
> >           /* Remember where the main program starts in memory.  */
> >           mapstart = (main_map->l_addr
> >                       + (ph->p_vaddr & ~(GLRO(dl_pagesize) - 1)));
> > @@ -1577,6 +1582,9 @@ dl_main (const ElfW(Phdr) *phdr,
> >         break;
> >        }
> >
> > +  if (ehdr != NULL)
> > +    _dl_check_ehdr (ehdr);
> > +
> >    /* Adjust the address of the TLS initialization image in case
> >       the executable is actually an ET_DYN object.  */
> >    if (main_map->l_tls_initimage != NULL)
> > diff --git a/elf/tst-elf-check.c b/elf/tst-elf-check.c
> > new file mode 100644
> > index 0000000000..175ba6fb5a
> > --- /dev/null
> > +++ b/elf/tst-elf-check.c
> > @@ -0,0 +1,209 @@
> > +/* Check ELF header error paths.
> > +   Copyright (C) 2021 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/>.  */
> > +
> > +#include <elf.h>
> > +#include <link.h>
> > +#include <libc-abis.h>
> > +#include <fcntl.h>
> > +#include <string.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <support/capture_subprocess.h>
> > +#include <support/check.h>
> > +#include <support/support.h>
> > +#include <support/xunistd.h>
> > +#include <support/temp_file.h>
> > +
> > +static char *spargv[6];
> > +static char *tmpbin;
> > +
> > +static void
> > +do_prepare (int argc, char *argv[])
> > +{
> > +  int fdin = xopen (argv[0], O_RDONLY | O_LARGEFILE, 0);
> > +  struct stat64 st;
> > +  xfstat (fdin, &st);
> > +  int fdout = create_temp_file ("tst-elf-check-", &tmpbin);
> > +  xfchmod (fdout, S_IXUSR | S_IRUSR | S_IWUSR);
> > +  TEST_VERIFY_EXIT (fdout >= 0);
> > +  xcopy_file_range (fdin, NULL, fdout, NULL, st.st_size, 0);
> > +  xclose (fdin);
> > +  xclose (fdout);
> > +}
> > +
> > +static void
> > +run_test_expect_failure (void (*modify)(ElfW(Ehdr) *), const char *errmsg)
> > +{
> > +  int fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
> > +  ElfW(Ehdr) orig_hdr;
> > +  if (read (fd, &orig_hdr, sizeof (orig_hdr)) != sizeof (orig_hdr))
> > +    FAIL_EXIT1 ("read (%s): %m\n", tmpbin);
> > +  ElfW(Ehdr) hdr = orig_hdr;
> > +  modify (&hdr);
> > +  if (lseek (fd, 0, SEEK_SET) != 0)
> > +    FAIL_EXIT1 ("lseek: %m");
> > +  xwrite (fd, &hdr, sizeof (hdr));
> > +  xclose (fd);
> > +
> > +  struct support_capture_subprocess proc =
> > +      support_capture_subprogram (spargv[0], spargv);
> > +  support_capture_subprocess_check (&proc, "tst-elf-check", 127,
> > +                                   sc_allow_stderr);
> > +  TEST_VERIFY (strstr (proc.err.buffer, errmsg) != NULL);
> > +  support_capture_subprocess_free (&proc);
> > +
> > +  /* Restore previous header.  */
> > +  fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
> > +  xwrite (fd, &orig_hdr, sizeof (orig_hdr));
> > +  xclose (fd);
> > +}
> > +
> > +static void
> > +modify_mag (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_ident[EI_MAG0] = EI_MAG3;
> > +  ehdr->e_ident[EI_MAG1] = EI_MAG2;
> > +  ehdr->e_ident[EI_MAG2] = EI_MAG1;
> > +  ehdr->e_ident[EI_MAG3] = EI_MAG0;
> > +}
> > +
> > +static void
> > +modify_class (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_ident[EI_CLASS] = ELFCLASSNONE;
> > +}
> > +
> > +static void
> > +modify_endian (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_ident[EI_DATA] = ELFDATANUM;
> > +}
> > +
> > +static void
> > +modify_eiversion (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_ident[EI_VERSION] = EV_CURRENT + 1;
> > +}
> > +
> > +static void
> > +modify_osabi (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_ident[EI_OSABI] = ELFOSABI_STANDALONE;
> > +}
> > +
> > +static void
> > +modify_abiversion (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
> > +}
> > +
> > +static void
> > +modify_pad (ElfW(Ehdr) *ehdr)
> > +{
> > +  memset (&ehdr->e_ident[EI_PAD], 0xff, EI_NIDENT - EI_PAD);
> > +}
> > +
> > +static void
> > +modify_version (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_version = EV_NONE;
> > +}
> > +
> > +static void
> > +modify_type (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_type = ET_NONE;
> > +}
> > +
> > +static void
> > +modify_phentsize (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_phentsize = sizeof (ElfW(Phdr)) + 1;
> > +}
> > +
> > +static void
> > +do_test_kernel (void)
> > +{
> > +  run_test_expect_failure (modify_mag,
> > +                          "invalid ELF header");
> > +  run_test_expect_failure (modify_type,
> > +                          "only ET_DYN and ET_EXEC can be loaded");
> > +  run_test_expect_failure (modify_phentsize,
> > +                          "ELF file's phentsize not the expected size");
> > +  run_test_expect_failure (modify_class,
> > +                          "wrong ELF class");
> > +}
> > +
> > +static void
> > +do_test_common (void)
> > +{
> > +  run_test_expect_failure (modify_endian,
> > +                          "ELF file data encoding not");
> > +  run_test_expect_failure (modify_eiversion,
> > +                          "ELF file version ident does not match current one");
> > +  run_test_expect_failure (modify_pad,
> > +                          "nonzero padding in e_ident");
> > +  run_test_expect_failure (modify_osabi,
> > +                          "ELF file OS ABI invalid");
> > +  run_test_expect_failure (modify_abiversion,
> > +                          "ELF file ABI version invalid");
> > +  run_test_expect_failure (modify_version,
> > +                          "ELF file version does not match current one");
> > +}
> > +
> > +static int
> > +do_test (int argc, char *argv[])
> > +{
> > +  /* We must have one or four parameters:
> > +     + argv[0]:   the application name
> > +     + argv[1]:   path for ld.so        optional
> > +     + argv[2]:   "--library-path"      optional
> > +     + argv[3]:   the library path      optional
> > +     + argv[4/1]: the application name  */
> > +
> > +  bool hardpath = argc == 2;
> > +
> > +  int i;
> > +  for (i = 0; i < argc - 2; i++)
> > +    spargv[i] = argv[i+1];
> > +  spargv[i++] = tmpbin;
> > +  spargv[i++] = (char *) "--direct";
> > +  spargv[i] = NULL;
> > +
> > +  /* Some fields are checked by the kernel results in a execve failure, so skip
> > +     them for --enable-hardcoded-path-in-tests.  */
> > +  if (!hardpath)
> > +    do_test_kernel ();
> > +  do_test_common ();
> > +
> > +  /* Also run the tests without issuing the loader.  */
> > +  if (hardpath)
> > +    return 0;
> > +
> > +  spargv[0] = tmpbin;
> > +  spargv[1] = (char *) "--direct";
> > +  spargv[2] = NULL;
> > +
> > +  do_test_common ();
> > +
> > +  return 0;
> > +}
> > +
> > +#define PREPARE do_prepare
> > +#define TEST_FUNCTION_ARGV do_test
> > +#include <support/test-driver.c>
> > diff --git a/sysdeps/generic/dl-elf-check.h b/sysdeps/generic/dl-elf-check.h
> > new file mode 100644
> > index 0000000000..48eb82e9e7
> > --- /dev/null
> > +++ b/sysdeps/generic/dl-elf-check.h
> > @@ -0,0 +1,28 @@
> > +/* ELF header consistency and ABI checks.
> > +   Copyright (C) 2021 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_ELF_CHECK_H
> > +#define _DL_ELF_CHECK_H
> > +
> > +/* Called from the loader just after the program headers are processed.  */
> > +static inline void
> > +_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
> > +{
> > +}
> > +
> > +#endif
> > diff --git a/sysdeps/unix/sysv/linux/dl-elf-check.h b/sysdeps/unix/sysv/linux/dl-elf-check.h
> > new file mode 100644
> > index 0000000000..9e4925c090
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/dl-elf-check.h
> > @@ -0,0 +1,32 @@
> > +/* ELF header consistency and ABI checks.
> > +   Copyright (C) 2021 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_ELF_CHECK_H
> > +#define _DL_ELF_CHECK_H
> > +
> > +#include <dl-check.h>
> > +
> > +static inline void
> > +_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
> > +{
> > +  int err = _dl_elfhdr_check (ehdr);
> > +  if (err != DL_ELFHDR_OK)
> > +    _dl_fatal_printf ("program loading error: %s\n", _dl_elfhdr_errstr (err));
> > +}
> > +
> > +#endif
> > --
> > 2.32.0
> >
>
>
> --
> H.J.
  
Adhemerval Zanella Netto Nov. 19, 2021, 5:06 p.m. UTC | #3
On 19/11/2021 13:05, H.J. Lu wrote:
> On Fri, Nov 19, 2021 at 7:33 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Fri, Nov 19, 2021 at 7:03 AM Adhemerval Zanella
>> <adhemerval.zanella@linaro.org> wrote:
>>>
>>> The ELF header integrity check is only done on open_verify(), i.e,
>>> for objects explicitly loaded.  For main executable (issued with
>>> execve() for the binary) only kernel checks are done, which does
>>> not check EI_ABIVERSION.
>>>
>>> To enable it, the loader needs to find where the ELF header is placed
>>> at program start, however Linux auxiliary vectors only provides
>>
>> Is it possible to check __ehdr_start?
> 
> Probably not.   I added this to binutils master branch:

I forgot about __ehdr_start, but at rtld.c context the __ehdr_start is the
loader one (not really what we want).  We might try to get the symbol from
the main application (with the extra complication it is a hidden one), but
since we already parse the programs headers it seems simpler to get from
PT_LOAD.

> 
> $ elfedit
> ...
>  --output-abiversion [0-255]
>                               Set output ABIVERSION
> 
> Please use it for both executable and shared library tests.

Is this really an improvement? It will need to either copy it to testroot
along with all its depedencies and/or make multiples copies, elfedit them,
and copy them on testroot.  It seems that edit the ELF directly seem
simpler, since it is only the header that require adjustments.

> 
>>> the program header table (AT_EHDR).  To avoid require upstream
>>> kernel support, the ELF header is implicitly obtained from the PT_LOAD
>>> values by checking for a segment with offset 0 and memory size
>>> different than 0 (For Linux it is start the ELF file issued by
>>> execve()).
>>>
>>> Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
>>> ---
>>>  elf/Makefile                           |   8 +-
>>>  elf/dl-check-err.h                     |  14 ++
>>>  elf/dl-check.c                         | 151 ++++++++++++++++++
>>>  elf/dl-check.h                         |  45 ++++++
>>>  elf/dl-load.c                          | 114 ++------------
>>>  elf/rtld.c                             |   8 +
>>>  elf/tst-elf-check.c                    | 209 +++++++++++++++++++++++++
>>>  sysdeps/generic/dl-elf-check.h         |  28 ++++
>>>  sysdeps/unix/sysv/linux/dl-elf-check.h |  32 ++++
>>>  9 files changed, 507 insertions(+), 102 deletions(-)
>>>  create mode 100644 elf/dl-check-err.h
>>>  create mode 100644 elf/dl-check.c
>>>  create mode 100644 elf/dl-check.h
>>>  create mode 100644 elf/tst-elf-check.c
>>>  create mode 100644 sysdeps/generic/dl-elf-check.h
>>>  create mode 100644 sysdeps/unix/sysv/linux/dl-elf-check.h
>>>
>>> diff --git a/elf/Makefile b/elf/Makefile
>>> index 4723c159cb..f09fc5c6ec 100644
>>> --- a/elf/Makefile
>>> +++ b/elf/Makefile
>>> @@ -36,7 +36,8 @@ dl-routines   = $(addprefix dl-,load lookup object reloc deps \
>>>                                   exception sort-maps lookup-direct \
>>>                                   call-libc-early-init write \
>>>                                   thread_gscope_wait tls_init_tp \
>>> -                                 debug-symbols minimal-malloc)
>>> +                                 debug-symbols minimal-malloc \
>>> +                                 check)
>>>  ifeq (yes,$(use-ldconfig))
>>>  dl-routines += dl-cache
>>>  endif
>>> @@ -238,7 +239,8 @@ tests-internal += loadtest unload unload2 circleload1 \
>>>          tst-ptrguard1 tst-stackguard1 \
>>>          tst-create_format1 tst-tls-surplus tst-dl-hwcaps_split
>>>  tests-container += tst-pldd tst-dlopen-tlsmodid-container \
>>> -  tst-dlopen-self-container tst-preload-pthread-libc
>>> +  tst-dlopen-self-container tst-preload-pthread-libc \
>>> +  tst-elf-check
>>>  test-srcs = tst-pathopt
>>>  selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
>>>  ifneq ($(selinux-enabled),1)
>>> @@ -494,6 +496,8 @@ tests-special += $(objpfx)order-cmp.out $(objpfx)tst-array1-cmp.out \
>>>                  $(objpfx)tst-unused-dep-cmp.out
>>>  endif
>>>
>>> +tst-elf-check-ARGS = -- $(host-test-program-cmd)
>>> +
>>>  ifndef avoid-generated
>>>  # DSO sorting tests:
>>>  # The dso-ordering-test.py script generates testcase source files in $(objpfx),
>>> diff --git a/elf/dl-check-err.h b/elf/dl-check-err.h
>>> new file mode 100644
>>> index 0000000000..6ca5246eb8
>>> --- /dev/null
>>> +++ b/elf/dl-check-err.h
>>> @@ -0,0 +1,14 @@
>>> +_S(DL_ELFHDR_OK, "")
>>> +_S(DL_ELFHDR_ERR_ELFMAG,     N_("invalid ELF header"))
>>> +_S(DL_ELFHDR_ERR_CLASS32,    N_("wrong ELF class: ELFCLASS32"))
>>> +_S(DL_ELFHDR_ERR_CLASS64,    N_("wrong ELF class: ELFCLASS64"))
>>> +_S(DL_ELFHDR_ERR_BENDIAN,    N_("ELF file data encoding not big-endian"))
>>> +_S(DL_ELFHDR_ERR_LENDIAN,    N_("ELF file data encoding not little-endian"))
>>> +_S(DL_ELFHDR_ERR_EIVERSION,  N_("ELF file version ident does not match current one"))
>>> +_S(DL_ELFHDR_ERR_OSABI,      N_("ELF file OS ABI invalid"))
>>> +_S(DL_ELFHDR_ERR_ABIVERSION, N_("ELF file ABI version invalid"))
>>> +_S(DL_ELFHDR_ERR_PAD,        N_("nonzero padding in e_ident"))
>>> +_S(DL_ELFHDR_ERR_VERSION,    N_("ELF file version does not match current one"))
>>> +_S(DL_ELFHDR_ERR_TYPE,       N_("only ET_DYN and ET_EXEC can be loaded"))
>>> +_S(DL_ELFHDR_ERR_PHENTSIZE,  N_("ELF file's phentsize not the expected size"))
>>> +_S(DL_ELFHDR_ERR_INTERNAL,   N_("internal error"))
>>> diff --git a/elf/dl-check.c b/elf/dl-check.c
>>> new file mode 100644
>>> index 0000000000..ef1720df2a
>>> --- /dev/null
>>> +++ b/elf/dl-check.c
>>> @@ -0,0 +1,151 @@
>>> +/* ELF header consistency and ABI checks.
>>> +   Copyright (C) 1995-2021 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/>.  */
>>> +
>>> +#include <array_length.h>
>>> +#include <dl-check.h>
>>> +#include <endian.h>
>>> +#include <ldsodefs.h>
>>> +#include <libintl.h>
>>> +
>>> +int
>>> +_dl_elfhdr_check (const ElfW(Ehdr) *ehdr)
>>> +{
>>> +#define ELF32_CLASS ELFCLASS32
>>> +#define ELF64_CLASS ELFCLASS64
>>> +#if BYTE_ORDER == BIG_ENDIAN
>>> +# define byteorder ELFDATA2MSB
>>> +#elif BYTE_ORDER == LITTLE_ENDIAN
>>> +# define byteorder ELFDATA2LSB
>>> +#else
>>> +# error "Unknown BYTE_ORDER " BYTE_ORDER
>>> +# define byteorder ELFDATANONE
>>> +#endif
>>> +  MORE_ELF_HEADER_DATA;
>>> +  static const unsigned char expected[EI_NIDENT] =
>>> +  {
>>> +    [EI_MAG0] = ELFMAG0,
>>> +    [EI_MAG1] = ELFMAG1,
>>> +    [EI_MAG2] = ELFMAG2,
>>> +    [EI_MAG3] = ELFMAG3,
>>> +    [EI_CLASS] = ELFW(CLASS),
>>> +    [EI_DATA] = byteorder,
>>> +    [EI_VERSION] = EV_CURRENT,
>>> +    [EI_OSABI] = ELFOSABI_SYSV,
>>> +    [EI_ABIVERSION] = 0
>>> +  };
>>> +
>>> +  /* See whether the ELF header is what we expect.  */
>>> +  if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
>>> +                                           EI_ABIVERSION)
>>> +                       || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
>>> +                                                 ehdr->e_ident[EI_ABIVERSION])
>>> +                       || memcmp (&ehdr->e_ident[EI_PAD],
>>> +                                  &expected[EI_PAD],
>>> +                                  EI_NIDENT - EI_PAD) != 0))
>>> +    {
>>> +      /* Something is wrong.  */
>>> +      const Elf32_Word *magp = (const void *) ehdr->e_ident;
>>> +      if (*magp !=
>>> +#if BYTE_ORDER == LITTLE_ENDIAN
>>> +         ((ELFMAG0 << (EI_MAG0 * 8))
>>> +          | (ELFMAG1 << (EI_MAG1 * 8))
>>> +          | (ELFMAG2 << (EI_MAG2 * 8))
>>> +          | (ELFMAG3 << (EI_MAG3 * 8)))
>>> +#else
>>> +         ((ELFMAG0 << (EI_MAG3 * 8))
>>> +          | (ELFMAG1 << (EI_MAG2 * 8))
>>> +          | (ELFMAG2 << (EI_MAG1 * 8))
>>> +          | (ELFMAG3 << (EI_MAG0 * 8)))
>>> +#endif
>>> +         )
>>> +       return DL_ELFHDR_ERR_ELFMAG;
>>> +      else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
>>> +       return ELFW(CLASS) == ELFCLASS32
>>> +              ? DL_ELFHDR_ERR_CLASS64
>>> +              : DL_ELFHDR_ERR_CLASS32;
>>> +      else if (ehdr->e_ident[EI_DATA] != byteorder)
>>> +       {
>>> +         if (BYTE_ORDER == BIG_ENDIAN)
>>> +           return DL_ELFHDR_ERR_BENDIAN;
>>> +         else
>>> +           return DL_ELFHDR_ERR_LENDIAN;
>>> +       }
>>> +      else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
>>> +       return DL_ELFHDR_ERR_EIVERSION;
>>> +      /* XXX We should be able so set system specific versions which are
>>> +        allowed here.  */
>>> +      else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
>>> +       return DL_ELFHDR_ERR_OSABI;
>>> +      else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
>>> +                                     ehdr->e_ident[EI_ABIVERSION]))
>>> +       return DL_ELFHDR_ERR_ABIVERSION;
>>> +      else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
>>> +                      EI_NIDENT - EI_PAD) != 0)
>>> +       return DL_ELFHDR_ERR_PAD;
>>> +      else
>>> +       return DL_ELFHDR_ERR_INTERNAL;
>>> +    }
>>> +
>>> +  if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
>>> +    return DL_ELFHDR_ERR_VERSION;
>>> +  else if (__glibc_unlikely (ehdr->e_type != ET_DYN
>>> +                            && ehdr->e_type != ET_EXEC))
>>> +    return DL_ELFHDR_ERR_TYPE;
>>> +  else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
>>> +    return DL_ELFHDR_ERR_PHENTSIZE;
>>> +
>>> +  return DL_ELFHDR_OK;
>>> +}
>>> +
>>> +static const union elfhdr_errstr_t
>>> +{
>>> +  struct
>>> +  {
>>> +#define _S(n, s) char str##n[sizeof (s)];
>>> +#include "dl-check-err.h"
>>> +#undef _S
>>> +  };
>>> +  char str[0];
>>> +} elfhdr_errstr =
>>> +{
>>> +  {
>>> +#define _S(n, s) s,
>>> +#include "dl-check-err.h"
>>> +#undef _S
>>> +  }
>>> +};
>>> +
>>> +static const unsigned short elfhder_erridx[] =
>>> +{
>>> +#define _S(n, s) [n] = offsetof(union elfhdr_errstr_t, str##n),
>>> +#include "dl-check-err.h"
>>> +#undef _S
>>> +};
>>> +
>>> +const char *
>>> +_dl_elfhdr_errstr (int err)
>>> +{
>>> +#if 0
>>> +  if (err >= 0 && err < array_length (elfhdr_errstr))
>>> +    return elfhdr_errstr[err];
>>> +  return NULL;
>>> +#endif
>>> +  if (err < 0 || err >= array_length (elfhder_erridx))
>>> +    err = 0;
>>> +  return elfhdr_errstr.str + elfhder_erridx[err];
>>> +}
>>> diff --git a/elf/dl-check.h b/elf/dl-check.h
>>> new file mode 100644
>>> index 0000000000..5104a353ed
>>> --- /dev/null
>>> +++ b/elf/dl-check.h
>>> @@ -0,0 +1,45 @@
>>> +/* ELF header consistency and ABI checks.
>>> +   Copyright (C) 1995-2021 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_OPENCHECK_H
>>> +#define _DL_OPENCHECK_H
>>> +
>>> +#include <link.h>
>>> +
>>> +enum
>>> + {
>>> +   DL_ELFHDR_OK,
>>> +   DL_ELFHDR_ERR_ELFMAG,     /* Invalid ELFMAGX value.  */
>>> +   DL_ELFHDR_ERR_CLASS32,    /* Mismatched EI_CLASS.  */
>>> +   DL_ELFHDR_ERR_CLASS64,    /* Mismatched EI_CLASS.  */
>>> +   DL_ELFHDR_ERR_BENDIAN,    /* Mismatched EI_DATA (not big-endian).  */
>>> +   DL_ELFHDR_ERR_LENDIAN,    /* Mismatched EI_DATA (not little-endian).  */
>>> +   DL_ELFHDR_ERR_EIVERSION,  /* Invalid EI_VERSION.  */
>>> +   DL_ELFHDR_ERR_OSABI,      /* Invalid EI_OSABI.  */
>>> +   DL_ELFHDR_ERR_ABIVERSION, /* Invalid ABI vrsion.  */
>>> +   DL_ELFHDR_ERR_PAD,        /* Invalid EI_PAD value.  */
>>> +   DL_ELFHDR_ERR_VERSION,    /* Invalid e_version.  */
>>> +   DL_ELFHDR_ERR_TYPE,       /* Invalid e_type.  */
>>> +   DL_ELFHDR_ERR_PHENTSIZE,  /* Invalid e_phentsize.  */
>>> +   DL_ELFHDR_ERR_INTERNAL    /* Internal error.  */
>>> + };
>>> +
>>> +int _dl_elfhdr_check (const ElfW(Ehdr) *ehdr) attribute_hidden;
>>> +const char *_dl_elfhdr_errstr (int err) attribute_hidden;
>>> +
>>> +#endif
>>> diff --git a/elf/dl-load.c b/elf/dl-load.c
>>> index bf8957e73c..45266c3501 100644
>>> --- a/elf/dl-load.c
>>> +++ b/elf/dl-load.c
>>> @@ -73,19 +73,9 @@ struct filebuf
>>>  #include <dl-machine-reject-phdr.h>
>>>  #include <dl-sysdep-open.h>
>>>  #include <dl-prop.h>
>>> +#include <dl-check.h>
>>>  #include <not-cancel.h>
>>>
>>> -#include <endian.h>
>>> -#if BYTE_ORDER == BIG_ENDIAN
>>> -# define byteorder ELFDATA2MSB
>>> -#elif BYTE_ORDER == LITTLE_ENDIAN
>>> -# define byteorder ELFDATA2LSB
>>> -#else
>>> -# error "Unknown BYTE_ORDER " BYTE_ORDER
>>> -# define byteorder ELFDATANONE
>>> -#endif
>>> -
>>> -#define STRING(x) __STRING (x)
>>>
>>>
>>>  int __stack_prot attribute_hidden attribute_relro
>>> @@ -1598,25 +1588,6 @@ open_verify (const char *name, int fd,
>>>    /* This is the expected ELF header.  */
>>>  #define ELF32_CLASS ELFCLASS32
>>>  #define ELF64_CLASS ELFCLASS64
>>> -#ifndef VALID_ELF_HEADER
>>> -# define VALID_ELF_HEADER(hdr,exp,size)        (memcmp (hdr, exp, size) == 0)
>>> -# define VALID_ELF_OSABI(osabi)                (osabi == ELFOSABI_SYSV)
>>> -# define VALID_ELF_ABIVERSION(osabi,ver) (ver == 0)
>>> -#elif defined MORE_ELF_HEADER_DATA
>>> -  MORE_ELF_HEADER_DATA;
>>> -#endif
>>> -  static const unsigned char expected[EI_NIDENT] =
>>> -  {
>>> -    [EI_MAG0] = ELFMAG0,
>>> -    [EI_MAG1] = ELFMAG1,
>>> -    [EI_MAG2] = ELFMAG2,
>>> -    [EI_MAG3] = ELFMAG3,
>>> -    [EI_CLASS] = ELFW(CLASS),
>>> -    [EI_DATA] = byteorder,
>>> -    [EI_VERSION] = EV_CURRENT,
>>> -    [EI_OSABI] = ELFOSABI_SYSV,
>>> -    [EI_ABIVERSION] = 0
>>> -  };
>>>    static const struct
>>>    {
>>>      ElfW(Word) vendorlen;
>>> @@ -1709,83 +1680,26 @@ open_verify (const char *name, int fd,
>>>         }
>>>
>>>        /* See whether the ELF header is what we expect.  */
>>> -      if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
>>> -                                               EI_ABIVERSION)
>>> -                           || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
>>> -                                                     ehdr->e_ident[EI_ABIVERSION])
>>> -                           || memcmp (&ehdr->e_ident[EI_PAD],
>>> -                                      &expected[EI_PAD],
>>> -                                      EI_NIDENT - EI_PAD) != 0))
>>> +      int err = _dl_elfhdr_check (ehdr);
>>> +      switch (err)
>>>         {
>>> -         /* Something is wrong.  */
>>> -         const Elf32_Word *magp = (const void *) ehdr->e_ident;
>>> -         if (*magp !=
>>> -#if BYTE_ORDER == LITTLE_ENDIAN
>>> -             ((ELFMAG0 << (EI_MAG0 * 8))
>>> -              | (ELFMAG1 << (EI_MAG1 * 8))
>>> -              | (ELFMAG2 << (EI_MAG2 * 8))
>>> -              | (ELFMAG3 << (EI_MAG3 * 8)))
>>> -#else
>>> -             ((ELFMAG0 << (EI_MAG3 * 8))
>>> -              | (ELFMAG1 << (EI_MAG2 * 8))
>>> -              | (ELFMAG2 << (EI_MAG1 * 8))
>>> -              | (ELFMAG3 << (EI_MAG0 * 8)))
>>> -#endif
>>> -             )
>>> -           errstring = N_("invalid ELF header");
>>> -         else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
>>> -           {
>>> -             /* This is not a fatal error.  On architectures where
>>> -                32-bit and 64-bit binaries can be run this might
>>> -                happen.  */
>>> -             *found_other_class = true;
>>> -             goto close_and_out;
>>> -           }
>>> -         else if (ehdr->e_ident[EI_DATA] != byteorder)
>>> -           {
>>> -             if (BYTE_ORDER == BIG_ENDIAN)
>>> -               errstring = N_("ELF file data encoding not big-endian");
>>> -             else
>>> -               errstring = N_("ELF file data encoding not little-endian");
>>> -           }
>>> -         else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
>>> -           errstring
>>> -             = N_("ELF file version ident does not match current one");
>>> -         /* XXX We should be able so set system specific versions which are
>>> -            allowed here.  */
>>> -         else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
>>> -           errstring = N_("ELF file OS ABI invalid");
>>> -         else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
>>> -                                         ehdr->e_ident[EI_ABIVERSION]))
>>> -           errstring = N_("ELF file ABI version invalid");
>>> -         else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
>>> -                          EI_NIDENT - EI_PAD) != 0)
>>> -           errstring = N_("nonzero padding in e_ident");
>>> -         else
>>> -           /* Otherwise we don't know what went wrong.  */
>>> -           errstring = N_("internal error");
>>> +       case DL_ELFHDR_OK:
>>> +         break;
>>>
>>> -         goto lose;
>>> -       }
>>> +       case DL_ELFHDR_ERR_CLASS32:
>>> +       case DL_ELFHDR_ERR_CLASS64:
>>> +         /* This is not a fatal error.  On architectures where 32-bit and
>>> +            64-bit binaries can be run this might happen.  */
>>> +         *found_other_class = true;
>>> +         goto close_and_out;
>>>
>>> -      if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
>>> -       {
>>> -         errstring = N_("ELF file version does not match current one");
>>> +       default:
>>> +         errstring = _dl_elfhdr_errstr (err);
>>>           goto lose;
>>>         }
>>> +
>>>        if (! __glibc_likely (elf_machine_matches_host (ehdr)))
>>>         goto close_and_out;
>>> -      else if (__glibc_unlikely (ehdr->e_type != ET_DYN
>>> -                                && ehdr->e_type != ET_EXEC))
>>> -       {
>>> -         errstring = N_("only ET_DYN and ET_EXEC can be loaded");
>>> -         goto lose;
>>> -       }
>>> -      else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
>>> -       {
>>> -         errstring = N_("ELF file's phentsize not the expected size");
>>> -         goto lose;
>>> -       }
>>>
>>>        maplength = ehdr->e_phnum * sizeof (ElfW(Phdr));
>>>        if (ehdr->e_phoff + maplength <= (size_t) fbp->len)
>>> diff --git a/elf/rtld.c b/elf/rtld.c
>>> index 847141e21d..89b3157f31 100644
>>> --- a/elf/rtld.c
>>> +++ b/elf/rtld.c
>>> @@ -50,6 +50,7 @@
>>>  #include <gnu/lib-names.h>
>>>  #include <dl-tunables.h>
>>>  #include <get-dynamic-info.h>
>>> +#include <dl-elf-check.h>
>>>
>>>  #include <assert.h>
>>>
>>> @@ -1112,6 +1113,7 @@ dl_main (const ElfW(Phdr) *phdr,
>>>          ElfW(Addr) *user_entry,
>>>          ElfW(auxv_t) *auxv)
>>>  {
>>> +  const ElfW(Ehdr) *ehdr = NULL;
>>>    const ElfW(Phdr) *ph;
>>>    struct link_map *main_map;
>>>    size_t file_size;
>>> @@ -1518,6 +1520,9 @@ dl_main (const ElfW(Phdr) *phdr,
>>>           ElfW(Addr) mapstart;
>>>           ElfW(Addr) allocend;
>>>
>>> +         if (ph->p_offset == 0 && ph->p_memsz > 0)
>>> +           ehdr = (void *) ph->p_vaddr;
>>> +
>>>           /* Remember where the main program starts in memory.  */
>>>           mapstart = (main_map->l_addr
>>>                       + (ph->p_vaddr & ~(GLRO(dl_pagesize) - 1)));
>>> @@ -1577,6 +1582,9 @@ dl_main (const ElfW(Phdr) *phdr,
>>>         break;
>>>        }
>>>
>>> +  if (ehdr != NULL)
>>> +    _dl_check_ehdr (ehdr);
>>> +
>>>    /* Adjust the address of the TLS initialization image in case
>>>       the executable is actually an ET_DYN object.  */
>>>    if (main_map->l_tls_initimage != NULL)
>>> diff --git a/elf/tst-elf-check.c b/elf/tst-elf-check.c
>>> new file mode 100644
>>> index 0000000000..175ba6fb5a
>>> --- /dev/null
>>> +++ b/elf/tst-elf-check.c
>>> @@ -0,0 +1,209 @@
>>> +/* Check ELF header error paths.
>>> +   Copyright (C) 2021 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/>.  */
>>> +
>>> +#include <elf.h>
>>> +#include <link.h>
>>> +#include <libc-abis.h>
>>> +#include <fcntl.h>
>>> +#include <string.h>
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <support/capture_subprocess.h>
>>> +#include <support/check.h>
>>> +#include <support/support.h>
>>> +#include <support/xunistd.h>
>>> +#include <support/temp_file.h>
>>> +
>>> +static char *spargv[6];
>>> +static char *tmpbin;
>>> +
>>> +static void
>>> +do_prepare (int argc, char *argv[])
>>> +{
>>> +  int fdin = xopen (argv[0], O_RDONLY | O_LARGEFILE, 0);
>>> +  struct stat64 st;
>>> +  xfstat (fdin, &st);
>>> +  int fdout = create_temp_file ("tst-elf-check-", &tmpbin);
>>> +  xfchmod (fdout, S_IXUSR | S_IRUSR | S_IWUSR);
>>> +  TEST_VERIFY_EXIT (fdout >= 0);
>>> +  xcopy_file_range (fdin, NULL, fdout, NULL, st.st_size, 0);
>>> +  xclose (fdin);
>>> +  xclose (fdout);
>>> +}
>>> +
>>> +static void
>>> +run_test_expect_failure (void (*modify)(ElfW(Ehdr) *), const char *errmsg)
>>> +{
>>> +  int fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
>>> +  ElfW(Ehdr) orig_hdr;
>>> +  if (read (fd, &orig_hdr, sizeof (orig_hdr)) != sizeof (orig_hdr))
>>> +    FAIL_EXIT1 ("read (%s): %m\n", tmpbin);
>>> +  ElfW(Ehdr) hdr = orig_hdr;
>>> +  modify (&hdr);
>>> +  if (lseek (fd, 0, SEEK_SET) != 0)
>>> +    FAIL_EXIT1 ("lseek: %m");
>>> +  xwrite (fd, &hdr, sizeof (hdr));
>>> +  xclose (fd);
>>> +
>>> +  struct support_capture_subprocess proc =
>>> +      support_capture_subprogram (spargv[0], spargv);
>>> +  support_capture_subprocess_check (&proc, "tst-elf-check", 127,
>>> +                                   sc_allow_stderr);
>>> +  TEST_VERIFY (strstr (proc.err.buffer, errmsg) != NULL);
>>> +  support_capture_subprocess_free (&proc);
>>> +
>>> +  /* Restore previous header.  */
>>> +  fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
>>> +  xwrite (fd, &orig_hdr, sizeof (orig_hdr));
>>> +  xclose (fd);
>>> +}
>>> +
>>> +static void
>>> +modify_mag (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_ident[EI_MAG0] = EI_MAG3;
>>> +  ehdr->e_ident[EI_MAG1] = EI_MAG2;
>>> +  ehdr->e_ident[EI_MAG2] = EI_MAG1;
>>> +  ehdr->e_ident[EI_MAG3] = EI_MAG0;
>>> +}
>>> +
>>> +static void
>>> +modify_class (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_ident[EI_CLASS] = ELFCLASSNONE;
>>> +}
>>> +
>>> +static void
>>> +modify_endian (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_ident[EI_DATA] = ELFDATANUM;
>>> +}
>>> +
>>> +static void
>>> +modify_eiversion (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_ident[EI_VERSION] = EV_CURRENT + 1;
>>> +}
>>> +
>>> +static void
>>> +modify_osabi (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_ident[EI_OSABI] = ELFOSABI_STANDALONE;
>>> +}
>>> +
>>> +static void
>>> +modify_abiversion (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
>>> +}
>>> +
>>> +static void
>>> +modify_pad (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  memset (&ehdr->e_ident[EI_PAD], 0xff, EI_NIDENT - EI_PAD);
>>> +}
>>> +
>>> +static void
>>> +modify_version (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_version = EV_NONE;
>>> +}
>>> +
>>> +static void
>>> +modify_type (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_type = ET_NONE;
>>> +}
>>> +
>>> +static void
>>> +modify_phentsize (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_phentsize = sizeof (ElfW(Phdr)) + 1;
>>> +}
>>> +
>>> +static void
>>> +do_test_kernel (void)
>>> +{
>>> +  run_test_expect_failure (modify_mag,
>>> +                          "invalid ELF header");
>>> +  run_test_expect_failure (modify_type,
>>> +                          "only ET_DYN and ET_EXEC can be loaded");
>>> +  run_test_expect_failure (modify_phentsize,
>>> +                          "ELF file's phentsize not the expected size");
>>> +  run_test_expect_failure (modify_class,
>>> +                          "wrong ELF class");
>>> +}
>>> +
>>> +static void
>>> +do_test_common (void)
>>> +{
>>> +  run_test_expect_failure (modify_endian,
>>> +                          "ELF file data encoding not");
>>> +  run_test_expect_failure (modify_eiversion,
>>> +                          "ELF file version ident does not match current one");
>>> +  run_test_expect_failure (modify_pad,
>>> +                          "nonzero padding in e_ident");
>>> +  run_test_expect_failure (modify_osabi,
>>> +                          "ELF file OS ABI invalid");
>>> +  run_test_expect_failure (modify_abiversion,
>>> +                          "ELF file ABI version invalid");
>>> +  run_test_expect_failure (modify_version,
>>> +                          "ELF file version does not match current one");
>>> +}
>>> +
>>> +static int
>>> +do_test (int argc, char *argv[])
>>> +{
>>> +  /* We must have one or four parameters:
>>> +     + argv[0]:   the application name
>>> +     + argv[1]:   path for ld.so        optional
>>> +     + argv[2]:   "--library-path"      optional
>>> +     + argv[3]:   the library path      optional
>>> +     + argv[4/1]: the application name  */
>>> +
>>> +  bool hardpath = argc == 2;
>>> +
>>> +  int i;
>>> +  for (i = 0; i < argc - 2; i++)
>>> +    spargv[i] = argv[i+1];
>>> +  spargv[i++] = tmpbin;
>>> +  spargv[i++] = (char *) "--direct";
>>> +  spargv[i] = NULL;
>>> +
>>> +  /* Some fields are checked by the kernel results in a execve failure, so skip
>>> +     them for --enable-hardcoded-path-in-tests.  */
>>> +  if (!hardpath)
>>> +    do_test_kernel ();
>>> +  do_test_common ();
>>> +
>>> +  /* Also run the tests without issuing the loader.  */
>>> +  if (hardpath)
>>> +    return 0;
>>> +
>>> +  spargv[0] = tmpbin;
>>> +  spargv[1] = (char *) "--direct";
>>> +  spargv[2] = NULL;
>>> +
>>> +  do_test_common ();
>>> +
>>> +  return 0;
>>> +}
>>> +
>>> +#define PREPARE do_prepare
>>> +#define TEST_FUNCTION_ARGV do_test
>>> +#include <support/test-driver.c>
>>> diff --git a/sysdeps/generic/dl-elf-check.h b/sysdeps/generic/dl-elf-check.h
>>> new file mode 100644
>>> index 0000000000..48eb82e9e7
>>> --- /dev/null
>>> +++ b/sysdeps/generic/dl-elf-check.h
>>> @@ -0,0 +1,28 @@
>>> +/* ELF header consistency and ABI checks.
>>> +   Copyright (C) 2021 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_ELF_CHECK_H
>>> +#define _DL_ELF_CHECK_H
>>> +
>>> +/* Called from the loader just after the program headers are processed.  */
>>> +static inline void
>>> +_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
>>> +{
>>> +}
>>> +
>>> +#endif
>>> diff --git a/sysdeps/unix/sysv/linux/dl-elf-check.h b/sysdeps/unix/sysv/linux/dl-elf-check.h
>>> new file mode 100644
>>> index 0000000000..9e4925c090
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/dl-elf-check.h
>>> @@ -0,0 +1,32 @@
>>> +/* ELF header consistency and ABI checks.
>>> +   Copyright (C) 2021 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_ELF_CHECK_H
>>> +#define _DL_ELF_CHECK_H
>>> +
>>> +#include <dl-check.h>
>>> +
>>> +static inline void
>>> +_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
>>> +{
>>> +  int err = _dl_elfhdr_check (ehdr);
>>> +  if (err != DL_ELFHDR_OK)
>>> +    _dl_fatal_printf ("program loading error: %s\n", _dl_elfhdr_errstr (err));
>>> +}
>>> +
>>> +#endif
>>> --
>>> 2.32.0
>>>
>>
>>
>> --
>> H.J.
> 
> 
>
  
H.J. Lu Dec. 6, 2021, 7:03 p.m. UTC | #4
On Fri, Nov 19, 2021 at 7:03 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> The ELF header integrity check is only done on open_verify(), i.e,
> for objects explicitly loaded.  For main executable (issued with
> execve() for the binary) only kernel checks are done, which does
> not check EI_ABIVERSION.
>
> To enable it, the loader needs to find where the ELF header is placed
> at program start, however Linux auxiliary vectors only provides
> the program header table (AT_EHDR).  To avoid require upstream
> kernel support, the ELF header is implicitly obtained from the PT_LOAD
> values by checking for a segment with offset 0 and memory size
> different than 0 (For Linux it is start the ELF file issued by
> execve()).
>
> Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
> ---
>  elf/Makefile                           |   8 +-
>  elf/dl-check-err.h                     |  14 ++
>  elf/dl-check.c                         | 151 ++++++++++++++++++
>  elf/dl-check.h                         |  45 ++++++
>  elf/dl-load.c                          | 114 ++------------
>  elf/rtld.c                             |   8 +
>  elf/tst-elf-check.c                    | 209 +++++++++++++++++++++++++
>  sysdeps/generic/dl-elf-check.h         |  28 ++++
>  sysdeps/unix/sysv/linux/dl-elf-check.h |  32 ++++
>  9 files changed, 507 insertions(+), 102 deletions(-)
>  create mode 100644 elf/dl-check-err.h
>  create mode 100644 elf/dl-check.c
>  create mode 100644 elf/dl-check.h
>  create mode 100644 elf/tst-elf-check.c
>  create mode 100644 sysdeps/generic/dl-elf-check.h
>  create mode 100644 sysdeps/unix/sysv/linux/dl-elf-check.h
>
> diff --git a/elf/Makefile b/elf/Makefile
> index 4723c159cb..f09fc5c6ec 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -36,7 +36,8 @@ dl-routines   = $(addprefix dl-,load lookup object reloc deps \
>                                   exception sort-maps lookup-direct \
>                                   call-libc-early-init write \
>                                   thread_gscope_wait tls_init_tp \
> -                                 debug-symbols minimal-malloc)
> +                                 debug-symbols minimal-malloc \
> +                                 check)
>  ifeq (yes,$(use-ldconfig))
>  dl-routines += dl-cache
>  endif
> @@ -238,7 +239,8 @@ tests-internal += loadtest unload unload2 circleload1 \
>          tst-ptrguard1 tst-stackguard1 \
>          tst-create_format1 tst-tls-surplus tst-dl-hwcaps_split
>  tests-container += tst-pldd tst-dlopen-tlsmodid-container \
> -  tst-dlopen-self-container tst-preload-pthread-libc
> +  tst-dlopen-self-container tst-preload-pthread-libc \
> +  tst-elf-check
>  test-srcs = tst-pathopt
>  selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
>  ifneq ($(selinux-enabled),1)
> @@ -494,6 +496,8 @@ tests-special += $(objpfx)order-cmp.out $(objpfx)tst-array1-cmp.out \
>                  $(objpfx)tst-unused-dep-cmp.out
>  endif
>
> +tst-elf-check-ARGS = -- $(host-test-program-cmd)
> +
>  ifndef avoid-generated
>  # DSO sorting tests:
>  # The dso-ordering-test.py script generates testcase source files in $(objpfx),
> diff --git a/elf/dl-check-err.h b/elf/dl-check-err.h
> new file mode 100644
> index 0000000000..6ca5246eb8
> --- /dev/null
> +++ b/elf/dl-check-err.h
> @@ -0,0 +1,14 @@
> +_S(DL_ELFHDR_OK, "")
> +_S(DL_ELFHDR_ERR_ELFMAG,     N_("invalid ELF header"))
> +_S(DL_ELFHDR_ERR_CLASS32,    N_("wrong ELF class: ELFCLASS32"))
> +_S(DL_ELFHDR_ERR_CLASS64,    N_("wrong ELF class: ELFCLASS64"))
> +_S(DL_ELFHDR_ERR_BENDIAN,    N_("ELF file data encoding not big-endian"))
> +_S(DL_ELFHDR_ERR_LENDIAN,    N_("ELF file data encoding not little-endian"))
> +_S(DL_ELFHDR_ERR_EIVERSION,  N_("ELF file version ident does not match current one"))
> +_S(DL_ELFHDR_ERR_OSABI,      N_("ELF file OS ABI invalid"))
> +_S(DL_ELFHDR_ERR_ABIVERSION, N_("ELF file ABI version invalid"))
> +_S(DL_ELFHDR_ERR_PAD,        N_("nonzero padding in e_ident"))
> +_S(DL_ELFHDR_ERR_VERSION,    N_("ELF file version does not match current one"))
> +_S(DL_ELFHDR_ERR_TYPE,       N_("only ET_DYN and ET_EXEC can be loaded"))
> +_S(DL_ELFHDR_ERR_PHENTSIZE,  N_("ELF file's phentsize not the expected size"))
> +_S(DL_ELFHDR_ERR_INTERNAL,   N_("internal error"))
> diff --git a/elf/dl-check.c b/elf/dl-check.c
> new file mode 100644
> index 0000000000..ef1720df2a
> --- /dev/null
> +++ b/elf/dl-check.c
> @@ -0,0 +1,151 @@
> +/* ELF header consistency and ABI checks.
> +   Copyright (C) 1995-2021 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/>.  */
> +
> +#include <array_length.h>
> +#include <dl-check.h>
> +#include <endian.h>
> +#include <ldsodefs.h>
> +#include <libintl.h>
> +
> +int
> +_dl_elfhdr_check (const ElfW(Ehdr) *ehdr)
> +{
> +#define ELF32_CLASS ELFCLASS32
> +#define ELF64_CLASS ELFCLASS64
> +#if BYTE_ORDER == BIG_ENDIAN
> +# define byteorder ELFDATA2MSB
> +#elif BYTE_ORDER == LITTLE_ENDIAN
> +# define byteorder ELFDATA2LSB
> +#else
> +# error "Unknown BYTE_ORDER " BYTE_ORDER
> +# define byteorder ELFDATANONE
> +#endif
> +  MORE_ELF_HEADER_DATA;
> +  static const unsigned char expected[EI_NIDENT] =
> +  {
> +    [EI_MAG0] = ELFMAG0,
> +    [EI_MAG1] = ELFMAG1,
> +    [EI_MAG2] = ELFMAG2,
> +    [EI_MAG3] = ELFMAG3,
> +    [EI_CLASS] = ELFW(CLASS),
> +    [EI_DATA] = byteorder,
> +    [EI_VERSION] = EV_CURRENT,
> +    [EI_OSABI] = ELFOSABI_SYSV,
> +    [EI_ABIVERSION] = 0
> +  };
> +
> +  /* See whether the ELF header is what we expect.  */
> +  if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
> +                                           EI_ABIVERSION)
> +                       || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> +                                                 ehdr->e_ident[EI_ABIVERSION])
> +                       || memcmp (&ehdr->e_ident[EI_PAD],
> +                                  &expected[EI_PAD],
> +                                  EI_NIDENT - EI_PAD) != 0))
> +    {
> +      /* Something is wrong.  */
> +      const Elf32_Word *magp = (const void *) ehdr->e_ident;
> +      if (*magp !=
> +#if BYTE_ORDER == LITTLE_ENDIAN
> +         ((ELFMAG0 << (EI_MAG0 * 8))
> +          | (ELFMAG1 << (EI_MAG1 * 8))
> +          | (ELFMAG2 << (EI_MAG2 * 8))
> +          | (ELFMAG3 << (EI_MAG3 * 8)))
> +#else
> +         ((ELFMAG0 << (EI_MAG3 * 8))
> +          | (ELFMAG1 << (EI_MAG2 * 8))
> +          | (ELFMAG2 << (EI_MAG1 * 8))
> +          | (ELFMAG3 << (EI_MAG0 * 8)))
> +#endif
> +         )
> +       return DL_ELFHDR_ERR_ELFMAG;
> +      else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
> +       return ELFW(CLASS) == ELFCLASS32
> +              ? DL_ELFHDR_ERR_CLASS64
> +              : DL_ELFHDR_ERR_CLASS32;
> +      else if (ehdr->e_ident[EI_DATA] != byteorder)
> +       {
> +         if (BYTE_ORDER == BIG_ENDIAN)
> +           return DL_ELFHDR_ERR_BENDIAN;
> +         else
> +           return DL_ELFHDR_ERR_LENDIAN;
> +       }
> +      else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
> +       return DL_ELFHDR_ERR_EIVERSION;
> +      /* XXX We should be able so set system specific versions which are
> +        allowed here.  */
> +      else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
> +       return DL_ELFHDR_ERR_OSABI;
> +      else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> +                                     ehdr->e_ident[EI_ABIVERSION]))
> +       return DL_ELFHDR_ERR_ABIVERSION;
> +      else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
> +                      EI_NIDENT - EI_PAD) != 0)
> +       return DL_ELFHDR_ERR_PAD;
> +      else
> +       return DL_ELFHDR_ERR_INTERNAL;
> +    }
> +
> +  if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
> +    return DL_ELFHDR_ERR_VERSION;
> +  else if (__glibc_unlikely (ehdr->e_type != ET_DYN
> +                            && ehdr->e_type != ET_EXEC))
> +    return DL_ELFHDR_ERR_TYPE;
> +  else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
> +    return DL_ELFHDR_ERR_PHENTSIZE;
> +
> +  return DL_ELFHDR_OK;
> +}
> +
> +static const union elfhdr_errstr_t
> +{
> +  struct
> +  {
> +#define _S(n, s) char str##n[sizeof (s)];
> +#include "dl-check-err.h"
> +#undef _S
> +  };
> +  char str[0];
> +} elfhdr_errstr =
> +{
> +  {
> +#define _S(n, s) s,
> +#include "dl-check-err.h"
> +#undef _S
> +  }
> +};
> +
> +static const unsigned short elfhder_erridx[] =
> +{
> +#define _S(n, s) [n] = offsetof(union elfhdr_errstr_t, str##n),
> +#include "dl-check-err.h"
> +#undef _S
> +};
> +
> +const char *
> +_dl_elfhdr_errstr (int err)
> +{
> +#if 0
> +  if (err >= 0 && err < array_length (elfhdr_errstr))
> +    return elfhdr_errstr[err];
> +  return NULL;
> +#endif
> +  if (err < 0 || err >= array_length (elfhder_erridx))
> +    err = 0;
> +  return elfhdr_errstr.str + elfhder_erridx[err];
> +}
> diff --git a/elf/dl-check.h b/elf/dl-check.h
> new file mode 100644
> index 0000000000..5104a353ed
> --- /dev/null
> +++ b/elf/dl-check.h
> @@ -0,0 +1,45 @@
> +/* ELF header consistency and ABI checks.
> +   Copyright (C) 1995-2021 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_OPENCHECK_H
> +#define _DL_OPENCHECK_H
> +
> +#include <link.h>
> +
> +enum
> + {
> +   DL_ELFHDR_OK,
> +   DL_ELFHDR_ERR_ELFMAG,     /* Invalid ELFMAGX value.  */
> +   DL_ELFHDR_ERR_CLASS32,    /* Mismatched EI_CLASS.  */
> +   DL_ELFHDR_ERR_CLASS64,    /* Mismatched EI_CLASS.  */
> +   DL_ELFHDR_ERR_BENDIAN,    /* Mismatched EI_DATA (not big-endian).  */
> +   DL_ELFHDR_ERR_LENDIAN,    /* Mismatched EI_DATA (not little-endian).  */
> +   DL_ELFHDR_ERR_EIVERSION,  /* Invalid EI_VERSION.  */
> +   DL_ELFHDR_ERR_OSABI,      /* Invalid EI_OSABI.  */
> +   DL_ELFHDR_ERR_ABIVERSION, /* Invalid ABI vrsion.  */
> +   DL_ELFHDR_ERR_PAD,        /* Invalid EI_PAD value.  */
> +   DL_ELFHDR_ERR_VERSION,    /* Invalid e_version.  */
> +   DL_ELFHDR_ERR_TYPE,       /* Invalid e_type.  */
> +   DL_ELFHDR_ERR_PHENTSIZE,  /* Invalid e_phentsize.  */
> +   DL_ELFHDR_ERR_INTERNAL    /* Internal error.  */
> + };
> +
> +int _dl_elfhdr_check (const ElfW(Ehdr) *ehdr) attribute_hidden;
> +const char *_dl_elfhdr_errstr (int err) attribute_hidden;
> +
> +#endif
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index bf8957e73c..45266c3501 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -73,19 +73,9 @@ struct filebuf
>  #include <dl-machine-reject-phdr.h>
>  #include <dl-sysdep-open.h>
>  #include <dl-prop.h>
> +#include <dl-check.h>
>  #include <not-cancel.h>
>
> -#include <endian.h>
> -#if BYTE_ORDER == BIG_ENDIAN
> -# define byteorder ELFDATA2MSB
> -#elif BYTE_ORDER == LITTLE_ENDIAN
> -# define byteorder ELFDATA2LSB
> -#else
> -# error "Unknown BYTE_ORDER " BYTE_ORDER
> -# define byteorder ELFDATANONE
> -#endif
> -
> -#define STRING(x) __STRING (x)
>
>
>  int __stack_prot attribute_hidden attribute_relro
> @@ -1598,25 +1588,6 @@ open_verify (const char *name, int fd,
>    /* This is the expected ELF header.  */
>  #define ELF32_CLASS ELFCLASS32
>  #define ELF64_CLASS ELFCLASS64
> -#ifndef VALID_ELF_HEADER
> -# define VALID_ELF_HEADER(hdr,exp,size)        (memcmp (hdr, exp, size) == 0)
> -# define VALID_ELF_OSABI(osabi)                (osabi == ELFOSABI_SYSV)
> -# define VALID_ELF_ABIVERSION(osabi,ver) (ver == 0)
> -#elif defined MORE_ELF_HEADER_DATA
> -  MORE_ELF_HEADER_DATA;
> -#endif
> -  static const unsigned char expected[EI_NIDENT] =
> -  {
> -    [EI_MAG0] = ELFMAG0,
> -    [EI_MAG1] = ELFMAG1,
> -    [EI_MAG2] = ELFMAG2,
> -    [EI_MAG3] = ELFMAG3,
> -    [EI_CLASS] = ELFW(CLASS),
> -    [EI_DATA] = byteorder,
> -    [EI_VERSION] = EV_CURRENT,
> -    [EI_OSABI] = ELFOSABI_SYSV,
> -    [EI_ABIVERSION] = 0
> -  };
>    static const struct
>    {
>      ElfW(Word) vendorlen;
> @@ -1709,83 +1680,26 @@ open_verify (const char *name, int fd,
>         }
>
>        /* See whether the ELF header is what we expect.  */
> -      if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
> -                                               EI_ABIVERSION)
> -                           || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> -                                                     ehdr->e_ident[EI_ABIVERSION])
> -                           || memcmp (&ehdr->e_ident[EI_PAD],
> -                                      &expected[EI_PAD],
> -                                      EI_NIDENT - EI_PAD) != 0))
> +      int err = _dl_elfhdr_check (ehdr);
> +      switch (err)
>         {
> -         /* Something is wrong.  */
> -         const Elf32_Word *magp = (const void *) ehdr->e_ident;
> -         if (*magp !=
> -#if BYTE_ORDER == LITTLE_ENDIAN
> -             ((ELFMAG0 << (EI_MAG0 * 8))
> -              | (ELFMAG1 << (EI_MAG1 * 8))
> -              | (ELFMAG2 << (EI_MAG2 * 8))
> -              | (ELFMAG3 << (EI_MAG3 * 8)))
> -#else
> -             ((ELFMAG0 << (EI_MAG3 * 8))
> -              | (ELFMAG1 << (EI_MAG2 * 8))
> -              | (ELFMAG2 << (EI_MAG1 * 8))
> -              | (ELFMAG3 << (EI_MAG0 * 8)))
> -#endif
> -             )
> -           errstring = N_("invalid ELF header");
> -         else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
> -           {
> -             /* This is not a fatal error.  On architectures where
> -                32-bit and 64-bit binaries can be run this might
> -                happen.  */
> -             *found_other_class = true;
> -             goto close_and_out;
> -           }
> -         else if (ehdr->e_ident[EI_DATA] != byteorder)
> -           {
> -             if (BYTE_ORDER == BIG_ENDIAN)
> -               errstring = N_("ELF file data encoding not big-endian");
> -             else
> -               errstring = N_("ELF file data encoding not little-endian");
> -           }
> -         else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
> -           errstring
> -             = N_("ELF file version ident does not match current one");
> -         /* XXX We should be able so set system specific versions which are
> -            allowed here.  */
> -         else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
> -           errstring = N_("ELF file OS ABI invalid");
> -         else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
> -                                         ehdr->e_ident[EI_ABIVERSION]))
> -           errstring = N_("ELF file ABI version invalid");
> -         else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
> -                          EI_NIDENT - EI_PAD) != 0)
> -           errstring = N_("nonzero padding in e_ident");
> -         else
> -           /* Otherwise we don't know what went wrong.  */
> -           errstring = N_("internal error");
> +       case DL_ELFHDR_OK:
> +         break;
>
> -         goto lose;
> -       }
> +       case DL_ELFHDR_ERR_CLASS32:
> +       case DL_ELFHDR_ERR_CLASS64:
> +         /* This is not a fatal error.  On architectures where 32-bit and
> +            64-bit binaries can be run this might happen.  */
> +         *found_other_class = true;
> +         goto close_and_out;
>
> -      if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
> -       {
> -         errstring = N_("ELF file version does not match current one");
> +       default:
> +         errstring = _dl_elfhdr_errstr (err);
>           goto lose;
>         }
> +
>        if (! __glibc_likely (elf_machine_matches_host (ehdr)))
>         goto close_and_out;
> -      else if (__glibc_unlikely (ehdr->e_type != ET_DYN
> -                                && ehdr->e_type != ET_EXEC))
> -       {
> -         errstring = N_("only ET_DYN and ET_EXEC can be loaded");
> -         goto lose;
> -       }
> -      else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
> -       {
> -         errstring = N_("ELF file's phentsize not the expected size");
> -         goto lose;
> -       }
>
>        maplength = ehdr->e_phnum * sizeof (ElfW(Phdr));
>        if (ehdr->e_phoff + maplength <= (size_t) fbp->len)
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 847141e21d..89b3157f31 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -50,6 +50,7 @@
>  #include <gnu/lib-names.h>
>  #include <dl-tunables.h>
>  #include <get-dynamic-info.h>
> +#include <dl-elf-check.h>
>
>  #include <assert.h>
>
> @@ -1112,6 +1113,7 @@ dl_main (const ElfW(Phdr) *phdr,
>          ElfW(Addr) *user_entry,
>          ElfW(auxv_t) *auxv)
>  {
> +  const ElfW(Ehdr) *ehdr = NULL;
>    const ElfW(Phdr) *ph;
>    struct link_map *main_map;
>    size_t file_size;
> @@ -1518,6 +1520,9 @@ dl_main (const ElfW(Phdr) *phdr,
>           ElfW(Addr) mapstart;
>           ElfW(Addr) allocend;
>
> +         if (ph->p_offset == 0 && ph->p_memsz > 0)
> +           ehdr = (void *) ph->p_vaddr;
> +
>           /* Remember where the main program starts in memory.  */
>           mapstart = (main_map->l_addr
>                       + (ph->p_vaddr & ~(GLRO(dl_pagesize) - 1)));
> @@ -1577,6 +1582,9 @@ dl_main (const ElfW(Phdr) *phdr,
>         break;
>        }
>
> +  if (ehdr != NULL)
> +    _dl_check_ehdr (ehdr);
> +
>    /* Adjust the address of the TLS initialization image in case
>       the executable is actually an ET_DYN object.  */
>    if (main_map->l_tls_initimage != NULL)
> diff --git a/elf/tst-elf-check.c b/elf/tst-elf-check.c
> new file mode 100644
> index 0000000000..175ba6fb5a
> --- /dev/null
> +++ b/elf/tst-elf-check.c
> @@ -0,0 +1,209 @@
> +/* Check ELF header error paths.
> +   Copyright (C) 2021 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/>.  */
> +
> +#include <elf.h>
> +#include <link.h>
> +#include <libc-abis.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <support/capture_subprocess.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <support/temp_file.h>
> +
> +static char *spargv[6];
> +static char *tmpbin;
> +
> +static void
> +do_prepare (int argc, char *argv[])
> +{
> +  int fdin = xopen (argv[0], O_RDONLY | O_LARGEFILE, 0);
> +  struct stat64 st;
> +  xfstat (fdin, &st);
> +  int fdout = create_temp_file ("tst-elf-check-", &tmpbin);
> +  xfchmod (fdout, S_IXUSR | S_IRUSR | S_IWUSR);
> +  TEST_VERIFY_EXIT (fdout >= 0);
> +  xcopy_file_range (fdin, NULL, fdout, NULL, st.st_size, 0);
> +  xclose (fdin);
> +  xclose (fdout);
> +}
> +
> +static void
> +run_test_expect_failure (void (*modify)(ElfW(Ehdr) *), const char *errmsg)
> +{
> +  int fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
> +  ElfW(Ehdr) orig_hdr;
> +  if (read (fd, &orig_hdr, sizeof (orig_hdr)) != sizeof (orig_hdr))
> +    FAIL_EXIT1 ("read (%s): %m\n", tmpbin);
> +  ElfW(Ehdr) hdr = orig_hdr;
> +  modify (&hdr);
> +  if (lseek (fd, 0, SEEK_SET) != 0)
> +    FAIL_EXIT1 ("lseek: %m");
> +  xwrite (fd, &hdr, sizeof (hdr));
> +  xclose (fd);
> +
> +  struct support_capture_subprocess proc =
> +      support_capture_subprogram (spargv[0], spargv);
> +  support_capture_subprocess_check (&proc, "tst-elf-check", 127,
> +                                   sc_allow_stderr);
> +  TEST_VERIFY (strstr (proc.err.buffer, errmsg) != NULL);
> +  support_capture_subprocess_free (&proc);
> +
> +  /* Restore previous header.  */
> +  fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
> +  xwrite (fd, &orig_hdr, sizeof (orig_hdr));
> +  xclose (fd);
> +}
> +
> +static void
> +modify_mag (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_MAG0] = EI_MAG3;
> +  ehdr->e_ident[EI_MAG1] = EI_MAG2;
> +  ehdr->e_ident[EI_MAG2] = EI_MAG1;
> +  ehdr->e_ident[EI_MAG3] = EI_MAG0;
> +}
> +
> +static void
> +modify_class (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_CLASS] = ELFCLASSNONE;
> +}
> +
> +static void
> +modify_endian (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_DATA] = ELFDATANUM;
> +}
> +
> +static void
> +modify_eiversion (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_VERSION] = EV_CURRENT + 1;
> +}
> +
> +static void
> +modify_osabi (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_OSABI] = ELFOSABI_STANDALONE;
> +}
> +
> +static void
> +modify_abiversion (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
> +}
> +
> +static void
> +modify_pad (ElfW(Ehdr) *ehdr)
> +{
> +  memset (&ehdr->e_ident[EI_PAD], 0xff, EI_NIDENT - EI_PAD);
> +}
> +
> +static void
> +modify_version (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_version = EV_NONE;
> +}
> +
> +static void
> +modify_type (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_type = ET_NONE;
> +}
> +
> +static void
> +modify_phentsize (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_phentsize = sizeof (ElfW(Phdr)) + 1;
> +}
> +
> +static void
> +do_test_kernel (void)
> +{
> +  run_test_expect_failure (modify_mag,
> +                          "invalid ELF header");
> +  run_test_expect_failure (modify_type,
> +                          "only ET_DYN and ET_EXEC can be loaded");
> +  run_test_expect_failure (modify_phentsize,
> +                          "ELF file's phentsize not the expected size");
> +  run_test_expect_failure (modify_class,
> +                          "wrong ELF class");
> +}
> +
> +static void
> +do_test_common (void)
> +{
> +  run_test_expect_failure (modify_endian,
> +                          "ELF file data encoding not");
> +  run_test_expect_failure (modify_eiversion,
> +                          "ELF file version ident does not match current one");
> +  run_test_expect_failure (modify_pad,
> +                          "nonzero padding in e_ident");
> +  run_test_expect_failure (modify_osabi,
> +                          "ELF file OS ABI invalid");
> +  run_test_expect_failure (modify_abiversion,
> +                          "ELF file ABI version invalid");
> +  run_test_expect_failure (modify_version,
> +                          "ELF file version does not match current one");
> +}
> +
> +static int
> +do_test (int argc, char *argv[])
> +{
> +  /* We must have one or four parameters:
> +     + argv[0]:   the application name
> +     + argv[1]:   path for ld.so        optional
> +     + argv[2]:   "--library-path"      optional
> +     + argv[3]:   the library path      optional
> +     + argv[4/1]: the application name  */
> +
> +  bool hardpath = argc == 2;
> +
> +  int i;
> +  for (i = 0; i < argc - 2; i++)
> +    spargv[i] = argv[i+1];
> +  spargv[i++] = tmpbin;
> +  spargv[i++] = (char *) "--direct";
> +  spargv[i] = NULL;
> +
> +  /* Some fields are checked by the kernel results in a execve failure, so skip
> +     them for --enable-hardcoded-path-in-tests.  */
> +  if (!hardpath)
> +    do_test_kernel ();
> +  do_test_common ();
> +
> +  /* Also run the tests without issuing the loader.  */
> +  if (hardpath)
> +    return 0;
> +
> +  spargv[0] = tmpbin;
> +  spargv[1] = (char *) "--direct";
> +  spargv[2] = NULL;
> +
> +  do_test_common ();
> +
> +  return 0;
> +}
> +
> +#define PREPARE do_prepare
> +#define TEST_FUNCTION_ARGV do_test
> +#include <support/test-driver.c>
> diff --git a/sysdeps/generic/dl-elf-check.h b/sysdeps/generic/dl-elf-check.h
> new file mode 100644
> index 0000000000..48eb82e9e7
> --- /dev/null
> +++ b/sysdeps/generic/dl-elf-check.h
> @@ -0,0 +1,28 @@
> +/* ELF header consistency and ABI checks.
> +   Copyright (C) 2021 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_ELF_CHECK_H
> +#define _DL_ELF_CHECK_H
> +
> +/* Called from the loader just after the program headers are processed.  */
> +static inline void
> +_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
> +{
> +}
> +
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/dl-elf-check.h b/sysdeps/unix/sysv/linux/dl-elf-check.h
> new file mode 100644
> index 0000000000..9e4925c090
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/dl-elf-check.h
> @@ -0,0 +1,32 @@
> +/* ELF header consistency and ABI checks.
> +   Copyright (C) 2021 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_ELF_CHECK_H
> +#define _DL_ELF_CHECK_H
> +
> +#include <dl-check.h>
> +
> +static inline void
> +_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
> +{
> +  int err = _dl_elfhdr_check (ehdr);
> +  if (err != DL_ELFHDR_OK)
> +    _dl_fatal_printf ("program loading error: %s\n", _dl_elfhdr_errstr (err));
> +}
> +
> +#endif
> --
> 2.32.0
>

LGTM.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Thanks.
  
Florian Weimer Dec. 6, 2021, 7:09 p.m. UTC | #5
* Adhemerval Zanella:

> +static void
> +modify_abiversion (ElfW(Ehdr) *ehdr)
> +{
> +  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
> +}

So this is eventually controlled by the libc-abis file, right?

I *thought* that the consensus was that binutils should bump version if
absolute symbols are used.  But I don't see that in the absolute symbol
tests.

Is this really doing anything?

Thanks,
Florian
  
H.J. Lu Dec. 6, 2021, 7:22 p.m. UTC | #6
On Mon, Dec 6, 2021 at 11:09 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Adhemerval Zanella:
>
> > +static void
> > +modify_abiversion (ElfW(Ehdr) *ehdr)
> > +{
> > +  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
> > +}
>
> So this is eventually controlled by the libc-abis file, right?
>
> I *thought* that the consensus was that binutils should bump version if
> absolute symbols are used.  But I don't see that in the absolute symbol
> tests.
>
> Is this really doing anything?
>

EI_ABIVERSION check works on executables created by the new linker
which bumps EI_ABIVERSION.  This complements the existing
EI_ABIVERSION check on DSOs.  This is orthogonal to the ABI version
check for existing ld.so binaries which needs an ABIVERSION version.
  
Adhemerval Zanella Netto Dec. 6, 2021, 8:31 p.m. UTC | #7
On 06/12/2021 16:22, H.J. Lu wrote:
> On Mon, Dec 6, 2021 at 11:09 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Adhemerval Zanella:
>>
>>> +static void
>>> +modify_abiversion (ElfW(Ehdr) *ehdr)
>>> +{
>>> +  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
>>> +}
>>
>> So this is eventually controlled by the libc-abis file, right?
>>
>> I *thought* that the consensus was that binutils should bump version if
>> absolute symbols are used.  But I don't see that in the absolute symbol
>> tests.
>>
>> Is this really doing anything?
>>
> 
> EI_ABIVERSION check works on executables created by the new linker
> which bumps EI_ABIVERSION.  This complements the existing
> EI_ABIVERSION check on DSOs.  This is orthogonal to the ABI version
> check for existing ld.so binaries which needs an ABIVERSION version.
> 

Currently only mips does actually set and checks different EI_ABIVERSION
through VALID_ELF_ABIVERSION. For instance, -Wl,--hash-style=gnu with
mips64 will set EI_ABIVERSION to 5.
  
Florian Weimer Dec. 6, 2021, 8:37 p.m. UTC | #8
* Adhemerval Zanella:

> On 06/12/2021 16:22, H.J. Lu wrote:
>> On Mon, Dec 6, 2021 at 11:09 AM Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>> * Adhemerval Zanella:
>>>
>>>> +static void
>>>> +modify_abiversion (ElfW(Ehdr) *ehdr)
>>>> +{
>>>> +  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
>>>> +}
>>>
>>> So this is eventually controlled by the libc-abis file, right?
>>>
>>> I *thought* that the consensus was that binutils should bump version if
>>> absolute symbols are used.  But I don't see that in the absolute symbol
>>> tests.
>>>
>>> Is this really doing anything?
>>>
>> 
>> EI_ABIVERSION check works on executables created by the new linker
>> which bumps EI_ABIVERSION.  This complements the existing
>> EI_ABIVERSION check on DSOs.  This is orthogonal to the ABI version
>> check for existing ld.so binaries which needs an ABIVERSION version.
>> 
>
> Currently only mips does actually set and checks different EI_ABIVERSION
> through VALID_ELF_ABIVERSION. For instance, -Wl,--hash-style=gnu with
> mips64 will set EI_ABIVERSION to 5.

What surprised me is that binutils does not seem to *generate* the bump
for ABSOLUTE.  Not so much the lack of checking in glibc.

Thanks,
Florian
  
Adhemerval Zanella Netto Dec. 6, 2021, 9:07 p.m. UTC | #9
On 06/12/2021 17:37, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 06/12/2021 16:22, H.J. Lu wrote:
>>> On Mon, Dec 6, 2021 at 11:09 AM Florian Weimer <fweimer@redhat.com> wrote:
>>>>
>>>> * Adhemerval Zanella:
>>>>
>>>>> +static void
>>>>> +modify_abiversion (ElfW(Ehdr) *ehdr)
>>>>> +{
>>>>> +  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
>>>>> +}
>>>>
>>>> So this is eventually controlled by the libc-abis file, right?
>>>>
>>>> I *thought* that the consensus was that binutils should bump version if
>>>> absolute symbols are used.  But I don't see that in the absolute symbol
>>>> tests.
>>>>
>>>> Is this really doing anything?
>>>>
>>>
>>> EI_ABIVERSION check works on executables created by the new linker
>>> which bumps EI_ABIVERSION.  This complements the existing
>>> EI_ABIVERSION check on DSOs.  This is orthogonal to the ABI version
>>> check for existing ld.so binaries which needs an ABIVERSION version.
>>>
>>
>> Currently only mips does actually set and checks different EI_ABIVERSION
>> through VALID_ELF_ABIVERSION. For instance, -Wl,--hash-style=gnu with
>> mips64 will set EI_ABIVERSION to 5.
> 
> What surprised me is that binutils does not seem to *generate* the bump
> for ABSOLUTE.  Not so much the lack of checking in glibc.

It does, but only for specific configurations.  bintutils does have a
testcase for it, pr21375*, but not all configurations does bump because
they do not require ABS relocations (for instance, for n64 -mmicromips
--defsym hidn=1 does set the ABI version to 4).
  
Florian Weimer Dec. 7, 2021, 3:45 p.m. UTC | #10
* Adhemerval Zanella:

> It does, but only for specific configurations.  bintutils does have a
> testcase for it, pr21375*, but not all configurations does bump because
> they do not require ABS relocations (for instance, for n64 -mmicromips
> --defsym hidn=1 does set the ABI version to 4).

Should this tell us something at DT_RELR?  That binutils doesn't help us
to prevent crashes for other features?

Thanks,
Florian
  
Adhemerval Zanella Netto Dec. 7, 2021, 5:35 p.m. UTC | #11
On 07/12/2021 12:45, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> It does, but only for specific configurations.  bintutils does have a
>> testcase for it, pr21375*, but not all configurations does bump because
>> they do not require ABS relocations (for instance, for n64 -mmicromips
>> --defsym hidn=1 does set the ABI version to 4).
> 
> Should this tell us something at DT_RELR?  That binutils doesn't help us
> to prevent crashes for other features?

I think it tell us that binutils support for DT_RELR or any other potential
abi disruptive feature will need a better ABI enforce for all Linux or
affected ABI.  It seems that binutils support are not really unified with
the multiples architectures and ABI. 

But I think it should be doable on linker side.
  
Fangrui Song Dec. 8, 2021, 12:01 a.m. UTC | #12
On Tue, Dec 7, 2021 at 12:35 PM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 07/12/2021 12:45, Florian Weimer wrote:
> > * Adhemerval Zanella:
> >
> >> It does, but only for specific configurations.  bintutils does have a
> >> testcase for it, pr21375*, but not all configurations does bump because
> >> they do not require ABS relocations (for instance, for n64 -mmicromips
> >> --defsym hidn=1 does set the ABI version to 4).
> >
> > Should this tell us something at DT_RELR?  That binutils doesn't help us
> > to prevent crashes for other features?
>
> I think it tell us that binutils support for DT_RELR or any other potential
> abi disruptive feature will need a better ABI enforce for all Linux or
> affected ABI.  It seems that binutils support are not really unified with
> the multiples architectures and ABI.
>
> But I think it should be doable on linker side.

For DT_RELR, you may see
https://maskray.me/blog/2021-10-31-relative-relocations-and-relr#ei_abiversion
Many Linux executables (STB_GNU_UNIQUE/STT_GNU_IFUNC are not used) use
ELFOSABI_NONE and the linker does not and should not bump
EI_ABIVERSION.
  
Florian Weimer Dec. 8, 2021, 10:19 a.m. UTC | #13
* Fāng-ruì Sòng:

> On Tue, Dec 7, 2021 at 12:35 PM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>>
>>
>> On 07/12/2021 12:45, Florian Weimer wrote:
>> > * Adhemerval Zanella:
>> >
>> >> It does, but only for specific configurations.  bintutils does have a
>> >> testcase for it, pr21375*, but not all configurations does bump because
>> >> they do not require ABS relocations (for instance, for n64 -mmicromips
>> >> --defsym hidn=1 does set the ABI version to 4).
>> >
>> > Should this tell us something at DT_RELR?  That binutils doesn't help us
>> > to prevent crashes for other features?
>>
>> I think it tell us that binutils support for DT_RELR or any other potential
>> abi disruptive feature will need a better ABI enforce for all Linux or
>> affected ABI.  It seems that binutils support are not really unified with
>> the multiples architectures and ABI.
>>
>> But I think it should be doable on linker side.
>
> For DT_RELR, you may see
> https://maskray.me/blog/2021-10-31-relative-relocations-and-relr#ei_abiversion
> Many Linux executables (STB_GNU_UNIQUE/STT_GNU_IFUNC are not used) use
> ELFOSABI_NONE and the linker does not and should not bump
> EI_ABIVERSION.

That radare2 command is really confusing.  It changes the ABI version to
16.  It does not change OSABI to GNU.

So I think we actually agree on the ld behavior, that the
OSABI/ABIVERSION is not really used by binutils today.

One question I meant to ask you: If the GNU toolchain uses any mechanism
for lockout of older glibc, would Google start building binaries using
that mechanism and patch their glibc forks that implement DT_RELR to be
able to load binaries with the lockout?

Thanks,
Florian
  
Fangrui Song Dec. 14, 2021, 12:17 a.m. UTC | #14
On 2021-12-08, Florian Weimer wrote:
>* Fāng-ruì Sòng:
>
>> On Tue, Dec 7, 2021 at 12:35 PM Adhemerval Zanella via Libc-alpha
>> <libc-alpha@sourceware.org> wrote:
>>>
>>>
>>>
>>> On 07/12/2021 12:45, Florian Weimer wrote:
>>> > * Adhemerval Zanella:
>>> >
>>> >> It does, but only for specific configurations.  bintutils does have a
>>> >> testcase for it, pr21375*, but not all configurations does bump because
>>> >> they do not require ABS relocations (for instance, for n64 -mmicromips
>>> >> --defsym hidn=1 does set the ABI version to 4).
>>> >
>>> > Should this tell us something at DT_RELR?  That binutils doesn't help us
>>> > to prevent crashes for other features?
>>>
>>> I think it tell us that binutils support for DT_RELR or any other potential
>>> abi disruptive feature will need a better ABI enforce for all Linux or
>>> affected ABI.  It seems that binutils support are not really unified with
>>> the multiples architectures and ABI.
>>>
>>> But I think it should be doable on linker side.
>>
>> For DT_RELR, you may see
>> https://maskray.me/blog/2021-10-31-relative-relocations-and-relr#ei_abiversion
>> Many Linux executables (STB_GNU_UNIQUE/STT_GNU_IFUNC are not used) use
>> ELFOSABI_NONE and the linker does not and should not bump
>> EI_ABIVERSION.

I was out-of-town for ~10 days. So sorry that I did not reply in time.

>That radare2 command is really confusing.  It changes the ABI version to
>16.  It does not change OSABI to GNU.

Thanks. I clarified my wording and added
"For ELFOSABI_GNU (r2 -nwqc 'wx 03 @ 7'), changing EI_ABIVERSION to 4 or
above will observe the failure with ld.so mapped objects but not with kernel
mapped objects."

>So I think we actually agree on the ld behavior, that the
>OSABI/ABIVERSION is not really used by binutils today.

Agree that OSABI/ABIVERSION is not enforced in the linker.

>One question I meant to ask you: If the GNU toolchain uses any mechanism
>for lockout of older glibc, would Google start building binaries using
>that mechanism and patch their glibc forks that implement DT_RELR to be
>able to load binaries with the lockout?

For ChromeOS patched glibc, I can forward the question to ChromeOS
toolchain folks. Actually the code review is open to the public and you
can comment on
https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/3320511

I think their viewpoint is similar to the Gentoo developer:
https://sourceware.org/pipermail/libc-alpha/2021-November/133388.html
If a group chooses a non-default feature, they need to understand the
implications and they cannot blame the upstream if porting the binary to
old systems would crash.

If the question is: "if upstream glibc implements the diagnostic, will
ChromeOS port the patch to their glibc".  My reply is non-authoritative,
but if this has not been a problem for more than 3 years now, I do know
see large value backporting the feature to their older glibc release.

If the question is to Google internal systems, such a diagnostic would
have a low value as production systems try hard not to have the
mismatching version problem (which is probably common on Linux desktops).
I believe many corporate users will share similar viewpoint.
They may control their production systems in such a way that running
a new executable on an old glibc is either impossible, or can be detected
with other means, so a built-in glibc diagnostic is not that useful.

While I am replying with my corporate email address (because I subscribe
to this list with it), I also need to speak up for other customers I
support as my non-work-related community duty (Android, Fuchsia,
FreeBSD, etc). For Android and Fuchsia, they have the feature for ~3
years now, while I think (running on an old system) is explicitly
unsupported for them, having a ld.so diagnostic (well, they use their
own libc implementations) at this point is definitely nearly useless.
For *BSD, I have heard multiple people saying something in line with
"this is a Linux problem. They'd not add new knobs to the toolchain".
  
Florian Weimer Dec. 14, 2021, 9:03 a.m. UTC | #15
* Fāng-ruì Sòng:

> If the question is: "if upstream glibc implements the diagnostic, will
> ChromeOS port the patch to their glibc".  My reply is non-authoritative,
> but if this has not been a problem for more than 3 years now, I do know
> see large value backporting the feature to their older glibc release.

But if you don't backport, you can teach your toolchain to start
producing binaries that fail to load on older glibc.  You will have to
keep creating binaries that lack proper markup.

My concern is that we go through all this trouble to implement a version
proper handshake, and yet Google binaries will still crash on older
glibc.

Thanks,
Florian
  
Fangrui Song Dec. 14, 2021, 9:09 a.m. UTC | #16
On Tue, Dec 14, 2021 at 1:03 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Fāng-ruì Sòng:
>
> > If the question is: "if upstream glibc implements the diagnostic, will
> > ChromeOS port the patch to their glibc".  My reply is non-authoritative,
> > but if this has not been a problem for more than 3 years now, I do know
> > see large value backporting the feature to their older glibc release.
>
> But if you don't backport, you can teach your toolchain to start
> producing binaries that fail to load on older glibc.  You will have to
> keep creating binaries that lack proper markup.
>
> My concern is that we go through all this trouble to implement a version
> proper handshake, and yet Google binaries will still crash on older
> glibc.

3+ years ago, "still crash on older glibc" was considered an
acceptable compromise as the scenario (cared by some upstream glibc
folks) is simply unsupported (and by many other groups).

Now after 3 years, (while this is unsupported) back porting RELR
executables to less-than-3 year old ChromeOS would still work because
all glibc releases in the past 3 years support RELR. If you consider
symbol versioning, many symbols get new symbols which won't run on
3-year-old glibc anyway.
  
Florian Weimer Dec. 14, 2021, 9:18 a.m. UTC | #17
* Fāng-ruì Sòng:

> On Tue, Dec 14, 2021 at 1:03 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Fāng-ruì Sòng:
>>
>> > If the question is: "if upstream glibc implements the diagnostic, will
>> > ChromeOS port the patch to their glibc".  My reply is non-authoritative,
>> > but if this has not been a problem for more than 3 years now, I do know
>> > see large value backporting the feature to their older glibc release.
>>
>> But if you don't backport, you can teach your toolchain to start
>> producing binaries that fail to load on older glibc.  You will have to
>> keep creating binaries that lack proper markup.
>>
>> My concern is that we go through all this trouble to implement a version
>> proper handshake, and yet Google binaries will still crash on older
>> glibc.
>
> 3+ years ago, "still crash on older glibc" was considered an
> acceptable compromise as the scenario (cared by some upstream glibc
> folks) is simply unsupported (and by many other groups).
>
> Now after 3 years, (while this is unsupported) back porting RELR
> executables to less-than-3 year old ChromeOS would still work because
> all glibc releases in the past 3 years support RELR. If you consider
> symbol versioning, many symbols get new symbols which won't run on
> 3-year-old glibc anyway.

I have a feeling we are talking past each other.

Based on previous mail exchanges, I think you would use an lld option to
build binaries which use DT_RELR but do not have any version lockout for
older glibc (so that these binaries still work on your older glibc with
DT_RELR).

Thanks,
Florian
  
Adhemerval Zanella Netto Dec. 14, 2021, 12:23 p.m. UTC | #18
On 14/12/2021 06:03, Florian Weimer wrote:
> * Fāng-ruì Sòng:
> 
>> If the question is: "if upstream glibc implements the diagnostic, will
>> ChromeOS port the patch to their glibc".  My reply is non-authoritative,
>> but if this has not been a problem for more than 3 years now, I do know
>> see large value backporting the feature to their older glibc release.
> 
> But if you don't backport, you can teach your toolchain to start
> producing binaries that fail to load on older glibc.  You will have to
> keep creating binaries that lack proper markup.
> 
> My concern is that we go through all this trouble to implement a version
> proper handshake, and yet Google binaries will still crash on older
> glibc.

Is ChromeOS glibc really binary compatible with glibc from other general
Linux distributions? Different from google/grte branches, ChromeOS seems 
to work by patching upstream glibc and my impression is they not really care 
nor aim to be (Fāng-ruì remarks also strength this idea).

In any case, ChromeOS's DT_RELR use does show that it is de facto a different
ABI, it is only unfortunate that its ELF ABI does not give us any standard
marking to advertise so (and we will need to resort on some hacking such as
scanning for some symbol or versioning to to do).

I am not sure if we should really care to handle such situations, it would
be nice if we can coordinate with ChromeOS to get it align its ABI with
mainstream and sort DT_RELR but I think *now* it is not a requisite and I
think we should move DT_RELR support independently.
  
Adhemerval Zanella Netto Dec. 14, 2021, 12:28 p.m. UTC | #19
On 14/12/2021 06:09, Fāng-ruì Sòng wrote:
> On Tue, Dec 14, 2021 at 1:03 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Fāng-ruì Sòng:
>>
>>> If the question is: "if upstream glibc implements the diagnostic, will
>>> ChromeOS port the patch to their glibc".  My reply is non-authoritative,
>>> but if this has not been a problem for more than 3 years now, I do know
>>> see large value backporting the feature to their older glibc release.
>>
>> But if you don't backport, you can teach your toolchain to start
>> producing binaries that fail to load on older glibc.  You will have to
>> keep creating binaries that lack proper markup.
>>
>> My concern is that we go through all this trouble to implement a version
>> proper handshake, and yet Google binaries will still crash on older
>> glibc.
> 
> 3+ years ago, "still crash on older glibc" was considered an
> acceptable compromise as the scenario (cared by some upstream glibc
> folks) is simply unsupported (and by many other groups).
> 
> Now after 3 years, (while this is unsupported) back porting RELR
> executables to less-than-3 year old ChromeOS would still work because
> all glibc releases in the past 3 years support RELR. If you consider
> symbol versioning, many symbols get new symbols which won't run on
> 3-year-old glibc anyway.

My view is we are trying to give better users diagnostic to avoid 
downstream discussion and bug reports that ended up in time being
waste to track down unsupported features. It is especially true with
some transitions, like the libpthread merge; which might be troublesome 
for some users.

So IMHO we should aim to to proper diagnostic without resorting in large 
performance penalty and hacks (such as hardcoded symbol/version checks).
  
Fangrui Song Dec. 14, 2021, 7:03 p.m. UTC | #20
On 2021-12-14, Florian Weimer wrote:
>* Fāng-ruì Sòng:
>
>> On Tue, Dec 14, 2021 at 1:03 AM Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>> * Fāng-ruì Sòng:
>>>
>>> > If the question is: "if upstream glibc implements the diagnostic, will
>>> > ChromeOS port the patch to their glibc".  My reply is non-authoritative,
>>> > but if this has not been a problem for more than 3 years now, I do know
>>> > see large value backporting the feature to their older glibc release.
>>>
>>> But if you don't backport, you can teach your toolchain to start
>>> producing binaries that fail to load on older glibc.  You will have to
>>> keep creating binaries that lack proper markup.
>>>
>>> My concern is that we go through all this trouble to implement a version
>>> proper handshake, and yet Google binaries will still crash on older
>>> glibc.
>>
>> 3+ years ago, "still crash on older glibc" was considered an
>> acceptable compromise as the scenario (cared by some upstream glibc
>> folks) is simply unsupported (and by many other groups).
>>
>> Now after 3 years, (while this is unsupported) back porting RELR
>> executables to less-than-3 year old ChromeOS would still work because
>> all glibc releases in the past 3 years support RELR. If you consider
>> symbol versioning, many symbols get new symbols which won't run on
>> 3-year-old glibc anyway.
>
>I have a feeling we are talking past each other.

I think it is just a fundamental difference in our viewpoints:

* I share the feeling with Android, ChromeOS, FreeBSD, Fuchsia, NetBSD, and
   Solaris): the "time travel compatibility" development model is simply
   unsupported and as a compromise poor diagnostics are acceptable.
   (For some groups "unsupported" means "cannot run" with other lockout
   mechanism.)
   The Gentoo Linux developer leans on this side.
   An Void Linux developer and an Arch Linux developer seem to lean on this side, too.
   (They just did not participate the discussion.)

* You and HJ really want a diagnostic for "time travel compatibility",
   even if RELR will remain non-default knob in majority Linux world for
   a long time.

The two camps generally don't interact, but here for the glibc RELR
patch ("Can DT_RELR catch up glibc 2.35?") we interact.

>Based on previous mail exchanges, I think you would use an lld option to
>build binaries which use DT_RELR but do not have any version lockout for
>older glibc (so that these binaries still work on your older glibc with
>DT_RELR).

Perhaps I am not clear about the question. Let me rephrase:
ld.lld --pack-dyn-relocs=relr will remain the current semantics.
It won't add version symbol/bump ABIVERSION to NOT run on older glibc.

Even if a glibc mechanism is developed, it likely has low/no adoption in the first
camp since (1) back porting requires some efforts (2) the mechanism solves a subset
of problems. And mostly importantly, the first camp people don't
consider this a problem.
  
Fangrui Song Dec. 14, 2021, 7:24 p.m. UTC | #21
On 2021-12-14, Adhemerval Zanella wrote:
>
>
>On 14/12/2021 06:03, Florian Weimer wrote:
>> * Fāng-ruì Sòng:
>>
>>> If the question is: "if upstream glibc implements the diagnostic, will
>>> ChromeOS port the patch to their glibc".  My reply is non-authoritative,
>>> but if this has not been a problem for more than 3 years now, I do know
>>> see large value backporting the feature to their older glibc release.
>>
>> But if you don't backport, you can teach your toolchain to start
>> producing binaries that fail to load on older glibc.  You will have to
>> keep creating binaries that lack proper markup.
>>
>> My concern is that we go through all this trouble to implement a version
>> proper handshake, and yet Google binaries will still crash on older
>> glibc.
>
>Is ChromeOS glibc really binary compatible with glibc from other general
>Linux distributions? Different from google/grte branches, ChromeOS seems
>to work by patching upstream glibc and my impression is they not really care
>nor aim to be (Fāng-ruì remarks also strength this idea).

Not binary compatible, like two very different Linux distributions (e.g.
a musl one vs a glibc one; a pure llvm-project toolchain based one vs a
traditional GNU toolchain based one).

>In any case, ChromeOS's DT_RELR use does show that it is de facto a different
>ABI, it is only unfortunate that its ELF ABI does not give us any standard
>marking to advertise so (and we will need to resort on some hacking such as
>scanning for some symbol or versioning to to do).
>
>I am not sure if we should really care to handle such situations, it would
>be nice if we can coordinate with ChromeOS to get it align its ABI with
>mainstream and sort DT_RELR but I think *now* it is not a requisite and I
>think we should move DT_RELR support independently.

Like every glibc Linux distribution carrying some glibc patches which
may decrease binary compatibility with other glibc Linux distributions,
ChromeOS carries some patches like RELR, Disable-float128, and
add-clang-style-FORTIFY patches. (I've heard about complaints about the
latter two patches, they are Clang oriented and will likely benefit
glibc-build-with-clang gets more love:
https://maskray.me/blog/2021-10-10-when-can-glibc-be-built-with-clang)

If you'd like to discuss with them to minimize ABI differences, they
will be happy to have a better communication with the upstream (and, the
software repository and code review website are all open).
Different Linux distributions have different priorities and focus.

For RELR, my suggestion is:

* add the DT_RELR patch without lockout to glibc 2.35
* if some distributions are really fond of "time travel compatibility",
   develop the lockout mechanism (in 2.36 or 2.37)
* add GNU ld support
* After a long time, add a know to GCC to pass the glibc oriented option to ld
* I'll likely not add a lackout mechanism to ld.lld --pack-dyn-relocs.
* After many years, make GCC default to the RELR linker option.
  
Adhemerval Zanella Netto Dec. 14, 2021, 9:07 p.m. UTC | #22
On 14/12/2021 16:24, Fangrui Song wrote:
> On 2021-12-14, Adhemerval Zanella wrote:
>>
>>
>> On 14/12/2021 06:03, Florian Weimer wrote:
>>> * Fāng-ruì Sòng:
>>>
>>>> If the question is: "if upstream glibc implements the diagnostic, will
>>>> ChromeOS port the patch to their glibc".  My reply is non-authoritative,
>>>> but if this has not been a problem for more than 3 years now, I do know
>>>> see large value backporting the feature to their older glibc release.
>>>
>>> But if you don't backport, you can teach your toolchain to start
>>> producing binaries that fail to load on older glibc.  You will have to
>>> keep creating binaries that lack proper markup.
>>>
>>> My concern is that we go through all this trouble to implement a version
>>> proper handshake, and yet Google binaries will still crash on older
>>> glibc.
>>
>> Is ChromeOS glibc really binary compatible with glibc from other general
>> Linux distributions? Different from google/grte branches, ChromeOS seems
>> to work by patching upstream glibc and my impression is they not really care
>> nor aim to be (Fāng-ruì remarks also strength this idea).
> 
> Not binary compatible, like two very different Linux distributions (e.g.
> a musl one vs a glibc one; a pure llvm-project toolchain based one vs a
> traditional GNU toolchain based one).

These are essentially two different ABIs due complete different libc, 
afaik musl support on gcc is doing with a different triplet to make it
explicit.

> 
>> In any case, ChromeOS's DT_RELR use does show that it is de facto a different
>> ABI, it is only unfortunate that its ELF ABI does not give us any standard
>> marking to advertise so (and we will need to resort on some hacking such as
>> scanning for some symbol or versioning to to do).
>>
>> I am not sure if we should really care to handle such situations, it would
>> be nice if we can coordinate with ChromeOS to get it align its ABI with
>> mainstream and sort DT_RELR but I think *now* it is not a requisite and I
>> think we should move DT_RELR support independently.
> 
> Like every glibc Linux distribution carrying some glibc patches which
> may decrease binary compatibility with other glibc Linux distributions,
> ChromeOS carries some patches like RELR, Disable-float128, and
> add-clang-style-FORTIFY patches. (I've heard about complaints about the
> latter two patches, they are Clang oriented and will likely benefit
> glibc-build-with-clang gets more love:
> https://maskray.me/blog/2021-10-10-when-can-glibc-be-built-with-clang)

My understanding it is something distributions *really* avoids to do.  
The only explicit deviation from upstream regarding ABI I am aware
is SH [1], where Debian carries a patch to enable some FPU mode switch
(I am not sure why it is not upstream). For instance, even for powerpc64le
IBM and Red Hat pushed hard to avoid diverge from upstream regarding 
starting ABI version (which resulted in some heated discussion to which
based ABI to use).

In any case, both RELR and float128 disable reinforce my point that
ChromeOS is a complete distinct ABI that I don't see much point in
trying to handle/sanitize it.  I think we should handle the same way we
do for objects builds against a different libc (for instance trying to 
load a shared library built against musl).

> 
> If you'd like to discuss with them to minimize ABI differences, they
> will be happy to have a better communication with the upstream (and, the
> software repository and code review website are all open).
> Different Linux distributions have different priorities and focus.

Yes, but I as far I as know they do not deviate from glibc upsteam ABI and
as long you build against a older libc, retro-compatibility is
maintained.  That's not the case for ChromeOS.

> 
> For RELR, my suggestion is:
> 
> * add the DT_RELR patch without lockout to glibc 2.35
> * if some distributions are really fond of "time travel compatibility",
>   develop the lockout mechanism (in 2.36 or 2.37)
> * add GNU ld support
> * After a long time, add a know to GCC to pass the glibc oriented option to ld
> * I'll likely not add a lackout mechanism to ld.lld --pack-dyn-relocs.
> * After many years, make GCC default to the RELR linker option.

I agree with your points, but I still thin using a different EI_ABIVERSION
for RELR should improve user experience for a opt-in feature.  I am
aware this does not help current way where kernel neither glibc checks
is for the main executable, but we can backport this fix and it might
support for general distribution once/if RELR is supported by GNU tools.


[1] https://sourceware.org/glibc/wiki/Release/2.30#SH
  
Fangrui Song Dec. 14, 2021, 9:30 p.m. UTC | #23
On 2021-12-14, Adhemerval Zanella wrote:
>
>
>On 14/12/2021 16:24, Fangrui Song wrote:
>> On 2021-12-14, Adhemerval Zanella wrote:
>>>
>>>
>>> On 14/12/2021 06:03, Florian Weimer wrote:
>>>> * Fāng-ruì Sòng:
>>>>
>>>>> If the question is: "if upstream glibc implements the diagnostic, will
>>>>> ChromeOS port the patch to their glibc".  My reply is non-authoritative,
>>>>> but if this has not been a problem for more than 3 years now, I do know
>>>>> see large value backporting the feature to their older glibc release.
>>>>
>>>> But if you don't backport, you can teach your toolchain to start
>>>> producing binaries that fail to load on older glibc.  You will have to
>>>> keep creating binaries that lack proper markup.
>>>>
>>>> My concern is that we go through all this trouble to implement a version
>>>> proper handshake, and yet Google binaries will still crash on older
>>>> glibc.
>>>
>>> Is ChromeOS glibc really binary compatible with glibc from other general
>>> Linux distributions? Different from google/grte branches, ChromeOS seems
>>> to work by patching upstream glibc and my impression is they not really care
>>> nor aim to be (Fāng-ruì remarks also strength this idea).
>>
>> Not binary compatible, like two very different Linux distributions (e.g.
>> a musl one vs a glibc one; a pure llvm-project toolchain based one vs a
>> traditional GNU toolchain based one).
>
>These are essentially two different ABIs due complete different libc,
>afaik musl support on gcc is doing with a different triplet to make it
>explicit.
>
>>
>>> In any case, ChromeOS's DT_RELR use does show that it is de facto a different
>>> ABI, it is only unfortunate that its ELF ABI does not give us any standard
>>> marking to advertise so (and we will need to resort on some hacking such as
>>> scanning for some symbol or versioning to to do).
>>>
>>> I am not sure if we should really care to handle such situations, it would
>>> be nice if we can coordinate with ChromeOS to get it align its ABI with
>>> mainstream and sort DT_RELR but I think *now* it is not a requisite and I
>>> think we should move DT_RELR support independently.
>>
>> Like every glibc Linux distribution carrying some glibc patches which
>> may decrease binary compatibility with other glibc Linux distributions,
>> ChromeOS carries some patches like RELR, Disable-float128, and
>> add-clang-style-FORTIFY patches. (I've heard about complaints about the
>> latter two patches, they are Clang oriented and will likely benefit
>> glibc-build-with-clang gets more love:
>> https://maskray.me/blog/2021-10-10-when-can-glibc-be-built-with-clang)
>
>My understanding it is something distributions *really* avoids to do.
>The only explicit deviation from upstream regarding ABI I am aware
>is SH [1], where Debian carries a patch to enable some FPU mode switch
>(I am not sure why it is not upstream). For instance, even for powerpc64le
>IBM and Red Hat pushed hard to avoid diverge from upstream regarding
>starting ABI version (which resulted in some heated discussion to which
>based ABI to use).
>
>In any case, both RELR and float128 disable reinforce my point that
>ChromeOS is a complete distinct ABI that I don't see much point in
>trying to handle/sanitize it.  I think we should handle the same way we
>do for objects builds against a different libc (for instance trying to
>load a shared library built against musl).

Should we consider different target triples as different ABI?

* Debian/Ubuntu use Debian-style multiarch target triples like x86_64-linux-gnu.
* RedHat uses target triples like x86_64-redhat-linux (considered questionable by some folks: https://reviews.llvm.org/D110900)
* Suse uses target triples like x86_64-suse-linux (considered questionable by some folks).

Currently there is no rigid enforcement that one target triple doesn't
run on another, but a mechanism could be developed if you want to
prevent "running one distro's exe on another leads to crash".
This practice can make some software unhappy as they currently release
binary releases which may run a different distro, which even if brittle,
sometimes/oftentimes work.

>>
>> If you'd like to discuss with them to minimize ABI differences, they
>> will be happy to have a better communication with the upstream (and, the
>> software repository and code review website are all open).
>> Different Linux distributions have different priorities and focus.
>
>Yes, but I as far I as know they do not deviate from glibc upsteam ABI and
>as long you build against a older libc, retro-compatibility is
>maintained.  That's not the case for ChromeOS.
>
>>
>> For RELR, my suggestion is:
>>
>> * add the DT_RELR patch without lockout to glibc 2.35
>> * if some distributions are really fond of "time travel compatibility",
>>   develop the lockout mechanism (in 2.36 or 2.37)
>> * add GNU ld support
>> * After a long time, add a know to GCC to pass the glibc oriented option to ld
>> * I'll likely not add a lackout mechanism to ld.lld --pack-dyn-relocs.
>> * After many years, make GCC default to the RELR linker option.
>
>I agree with your points, but I still thin using a different EI_ABIVERSION
>for RELR should improve user experience for a opt-in feature.  I am
>aware this does not help current way where kernel neither glibc checks
>is for the main executable, but we can backport this fix and it might
>support for general distribution once/if RELR is supported by GNU tools.
>
>
>[1] https://sourceware.org/glibc/wiki/Release/2.30#SH

As my blog post says, ELFOSABI_NONE will unlikely change EI_ABIVERSION for RELR.
Is the proposal "if ld.bfd --pack-dyn-relocs=relr-glibc is specified,
and the output is otherwise ELFOSABI_NONE or ELFOSABI_GNU, use
ELFOSABI_GNU and bump EI_ABIVERSION to 4"?  This ELFOSABI_GNU is not
elegant as RELR is not a GNU extension. OK, I have objection but not so
strongly.

Since ld.lld --pack-dyn-relocs=relr is there and won't change the
behavior, ld.lld's relr-glibc will likely just be an alias without doing
the EI_ABIVERSION dance.
  
Florian Weimer Dec. 14, 2021, 9:53 p.m. UTC | #24
* Fangrui Song:

> Should we consider different target triples as different ABI?
>
> * Debian/Ubuntu use Debian-style multiarch target triples like x86_64-linux-gnu.
> * RedHat uses target triples like x86_64-redhat-linux (considered questionable by some folks: https://reviews.llvm.org/D110900)
> * Suse uses target triples like x86_64-suse-linux (considered questionable by some folks).

Debian path layout isn't quite compatible with glibc upstream
(/etc/ld.so.cache papers over that to some extent), but that's not the
issue here.

The LLVM review refers to an internal GCC path that can be queried from
the gcc command line, so this is mostly a performance issue or
distribution integration issue (e.g., llvm/clang package postinst could
run gcc to determine the right path).  Even if you know the subdirectory
under /usr/lib/gcc to use, you still need to guess the appropriate
compiler version.  So this is really deeply internal stuff.

config.guess reports x86_64-pc-linux-gnu on these systems, so there is
no material difference.  These internal GCC paths are not required at
all to match the triplet.

> Currently there is no rigid enforcement that one target triple doesn't
> run on another, but a mechanism could be developed if you want to
> prevent "running one distro's exe on another leads to crash".
> This practice can make some software unhappy as they currently release
> binary releases which may run a different distro, which even if brittle,
> sometimes/oftentimes work.

Debian, Red Hat, and SUSE pay fairly close attention to each other and
make sure that binaries are more or less portable, as far as the core
library version constraints permit (e.g. you can move a Debian 11 binary
that uses just glibc and libstdc++ to Red Hat Enterprise Linux 8 and it
is expected to work, unless of course it relies on some Debian specifics
in its application logic).

LSB may be dead, but its spirit lives on in the form of
distribution-independent ABIs for the core run-time libraries.  A
current example is the work on the libgfortran float128 ABI for POWER.
This is a bunch of tricky ABI work which could have been avoided by a
flag day and essentially a private per-distribution ABI (the ABI would
per quasi-distribution-specific for some time because different
distributions would switch to float128 at different times).

A much smaller example is the recent glibc rseq integration, which has a
large part that can be backported without ABI changes, for distributions
that are interested in the sched_getcpu acceleration.

> As my blog post says, ELFOSABI_NONE will unlikely change EI_ABIVERSION for RELR.
> Is the proposal "if ld.bfd --pack-dyn-relocs=relr-glibc is specified,
> and the output is otherwise ELFOSABI_NONE or ELFOSABI_GNU, use
> ELFOSABI_GNU and bump EI_ABIVERSION to 4"?  This ELFOSABI_GNU is not
> elegant as RELR is not a GNU extension. OK, I have objection but not so
> strongly.
>
> Since ld.lld --pack-dyn-relocs=relr is there and won't change the
> behavior, ld.lld's relr-glibc will likely just be an alias without doing
> the EI_ABIVERSION dance.

If ld.lld will ignore what the ABI says regarding feature lockout, we
have two options:

* Ditch the idea of the lockout because it won't work reliably anyway.

* In upstream glibc, accept DT_RELR only if the lockout is also present.
  That is, ld.lld DT_RELR will not work with upstream glibc until it
  produces the proper markup.

The second approach would qualify as the nuclear option, I guess.

Thanks,
Florian
  
Fangrui Song Dec. 14, 2021, 11:08 p.m. UTC | #25
On 2021-12-14, Florian Weimer wrote:
>* Fangrui Song:
>
>> As my blog post says, ELFOSABI_NONE will unlikely change EI_ABIVERSION for RELR.
>> Is the proposal "if ld.bfd --pack-dyn-relocs=relr-glibc is specified,
>> and the output is otherwise ELFOSABI_NONE or ELFOSABI_GNU, use
>> ELFOSABI_GNU and bump EI_ABIVERSION to 4"?  This ELFOSABI_GNU is not
>> elegant as RELR is not a GNU extension. OK, I have objection but not so
>> strongly.
>>
>> Since ld.lld --pack-dyn-relocs=relr is there and won't change the
>> behavior, ld.lld's relr-glibc will likely just be an alias without doing
>> the EI_ABIVERSION dance.
>
>If ld.lld will ignore what the ABI says regarding feature lockout, we
>have two options:
>
>* Ditch the idea of the lockout because it won't work reliably anyway.
>
>* In upstream glibc, accept DT_RELR only if the lockout is also present.
>  That is, ld.lld DT_RELR will not work with upstream glibc until it
>  produces the proper markup.
>
>The second approach would qualify as the nuclear option, I guess.
>
>Thanks,
>Florian

If glibc ether gives up DT_RELR completely or forces ld.lld to comply with the EI_ABIVERSION scheme[1],
I think I have no choice but to teach ld.lld the glibc style RELR.
My ultimate goal is to let the Linux glibc world have RELR.
If there is some acceptable compromise on ld.lld side, I can take it just with pain.
It is unfortunate that

* we incur implementation costs to both glibc and linkers
* tag a generic-abi feature as ELFOSABI_GNU
* it is not reliable: currently released 2.33/2.34/etc won't catch kernel mapped objects. (while we could also backport DT_RELR to these releases)
* older glibc will be protected by symbol versioning anyway. the mechanism would just protect very recent glibc versions, maybe just 2.34? does it protect 2.33?
* new architectures need to remember the EI_ABIVERSION value has been taken by glibc.

[1]: if ld.bfd --pack-dyn-relocs=relr-glibc is specified, and the output
is otherwise ELFOSABI_NONE or ELFOSABI_GNU, use ELFOSABI_GNU and bump
EI_ABIVERSION to 4
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 4723c159cb..f09fc5c6ec 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -36,7 +36,8 @@  dl-routines	= $(addprefix dl-,load lookup object reloc deps \
 				  exception sort-maps lookup-direct \
 				  call-libc-early-init write \
 				  thread_gscope_wait tls_init_tp \
-				  debug-symbols minimal-malloc)
+				  debug-symbols minimal-malloc \
+				  check)
 ifeq (yes,$(use-ldconfig))
 dl-routines += dl-cache
 endif
@@ -238,7 +239,8 @@  tests-internal += loadtest unload unload2 circleload1 \
 	 tst-ptrguard1 tst-stackguard1 \
 	 tst-create_format1 tst-tls-surplus tst-dl-hwcaps_split
 tests-container += tst-pldd tst-dlopen-tlsmodid-container \
-  tst-dlopen-self-container tst-preload-pthread-libc
+  tst-dlopen-self-container tst-preload-pthread-libc \
+  tst-elf-check
 test-srcs = tst-pathopt
 selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
 ifneq ($(selinux-enabled),1)
@@ -494,6 +496,8 @@  tests-special += $(objpfx)order-cmp.out $(objpfx)tst-array1-cmp.out \
 		 $(objpfx)tst-unused-dep-cmp.out
 endif
 
+tst-elf-check-ARGS = -- $(host-test-program-cmd)
+
 ifndef avoid-generated
 # DSO sorting tests:
 # The dso-ordering-test.py script generates testcase source files in $(objpfx),
diff --git a/elf/dl-check-err.h b/elf/dl-check-err.h
new file mode 100644
index 0000000000..6ca5246eb8
--- /dev/null
+++ b/elf/dl-check-err.h
@@ -0,0 +1,14 @@ 
+_S(DL_ELFHDR_OK, "")
+_S(DL_ELFHDR_ERR_ELFMAG,     N_("invalid ELF header"))
+_S(DL_ELFHDR_ERR_CLASS32,    N_("wrong ELF class: ELFCLASS32"))
+_S(DL_ELFHDR_ERR_CLASS64,    N_("wrong ELF class: ELFCLASS64"))
+_S(DL_ELFHDR_ERR_BENDIAN,    N_("ELF file data encoding not big-endian"))
+_S(DL_ELFHDR_ERR_LENDIAN,    N_("ELF file data encoding not little-endian"))
+_S(DL_ELFHDR_ERR_EIVERSION,  N_("ELF file version ident does not match current one"))
+_S(DL_ELFHDR_ERR_OSABI,      N_("ELF file OS ABI invalid"))
+_S(DL_ELFHDR_ERR_ABIVERSION, N_("ELF file ABI version invalid"))
+_S(DL_ELFHDR_ERR_PAD,        N_("nonzero padding in e_ident"))
+_S(DL_ELFHDR_ERR_VERSION,    N_("ELF file version does not match current one"))
+_S(DL_ELFHDR_ERR_TYPE,       N_("only ET_DYN and ET_EXEC can be loaded"))
+_S(DL_ELFHDR_ERR_PHENTSIZE,  N_("ELF file's phentsize not the expected size"))
+_S(DL_ELFHDR_ERR_INTERNAL,   N_("internal error"))
diff --git a/elf/dl-check.c b/elf/dl-check.c
new file mode 100644
index 0000000000..ef1720df2a
--- /dev/null
+++ b/elf/dl-check.c
@@ -0,0 +1,151 @@ 
+/* ELF header consistency and ABI checks.
+   Copyright (C) 1995-2021 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/>.  */
+
+#include <array_length.h>
+#include <dl-check.h>
+#include <endian.h>
+#include <ldsodefs.h>
+#include <libintl.h>
+
+int
+_dl_elfhdr_check (const ElfW(Ehdr) *ehdr)
+{
+#define ELF32_CLASS ELFCLASS32
+#define ELF64_CLASS ELFCLASS64
+#if BYTE_ORDER == BIG_ENDIAN
+# define byteorder ELFDATA2MSB
+#elif BYTE_ORDER == LITTLE_ENDIAN
+# define byteorder ELFDATA2LSB
+#else
+# error "Unknown BYTE_ORDER " BYTE_ORDER
+# define byteorder ELFDATANONE
+#endif
+  MORE_ELF_HEADER_DATA;
+  static const unsigned char expected[EI_NIDENT] =
+  {
+    [EI_MAG0] = ELFMAG0,
+    [EI_MAG1] = ELFMAG1,
+    [EI_MAG2] = ELFMAG2,
+    [EI_MAG3] = ELFMAG3,
+    [EI_CLASS] = ELFW(CLASS),
+    [EI_DATA] = byteorder,
+    [EI_VERSION] = EV_CURRENT,
+    [EI_OSABI] = ELFOSABI_SYSV,
+    [EI_ABIVERSION] = 0
+  };
+
+  /* See whether the ELF header is what we expect.  */
+  if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
+					    EI_ABIVERSION)
+			|| !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
+						  ehdr->e_ident[EI_ABIVERSION])
+			|| memcmp (&ehdr->e_ident[EI_PAD],
+				   &expected[EI_PAD],
+				   EI_NIDENT - EI_PAD) != 0))
+    {
+      /* Something is wrong.  */
+      const Elf32_Word *magp = (const void *) ehdr->e_ident;
+      if (*magp !=
+#if BYTE_ORDER == LITTLE_ENDIAN
+	  ((ELFMAG0 << (EI_MAG0 * 8))
+	   | (ELFMAG1 << (EI_MAG1 * 8))
+	   | (ELFMAG2 << (EI_MAG2 * 8))
+	   | (ELFMAG3 << (EI_MAG3 * 8)))
+#else
+	  ((ELFMAG0 << (EI_MAG3 * 8))
+	   | (ELFMAG1 << (EI_MAG2 * 8))
+	   | (ELFMAG2 << (EI_MAG1 * 8))
+	   | (ELFMAG3 << (EI_MAG0 * 8)))
+#endif
+	  )
+	return DL_ELFHDR_ERR_ELFMAG;
+      else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
+	return ELFW(CLASS) == ELFCLASS32
+	       ? DL_ELFHDR_ERR_CLASS64
+	       : DL_ELFHDR_ERR_CLASS32;
+      else if (ehdr->e_ident[EI_DATA] != byteorder)
+	{
+	  if (BYTE_ORDER == BIG_ENDIAN)
+	    return DL_ELFHDR_ERR_BENDIAN;
+	  else
+	    return DL_ELFHDR_ERR_LENDIAN;
+	}
+      else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
+	return DL_ELFHDR_ERR_EIVERSION;
+      /* XXX We should be able so set system specific versions which are
+	 allowed here.  */
+      else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
+	return DL_ELFHDR_ERR_OSABI;
+      else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
+				      ehdr->e_ident[EI_ABIVERSION]))
+	return DL_ELFHDR_ERR_ABIVERSION;
+      else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
+		       EI_NIDENT - EI_PAD) != 0)
+	return DL_ELFHDR_ERR_PAD;
+      else
+	return DL_ELFHDR_ERR_INTERNAL;
+    }
+
+  if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
+    return DL_ELFHDR_ERR_VERSION;
+  else if (__glibc_unlikely (ehdr->e_type != ET_DYN
+			     && ehdr->e_type != ET_EXEC))
+    return DL_ELFHDR_ERR_TYPE;
+  else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
+    return DL_ELFHDR_ERR_PHENTSIZE;
+
+  return DL_ELFHDR_OK;
+}
+
+static const union elfhdr_errstr_t
+{
+  struct
+  {
+#define _S(n, s) char str##n[sizeof (s)];
+#include "dl-check-err.h"
+#undef _S
+  };
+  char str[0];
+} elfhdr_errstr =
+{
+  {
+#define _S(n, s) s,
+#include "dl-check-err.h"
+#undef _S
+  }
+};
+
+static const unsigned short elfhder_erridx[] =
+{
+#define _S(n, s) [n] = offsetof(union elfhdr_errstr_t, str##n),
+#include "dl-check-err.h"
+#undef _S
+};
+
+const char *
+_dl_elfhdr_errstr (int err)
+{
+#if 0
+  if (err >= 0 && err < array_length (elfhdr_errstr))
+    return elfhdr_errstr[err];
+  return NULL;
+#endif
+  if (err < 0 || err >= array_length (elfhder_erridx))
+    err = 0;
+  return elfhdr_errstr.str + elfhder_erridx[err];
+}
diff --git a/elf/dl-check.h b/elf/dl-check.h
new file mode 100644
index 0000000000..5104a353ed
--- /dev/null
+++ b/elf/dl-check.h
@@ -0,0 +1,45 @@ 
+/* ELF header consistency and ABI checks.
+   Copyright (C) 1995-2021 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_OPENCHECK_H
+#define _DL_OPENCHECK_H
+
+#include <link.h>
+
+enum
+ {
+   DL_ELFHDR_OK,
+   DL_ELFHDR_ERR_ELFMAG,     /* Invalid ELFMAGX value.  */
+   DL_ELFHDR_ERR_CLASS32,    /* Mismatched EI_CLASS.  */
+   DL_ELFHDR_ERR_CLASS64,    /* Mismatched EI_CLASS.  */
+   DL_ELFHDR_ERR_BENDIAN,    /* Mismatched EI_DATA (not big-endian).  */
+   DL_ELFHDR_ERR_LENDIAN,    /* Mismatched EI_DATA (not little-endian).  */
+   DL_ELFHDR_ERR_EIVERSION,  /* Invalid EI_VERSION.  */
+   DL_ELFHDR_ERR_OSABI,      /* Invalid EI_OSABI.  */
+   DL_ELFHDR_ERR_ABIVERSION, /* Invalid ABI vrsion.  */
+   DL_ELFHDR_ERR_PAD,        /* Invalid EI_PAD value.  */
+   DL_ELFHDR_ERR_VERSION,    /* Invalid e_version.  */
+   DL_ELFHDR_ERR_TYPE,       /* Invalid e_type.  */
+   DL_ELFHDR_ERR_PHENTSIZE,  /* Invalid e_phentsize.  */
+   DL_ELFHDR_ERR_INTERNAL    /* Internal error.  */
+ };
+
+int _dl_elfhdr_check (const ElfW(Ehdr) *ehdr) attribute_hidden;
+const char *_dl_elfhdr_errstr (int err) attribute_hidden;
+
+#endif
diff --git a/elf/dl-load.c b/elf/dl-load.c
index bf8957e73c..45266c3501 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -73,19 +73,9 @@  struct filebuf
 #include <dl-machine-reject-phdr.h>
 #include <dl-sysdep-open.h>
 #include <dl-prop.h>
+#include <dl-check.h>
 #include <not-cancel.h>
 
-#include <endian.h>
-#if BYTE_ORDER == BIG_ENDIAN
-# define byteorder ELFDATA2MSB
-#elif BYTE_ORDER == LITTLE_ENDIAN
-# define byteorder ELFDATA2LSB
-#else
-# error "Unknown BYTE_ORDER " BYTE_ORDER
-# define byteorder ELFDATANONE
-#endif
-
-#define STRING(x) __STRING (x)
 
 
 int __stack_prot attribute_hidden attribute_relro
@@ -1598,25 +1588,6 @@  open_verify (const char *name, int fd,
   /* This is the expected ELF header.  */
 #define ELF32_CLASS ELFCLASS32
 #define ELF64_CLASS ELFCLASS64
-#ifndef VALID_ELF_HEADER
-# define VALID_ELF_HEADER(hdr,exp,size)	(memcmp (hdr, exp, size) == 0)
-# define VALID_ELF_OSABI(osabi)		(osabi == ELFOSABI_SYSV)
-# define VALID_ELF_ABIVERSION(osabi,ver) (ver == 0)
-#elif defined MORE_ELF_HEADER_DATA
-  MORE_ELF_HEADER_DATA;
-#endif
-  static const unsigned char expected[EI_NIDENT] =
-  {
-    [EI_MAG0] = ELFMAG0,
-    [EI_MAG1] = ELFMAG1,
-    [EI_MAG2] = ELFMAG2,
-    [EI_MAG3] = ELFMAG3,
-    [EI_CLASS] = ELFW(CLASS),
-    [EI_DATA] = byteorder,
-    [EI_VERSION] = EV_CURRENT,
-    [EI_OSABI] = ELFOSABI_SYSV,
-    [EI_ABIVERSION] = 0
-  };
   static const struct
   {
     ElfW(Word) vendorlen;
@@ -1709,83 +1680,26 @@  open_verify (const char *name, int fd,
 	}
 
       /* See whether the ELF header is what we expect.  */
-      if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
-						EI_ABIVERSION)
-			    || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
-						      ehdr->e_ident[EI_ABIVERSION])
-			    || memcmp (&ehdr->e_ident[EI_PAD],
-				       &expected[EI_PAD],
-				       EI_NIDENT - EI_PAD) != 0))
+      int err = _dl_elfhdr_check (ehdr);
+      switch (err)
 	{
-	  /* Something is wrong.  */
-	  const Elf32_Word *magp = (const void *) ehdr->e_ident;
-	  if (*magp !=
-#if BYTE_ORDER == LITTLE_ENDIAN
-	      ((ELFMAG0 << (EI_MAG0 * 8))
-	       | (ELFMAG1 << (EI_MAG1 * 8))
-	       | (ELFMAG2 << (EI_MAG2 * 8))
-	       | (ELFMAG3 << (EI_MAG3 * 8)))
-#else
-	      ((ELFMAG0 << (EI_MAG3 * 8))
-	       | (ELFMAG1 << (EI_MAG2 * 8))
-	       | (ELFMAG2 << (EI_MAG1 * 8))
-	       | (ELFMAG3 << (EI_MAG0 * 8)))
-#endif
-	      )
-	    errstring = N_("invalid ELF header");
-	  else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
-	    {
-	      /* This is not a fatal error.  On architectures where
-		 32-bit and 64-bit binaries can be run this might
-		 happen.  */
-	      *found_other_class = true;
-	      goto close_and_out;
-	    }
-	  else if (ehdr->e_ident[EI_DATA] != byteorder)
-	    {
-	      if (BYTE_ORDER == BIG_ENDIAN)
-		errstring = N_("ELF file data encoding not big-endian");
-	      else
-		errstring = N_("ELF file data encoding not little-endian");
-	    }
-	  else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
-	    errstring
-	      = N_("ELF file version ident does not match current one");
-	  /* XXX We should be able so set system specific versions which are
-	     allowed here.  */
-	  else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
-	    errstring = N_("ELF file OS ABI invalid");
-	  else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
-					  ehdr->e_ident[EI_ABIVERSION]))
-	    errstring = N_("ELF file ABI version invalid");
-	  else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
-			   EI_NIDENT - EI_PAD) != 0)
-	    errstring = N_("nonzero padding in e_ident");
-	  else
-	    /* Otherwise we don't know what went wrong.  */
-	    errstring = N_("internal error");
+	case DL_ELFHDR_OK:
+	  break;
 
-	  goto lose;
-	}
+	case DL_ELFHDR_ERR_CLASS32:
+	case DL_ELFHDR_ERR_CLASS64:
+	  /* This is not a fatal error.  On architectures where 32-bit and
+	     64-bit binaries can be run this might happen.  */
+	  *found_other_class = true;
+	  goto close_and_out;
 
-      if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
-	{
-	  errstring = N_("ELF file version does not match current one");
+	default:
+	  errstring = _dl_elfhdr_errstr (err);
 	  goto lose;
 	}
+
       if (! __glibc_likely (elf_machine_matches_host (ehdr)))
 	goto close_and_out;
-      else if (__glibc_unlikely (ehdr->e_type != ET_DYN
-				 && ehdr->e_type != ET_EXEC))
-	{
-	  errstring = N_("only ET_DYN and ET_EXEC can be loaded");
-	  goto lose;
-	}
-      else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
-	{
-	  errstring = N_("ELF file's phentsize not the expected size");
-	  goto lose;
-	}
 
       maplength = ehdr->e_phnum * sizeof (ElfW(Phdr));
       if (ehdr->e_phoff + maplength <= (size_t) fbp->len)
diff --git a/elf/rtld.c b/elf/rtld.c
index 847141e21d..89b3157f31 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -50,6 +50,7 @@ 
 #include <gnu/lib-names.h>
 #include <dl-tunables.h>
 #include <get-dynamic-info.h>
+#include <dl-elf-check.h>
 
 #include <assert.h>
 
@@ -1112,6 +1113,7 @@  dl_main (const ElfW(Phdr) *phdr,
 	 ElfW(Addr) *user_entry,
 	 ElfW(auxv_t) *auxv)
 {
+  const ElfW(Ehdr) *ehdr = NULL;
   const ElfW(Phdr) *ph;
   struct link_map *main_map;
   size_t file_size;
@@ -1518,6 +1520,9 @@  dl_main (const ElfW(Phdr) *phdr,
 	  ElfW(Addr) mapstart;
 	  ElfW(Addr) allocend;
 
+	  if (ph->p_offset == 0 && ph->p_memsz > 0)
+	    ehdr = (void *) ph->p_vaddr;
+
 	  /* Remember where the main program starts in memory.  */
 	  mapstart = (main_map->l_addr
 		      + (ph->p_vaddr & ~(GLRO(dl_pagesize) - 1)));
@@ -1577,6 +1582,9 @@  dl_main (const ElfW(Phdr) *phdr,
 	break;
       }
 
+  if (ehdr != NULL)
+    _dl_check_ehdr (ehdr);
+
   /* Adjust the address of the TLS initialization image in case
      the executable is actually an ET_DYN object.  */
   if (main_map->l_tls_initimage != NULL)
diff --git a/elf/tst-elf-check.c b/elf/tst-elf-check.c
new file mode 100644
index 0000000000..175ba6fb5a
--- /dev/null
+++ b/elf/tst-elf-check.c
@@ -0,0 +1,209 @@ 
+/* Check ELF header error paths.
+   Copyright (C) 2021 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/>.  */
+
+#include <elf.h>
+#include <link.h>
+#include <libc-abis.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <support/temp_file.h>
+
+static char *spargv[6];
+static char *tmpbin;
+
+static void
+do_prepare (int argc, char *argv[])
+{
+  int fdin = xopen (argv[0], O_RDONLY | O_LARGEFILE, 0);
+  struct stat64 st;
+  xfstat (fdin, &st);
+  int fdout = create_temp_file ("tst-elf-check-", &tmpbin);
+  xfchmod (fdout, S_IXUSR | S_IRUSR | S_IWUSR);
+  TEST_VERIFY_EXIT (fdout >= 0);
+  xcopy_file_range (fdin, NULL, fdout, NULL, st.st_size, 0);
+  xclose (fdin);
+  xclose (fdout);
+}
+
+static void
+run_test_expect_failure (void (*modify)(ElfW(Ehdr) *), const char *errmsg)
+{
+  int fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
+  ElfW(Ehdr) orig_hdr;
+  if (read (fd, &orig_hdr, sizeof (orig_hdr)) != sizeof (orig_hdr))
+    FAIL_EXIT1 ("read (%s): %m\n", tmpbin);
+  ElfW(Ehdr) hdr = orig_hdr;
+  modify (&hdr);
+  if (lseek (fd, 0, SEEK_SET) != 0)
+    FAIL_EXIT1 ("lseek: %m");
+  xwrite (fd, &hdr, sizeof (hdr));
+  xclose (fd);
+
+  struct support_capture_subprocess proc =
+      support_capture_subprogram (spargv[0], spargv);
+  support_capture_subprocess_check (&proc, "tst-elf-check", 127,
+				    sc_allow_stderr);
+  TEST_VERIFY (strstr (proc.err.buffer, errmsg) != NULL);
+  support_capture_subprocess_free (&proc);
+
+  /* Restore previous header.  */
+  fd = xopen (tmpbin, O_RDWR | O_LARGEFILE, 0);
+  xwrite (fd, &orig_hdr, sizeof (orig_hdr));
+  xclose (fd);
+}
+
+static void
+modify_mag (ElfW(Ehdr) *ehdr)
+{
+  ehdr->e_ident[EI_MAG0] = EI_MAG3;
+  ehdr->e_ident[EI_MAG1] = EI_MAG2;
+  ehdr->e_ident[EI_MAG2] = EI_MAG1;
+  ehdr->e_ident[EI_MAG3] = EI_MAG0;
+}
+
+static void
+modify_class (ElfW(Ehdr) *ehdr)
+{
+  ehdr->e_ident[EI_CLASS] = ELFCLASSNONE;
+}
+
+static void
+modify_endian (ElfW(Ehdr) *ehdr)
+{
+  ehdr->e_ident[EI_DATA] = ELFDATANUM;
+}
+
+static void
+modify_eiversion (ElfW(Ehdr) *ehdr)
+{
+  ehdr->e_ident[EI_VERSION] = EV_CURRENT + 1;
+}
+
+static void
+modify_osabi (ElfW(Ehdr) *ehdr)
+{
+  ehdr->e_ident[EI_OSABI] = ELFOSABI_STANDALONE;
+}
+
+static void
+modify_abiversion (ElfW(Ehdr) *ehdr)
+{
+  ehdr->e_ident[EI_ABIVERSION] = LIBC_ABI_MAX;
+}
+
+static void
+modify_pad (ElfW(Ehdr) *ehdr)
+{
+  memset (&ehdr->e_ident[EI_PAD], 0xff, EI_NIDENT - EI_PAD);
+}
+
+static void
+modify_version (ElfW(Ehdr) *ehdr)
+{
+  ehdr->e_version = EV_NONE;
+}
+
+static void
+modify_type (ElfW(Ehdr) *ehdr)
+{
+  ehdr->e_type = ET_NONE;
+}
+
+static void
+modify_phentsize (ElfW(Ehdr) *ehdr)
+{
+  ehdr->e_phentsize = sizeof (ElfW(Phdr)) + 1;
+}
+
+static void
+do_test_kernel (void)
+{
+  run_test_expect_failure (modify_mag,
+			   "invalid ELF header");
+  run_test_expect_failure (modify_type,
+			   "only ET_DYN and ET_EXEC can be loaded");
+  run_test_expect_failure (modify_phentsize,
+			   "ELF file's phentsize not the expected size");
+  run_test_expect_failure (modify_class,
+			   "wrong ELF class");
+}
+
+static void
+do_test_common (void)
+{
+  run_test_expect_failure (modify_endian,
+			   "ELF file data encoding not");
+  run_test_expect_failure (modify_eiversion,
+			   "ELF file version ident does not match current one");
+  run_test_expect_failure (modify_pad,
+			   "nonzero padding in e_ident");
+  run_test_expect_failure (modify_osabi,
+			   "ELF file OS ABI invalid");
+  run_test_expect_failure (modify_abiversion,
+			   "ELF file ABI version invalid");
+  run_test_expect_failure (modify_version,
+			   "ELF file version does not match current one");
+}
+
+static int
+do_test (int argc, char *argv[])
+{
+  /* We must have one or four parameters:
+     + argv[0]:   the application name
+     + argv[1]:   path for ld.so        optional
+     + argv[2]:   "--library-path"      optional
+     + argv[3]:   the library path      optional
+     + argv[4/1]: the application name  */
+
+  bool hardpath = argc == 2;
+
+  int i;
+  for (i = 0; i < argc - 2; i++)
+    spargv[i] = argv[i+1];
+  spargv[i++] = tmpbin;
+  spargv[i++] = (char *) "--direct";
+  spargv[i] = NULL;
+
+  /* Some fields are checked by the kernel results in a execve failure, so skip
+     them for --enable-hardcoded-path-in-tests.  */
+  if (!hardpath)
+    do_test_kernel ();
+  do_test_common ();
+
+  /* Also run the tests without issuing the loader.  */
+  if (hardpath)
+    return 0;
+
+  spargv[0] = tmpbin;
+  spargv[1] = (char *) "--direct";
+  spargv[2] = NULL;
+
+  do_test_common ();
+
+  return 0;
+}
+
+#define PREPARE do_prepare
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
diff --git a/sysdeps/generic/dl-elf-check.h b/sysdeps/generic/dl-elf-check.h
new file mode 100644
index 0000000000..48eb82e9e7
--- /dev/null
+++ b/sysdeps/generic/dl-elf-check.h
@@ -0,0 +1,28 @@ 
+/* ELF header consistency and ABI checks.
+   Copyright (C) 2021 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_ELF_CHECK_H
+#define _DL_ELF_CHECK_H
+
+/* Called from the loader just after the program headers are processed.  */
+static inline void
+_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
+{
+}
+
+#endif
diff --git a/sysdeps/unix/sysv/linux/dl-elf-check.h b/sysdeps/unix/sysv/linux/dl-elf-check.h
new file mode 100644
index 0000000000..9e4925c090
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/dl-elf-check.h
@@ -0,0 +1,32 @@ 
+/* ELF header consistency and ABI checks.
+   Copyright (C) 2021 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_ELF_CHECK_H
+#define _DL_ELF_CHECK_H
+
+#include <dl-check.h>
+
+static inline void
+_dl_check_ehdr (const ElfW(Ehdr) *ehdr)
+{
+  int err = _dl_elfhdr_check (ehdr);
+  if (err != DL_ELFHDR_OK)
+    _dl_fatal_printf ("program loading error: %s\n", _dl_elfhdr_errstr (err));
+}
+
+#endif