[RFC] elf: Add elf checks for main executable

  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 main_map->l_map_start.  For Linux it is start the ELF file
issued by execve().
---
 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                             |   3 +
 elf/tst-elf-check.c                    | 209 +++++++++++++++++++++++++
 sysdeps/generic/dl-elf-check.h         |  28 ++++
 sysdeps/unix/sysv/linux/dl-elf-check.h |  33 ++++
 9 files changed, 503 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
  

On Thu, Nov 18, 2021 at 11:55 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.

I believe the EI_ABIVERSION check on the executable should be
done in kernel, not in glibc, so that static PIE can be checked easily.

> 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 main_map->l_map_start.  For Linux it is start the ELF file
> issued by execve().
> ---
>  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                             |   3 +
>  elf/tst-elf-check.c                    | 209 +++++++++++++++++++++++++
>  sysdeps/generic/dl-elf-check.h         |  28 ++++
>  sysdeps/unix/sysv/linux/dl-elf-check.h |  33 ++++
>  9 files changed, 503 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 525f302d1a..a036890546 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
> @@ -237,7 +238,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)
> @@ -491,6 +493,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 9f4fa9617d..e80914b76a 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
> @@ -1596,25 +1586,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;
> @@ -1707,83 +1678,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 be2d5d8e74..fd32735ee9 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>
>
> @@ -1576,6 +1577,8 @@ dl_main (const ElfW(Phdr) *phdr,
>         break;
>        }
>
> +  _dl_check_ehdr (main_map);
> +
>    /* 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..98ad3ca723
> --- /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 (struct link_map *main_map)
> +{
> +}
> +
> +#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..f9c08a4b8e
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/dl-elf-check.h
> @@ -0,0 +1,33 @@
> +/* 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 (struct link_map *main_map)
> +{
> +  ElfW(Ehdr) *ehdr = (void *) main_map->l_map_start;
> +  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
>
  

* Adhemerval Zanella:

> +static inline void
> +_dl_check_ehdr (struct link_map *main_map)
> +{
> +  ElfW(Ehdr) *ehdr = (void *) main_map->l_map_start;
> +  int err = _dl_elfhdr_check (ehdr);
> +  if (err != DL_ELFHDR_OK)
> +    _dl_fatal_printf ("program loading error: %s\n", _dl_elfhdr_errstr (err));
> +}

I think you need to iterate through the load segments and see if there
is one with p_offset == 0 and p_memsz > 0 (the ELF header is smaller
than a page).  That's the one to use for this check.  If there is no
such segment, we have to skip the check.

Thanks,
Florian
  

On 18/11/2021 16:58, H.J. Lu wrote:
>  On Thu, Nov 18, 2021 at 11:55 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.
> 
> I believe the EI_ABIVERSION check on the executable should be
> done in kernel, not in glibc, so that static PIE can be checked easily.

My understanding is EI_ABIVERSION is purely a userland information,
kernel should not act uppon it.
  

* H. J. Lu:

>  On Thu, Nov 18, 2021 at 11:55 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.
>
> I believe the EI_ABIVERSION check on the executable should be
> done in kernel, not in glibc, so that static PIE can be checked easily.

There is no need to check statically linked binaries.  The kernel does
not do any relocation processing.  The link editor plus the rest of the
toolchain can make sure t hat the built-in relocator can process the
relocations in the program.

Thanks,
Florian
  

On 18/11/2021 17:00, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> +static inline void
>> +_dl_check_ehdr (struct link_map *main_map)
>> +{
>> +  ElfW(Ehdr) *ehdr = (void *) main_map->l_map_start;
>> +  int err = _dl_elfhdr_check (ehdr);
>> +  if (err != DL_ELFHDR_OK)
>> +    _dl_fatal_printf ("program loading error: %s\n", _dl_elfhdr_errstr (err));
>> +}
> 
> I think you need to iterate through the load segments and see if there
> is one with p_offset == 0 and p_memsz > 0 (the ELF header is smaller
> than a page).  That's the one to use for this check.  If there is no
> such segment, we have to skip the check.

Hum, should we set the l_map_start as well only if we found such segment?
  

On 18/11/2021 17:01, Florian Weimer wrote:
> * H. J. Lu:
> 
>>  On Thu, Nov 18, 2021 at 11:55 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.
>>
>> I believe the EI_ABIVERSION check on the executable should be
>> done in kernel, not in glibc, so that static PIE can be checked easily.
> 
> There is no need to check statically linked binaries.  The kernel does
> not do any relocation processing.  The link editor plus the rest of the
> toolchain can make sure t hat the built-in relocator can process the
> relocations in the program.

I was not really thinking about the relocation process itself, but
that the EI_ABIVERSION might advertise different things for different
libcs and the kernel should be agnostic regarding it.
  

* Adhemerval Zanella:

> On 18/11/2021 17:00, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> +static inline void
>>> +_dl_check_ehdr (struct link_map *main_map)
>>> +{
>>> +  ElfW(Ehdr) *ehdr = (void *) main_map->l_map_start;
>>> +  int err = _dl_elfhdr_check (ehdr);
>>> +  if (err != DL_ELFHDR_OK)
>>> +    _dl_fatal_printf ("program loading error: %s\n", _dl_elfhdr_errstr (err));
>>> +}
>> 
>> I think you need to iterate through the load segments and see if there
>> is one with p_offset == 0 and p_memsz > 0 (the ELF header is smaller
>> than a page).  That's the one to use for this check.  If there is no
>> such segment, we have to skip the check.
>
> Hum, should we set the l_map_start as well only if we found such segment?

No, we need l_map_start in _dl_find_dso_for_object (among other places).

Thanks,
Florian
  

On Thu, Nov 18, 2021 at 12:01 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> >  On Thu, Nov 18, 2021 at 11:55 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.
> >
> > I believe the EI_ABIVERSION check on the executable should be
> > done in kernel, not in glibc, so that static PIE can be checked easily.
>
> There is no need to check statically linked binaries.  The kernel does
> not do any relocation processing.  The link editor plus the rest of the
> toolchain can make sure t hat the built-in relocator can process the
> relocations in the program.

You are right.   But if we want to change ld.so on all glibc branches
to check EI_ABIVERSION, won't it be easier to implement something
like DT_REQUIRED, DT_MANDATORY, DT_CRITICAL?
  

On Thu, Nov 18, 2021 at 12:08 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Nov 18, 2021 at 12:01 PM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu:
> >
> > >  On Thu, Nov 18, 2021 at 11:55 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.
> > >
> > > I believe the EI_ABIVERSION check on the executable should be
> > > done in kernel, not in glibc, so that static PIE can be checked easily.
> >
> > There is no need to check statically linked binaries.  The kernel does
> > not do any relocation processing.  The link editor plus the rest of the
> > toolchain can make sure t hat the built-in relocator can process the
> > relocations in the program.
>
> You are right.   But if we want to change ld.so on all glibc branches
> to check EI_ABIVERSION, won't it be easier to implement something
> like DT_REQUIRED, DT_MANDATORY, DT_CRITICAL?
>

We can add GLIBC_PRIVATE_RELR when DT_RELR is added linking
against glibc:

Version needs section '.gnu.version_r' contains 1 entry:
 Addr: 0x0000000000400470  Offset: 0x000470  Link: 8 (.dynstr)
  000000: Version: 1  File: libc.so.6  Cnt: 2
  0x0010:   Name: GLIBC_PRIVATE_RELR  Flags: none  Version: 3
  0x0020:   Name: GLIBC_2.34  Flags: none  Version: 2