Re: [PATCH] elf: Check for corrupt symbol version info

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Aug 22, 2018 at 9:43 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> The BFD linker with PR ld/23499 may generate shared libraries with
> corrupt symbol version info which leads to linker error when the
> corrupt shared library is used:
>
> /usr/bin/ld: bin/libKF5Service.so.5.49.0: _edata: invalid version 21 (max 0)
> /usr/bin/ld: bin/libKF5Service.so.5.49.0: error adding symbols: bad value
>
> Add check for corrupt symbol version info to objdump:
>
> 00000000000af005 g    D  .data  0000000000000000  <corrupt>   _edata
>
> and readelf:
>
>    728: 00000000000af005     0 NOTYPE  GLOBAL DEFAULT   25 _edata@<corrupt> (5)

If there are no objections,  I will check it in next week.

> bfd/
>
>         PR ld/23499:
>         * elf.c (_bfd_elf_get_symbol_version_string): Return
>         _("<corrupt>") for corrupt symbol version info.
>
> binutils/
>
>         PR ld/23499:
>         * readelf.c (get_symbol_version_string): Return _("<corrupt>")
>         for corrupt symbol version info.
> ---
>  bfd/elf.c          | 2 +-
>  binutils/readelf.c | 9 +++++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/bfd/elf.c b/bfd/elf.c
> index efdaf2e256..b8860c453b 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -1877,7 +1877,7 @@ _bfd_elf_get_symbol_version_string (bfd *abfd, asymbol *symbol,
>         {
>           Elf_Internal_Verneed *t;
>
> -         version_string = "";
> +         version_string = _("<corrupt>");
>           for (t = elf_tdata (abfd)->verref;
>                t != NULL;
>                t = t->vn_nextref)
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index 3b5eb4038b..a8ddf02c45 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -11295,6 +11295,7 @@ get_symbol_version_string (Filedata *                   filedata,
>    unsigned char data[2];
>    unsigned short vers_data;
>    unsigned long offset;
> +  unsigned short max_vd_ndx;
>
>    if (!is_dynsym
>        || version_info[DT_VERSIONTAGIDX (DT_VERSYM)] == 0)
> @@ -11312,6 +11313,8 @@ get_symbol_version_string (Filedata *                   filedata,
>    if ((vers_data & VERSYM_HIDDEN) == 0 && vers_data == 0)
>      return NULL;
>
> +  max_vd_ndx = 0;
> +
>    /* Usually we'd only see verdef for defined symbols, and verneed for
>       undefined symbols.  However, symbols defined by the linker in
>       .dynbss for variables copied from a shared library in order to
> @@ -11354,6 +11357,9 @@ get_symbol_version_string (Filedata *                   filedata,
>               ivd.vd_flags = BYTE_GET (evd.vd_flags);
>             }
>
> +         if ((ivd.vd_ndx & VERSYM_VERSION) > max_vd_ndx)
> +           max_vd_ndx = ivd.vd_ndx & VERSYM_VERSION;
> +
>           off += ivd.vd_next;
>         }
>        while (ivd.vd_ndx != (vers_data & VERSYM_VERSION) && ivd.vd_next != 0);
> @@ -11445,6 +11451,9 @@ get_symbol_version_string (Filedata *                   filedata,
>           return (ivna.vna_name < strtab_size
>                   ? strtab + ivna.vna_name : _("<corrupt>"));
>         }
> +      else if ((max_vd_ndx || (vers_data & VERSYM_VERSION) != 1)
> +              && (vers_data & VERSYM_VERSION) > max_vd_ndx)
> +       return _("<corrupt>");
>      }
>    return NULL;
>  }
> --
> 2.17.1
>



-- 
H.J.