This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: [PATCH] ld (bfd) arm32 elf invalid strings offset in .strtab
- From: Tamar Christina <Tamar dot Christina at arm dot com>
- To: "tlaronde at polynum dot com" <tlaronde at polynum dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Cc: nd <nd at arm dot com>
- Date: Mon, 19 Aug 2019 14:30:01 +0000
- Subject: RE: [PATCH] ld (bfd) arm32 elf invalid strings offset in .strtab
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Sgz1MYACJsEkvJR12TgHcRj7wGMTTmVpPvBP63wCSfk=; b=M8bVMS36yu5End/Dl2IYgesZuVyeuuodKYz+V20/EGhx13AOMiMUxesvjf5+3pMSPoBXOZMbwl7rESWkPSMkQfjNtn4bzYhNHD2nC/q2N34kuTMN+l3GgX87d126B2WTy0wPNWZLkz+UgXdDAM134tcqivNpsgK8gLiYqEvIibTiBmpNgVuKQV1RS9S1GxZn9/M+9UUV1viSPftTppQzR6/b/I4lpYtoLYOwpVM7lmjOGhJl1kjp2uir6ntoP4/iYOEWnxjx4Mxfq6v1MxJb2U+3VGWGU/dhCJMAkMLw/qjqnWo6FFLCQz6XqkBT9LS3PmKOW84VetGt0uPYXRCTEw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kB3bztAC00wXWh7UvHs7P6/4SAs/0T8GHuKhfn/I6zxydreu9/gfbOu/GJv8QroMoBI4xa+QYiKftUPyowLM+49L8ys1u9ypQzqIiaCWbfdHzYnN+8QlerYv21Pg6mVjfD/KIm0tONW+hj0xhm7IWxWPSaDriLvM3Ds1siOMJWdKvjZB+UUWNJ33mEVWaOPXkAqceTs1gdU8gSh1yK39G6SXUXL/7zzxfTVXkiOhcjiMansAiWJRS3RoIhIlEVHihBzQoYkb7F5MTMh4DCo/TEaq2j4KFj1HjE0jHrqMRMfOXnC61IB2x3SQuxbssJpww94kxh+z9m4weSANHKF97g==
- Original-authentication-results: spf=none (sender IP is ) smtp.mailfrom=Tamar dot Christina at arm dot com;
- References: <20190816080809.GA645@polynum.com>
Hi Thierry,
Before I take a look at the patch I was wondering if you already have a copyright assignment with the FSF?
Kind Regards,
Tamar
> -----Original Message-----
> From: binutils-owner@sourceware.org <binutils-owner@sourceware.org>
> On Behalf Of tlaronde@polynum.com
> Sent: Friday, August 16, 2019 09:08
> To: binutils@sourceware.org
> Subject: [PATCH] ld (bfd) arm32 elf invalid strings offset in .strtab
>
> Hello,
>
> Note:
> The bug was investigated on NetBSD due to a Problem Report open.
> Hence the PR54159 mentionned below and some precisions added for not
> binutils developers.
>
>
> Description:
> The BFD GNU binutils linker when invoked for the arm32 elf target
> with a dshared linked object sometimes complains about string offsets
> in .strtab that are past the end of the section.
>
> How to reproduce:
> CC and LD have to be set to the arm32_elf programs (native or
> hosted).
>
> echo "#include <stdio.h>" >PR54159.h
> cat /dev/null >PR54159.exp
> for module in a b c d; do
> cat <<EOT >$module.c
> #include "PR54159.h"
> void
> $module(void)
> {
> printf("Hello from $module.\n");
> }
> EOT
> echo "extern void $module(void);" >>PR54159.h
> echo "$module" >>PR54159.exp
> done
>
> for module in a b c d; do
> $CC -I. -fPIC -c -o $module.o $module.c done
>
> $LD -shared -soname libPR54159.so.0\
> -retain-symbols-file ./PR54159.exp\
> -o libPR54159.so *.o
>
> Then invoking $LD on libPR54159.so will emit the complaints.
>
> Source of the bug:
> The bug was introduced in 2016-08-04:
> (elf32_arm_swap_symbol_in): Add detection code for CMSE
> special
> symbols.
>
> The problem is that bfd_elf_sym_name() is invoked inside of
> elf32_arm_swap_symbol_in() but this latter has no information,
> amongst its parameters, about the correct strings section associated
> with a symbol entry (while its caller have).
>
> So bfd_elf_sym_name() was called passing the standard symtab_hdr
> that is .strtab, which is wrong when the symbols are the dynamic
> ones whose offsets are relative to .dynstr.
>
> Hence, the code verifying that the offset was not past the end of
> the section detected that something was wrong only when
> the .strtab
> happened to be shortest than the offsets verified.
>
> So the bug was a permanent one but only seen when .strtab was
> shorten, which was the case when -retain-symbols-file was used to
> reduce the list of visible symbols.
>
> NOTE: the BFD code limits the size of the strings sections (all) by
> only emitting the longest strings but not adding strings that are
> suffixes of a longer one. Hence, names like "abcd", "bcd", "cd", "d"
> will produce only one entry in a string section: "abcd" since all
> the others are suffixes.
> For ARM, it can be misleading to see that for example .strtab has
> only "$a", "$d", "c" , "b" (in the test case above) "missing" symbol
> names "a" and "d". But they are not missing: they are suffix of
> resp. "$a" and "$d". No problem here.
>
> Machines concerned:
> Only arm32, elf. This renders debugging difficult and could eventually
> produce incorrect shared libraries or executable when there are thumbs and
> when adding a new module since names could match (because the wrong
> strings are used) when they should not.
>
> Solutions discarded:
> - Putting the chunk of code in the caller, that has the information
> about the correct string table. But the caller is a generic elf.c
> function and it would introduce target specific code ruining the
> whole purpose of the BFD abstraction;
>
> - Adding another function pointer in the BFD xvec so that no-ops
> (for now) for other backends are called and a dedicated function for
> arm32, with the correct information amongst the parameters, doing
> the job. But it will have to be called just after swap_symbol_in(),
> that is linking this "generic" function to this and this is not
> general at all. And if it is linked, it better stays inside
> swap_symbol_in();
>
> - Trying to refactor elf32-arm.c so that the processing is done
> inside the function that has directly or indirectly the information.
> But I have not at the moment a sufficient knowledge neither of BDF
> or of low level ARM to guess what amount of time will be necessary
> to achieve it (and ENOTIME) and it might solve one bug but open a
> whole can of worms elsewhere, delaying 9.0 release.
>
> Solution adopted:
> The simplest: since the caller of swap_symbol_in() has the
> information, I added a supplementary parameter to
> swap_symbol_in() to pass the correct symtab_hdr pointer,
> modified the calls and added ATTRIBUTE_UNUSED in the
> implementations that don't use it.
>
> Pros: it is the simplest and should not alter other targets so not
> introduce bugs elsewhere. It solves the problem without touching
> anything else in arm32_elf either. It could be argued that this
> supplementary parameter could be used for better diagnosing
> purposes
> (ability to print associated name in the swap_symbol_in()
> implementations).
>
> Cons: the function has, normally, nothing to do with names.
> Some targets now pass NULL instead of correct value for
> this supplementary parameter---because in some contexts,
> the value is not available---which is harmless as long as
> somebody doesn't try to add code in the generic versions that
> assume the value is always defined in every context.
>
> Regards,
> --
> Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
> http://www.kergis.com/
> http://www.sbfa.fr/ Key fingerprint = 0FF7 E906 FBAF FE95 FD89
> 250D 52B1 AE95 6006 F40C
>
> --fdj2RfSjLxBAspz7
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: attachment; filename="binutils.diff"
>
> Index: external/gpl3/binutils/dist/bfd/elf-bfd.h
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf-bfd.h,v
> retrieving revision 1.9
> diff -u -r1.9 elf-bfd.h
> --- external/gpl3/binutils/dist/bfd/elf-bfd.h 7 Nov 2018 01:13:52 -0000
> 1.9
> +++ external/gpl3/binutils/dist/bfd/elf-bfd.h 15 Aug 2019 07:43:11 -0000
> @@ -712,7 +712,7 @@
> void (*write_relocs)
> (bfd *, asection *, void *);
> bfd_boolean (*swap_symbol_in)
> - (bfd *, const void *, const void *, Elf_Internal_Sym *);
> + (bfd *, const void *, const void *, Elf_Internal_Sym *,
> + Elf_Internal_Shdr *);
> void (*swap_symbol_out)
> (bfd *, const Elf_Internal_Sym *, void *, void *);
> bfd_boolean (*slurp_reloc_table)
> @@ -2348,7 +2348,7 @@
> (bfd *);
>
> extern bfd_boolean bfd_elf32_swap_symbol_in
> - (bfd *, const void *, const void *, Elf_Internal_Sym *);
> + (bfd *, const void *, const void *, Elf_Internal_Sym *,
> + Elf_Internal_Shdr *);
> extern void bfd_elf32_swap_symbol_out
> (bfd *, const Elf_Internal_Sym *, void *, void *); extern void
> bfd_elf32_swap_reloc_in @@ -2394,7 +2394,7 @@
> (bfd *);
>
> extern bfd_boolean bfd_elf64_swap_symbol_in
> - (bfd *, const void *, const void *, Elf_Internal_Sym *);
> + (bfd *, const void *, const void *, Elf_Internal_Sym *,
> + Elf_Internal_Shdr *);
> extern void bfd_elf64_swap_symbol_out
> (bfd *, const Elf_Internal_Sym *, void *, void *); extern void
> bfd_elf64_swap_reloc_in
> Index: external/gpl3/binutils/dist/bfd/elf.c
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf.c,v
> retrieving revision 1.13
> diff -u -r1.13 elf.c
> --- external/gpl3/binutils/dist/bfd/elf.c 7 Nov 2018 01:13:52 -0000 1.13
> +++ external/gpl3/binutils/dist/bfd/elf.c 15 Aug 2019 07:43:15 -0000
> @@ -491,7 +491,7 @@
> shndx = extshndx_buf;
> isym < isymend;
> esym += extsym_size, isym++, shndx = shndx != NULL ? shndx + 1 : NULL)
> - if (!(*bed->s->swap_symbol_in) (ibfd, esym, shndx, isym))
> + if (!(*bed->s->swap_symbol_in) (ibfd, esym, shndx, isym,
> + symtab_hdr))
> {
> symoffset += (esym - (bfd_byte *) extsym_buf) / extsym_size;
> /* xgettext:c-format */
> Index: external/gpl3/binutils/dist/bfd/elf32-arm.c
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf32-arm.c,v
> retrieving revision 1.14
> diff -u -r1.14 elf32-arm.c
> --- external/gpl3/binutils/dist/bfd/elf32-arm.c 5 May 2019 21:49:53 -0000
> 1.14
> +++ external/gpl3/binutils/dist/bfd/elf32-arm.c 15 Aug 2019 07:43:22 -
> 0000
> @@ -19626,12 +19626,12 @@
> elf32_arm_swap_symbol_in (bfd * abfd,
> const void *psrc,
> const void *pshn,
> - Elf_Internal_Sym *dst)
> + Elf_Internal_Sym *dst,
> + Elf_Internal_Shdr *symtab_hdr)
> {
> - Elf_Internal_Shdr *symtab_hdr;
> const char *name = NULL;
>
> - if (!bfd_elf32_swap_symbol_in (abfd, psrc, pshn, dst))
> + if (!bfd_elf32_swap_symbol_in (abfd, psrc, pshn, dst, symtab_hdr))
> return FALSE;
> dst->st_target_internal = 0;
>
> @@ -19660,7 +19660,6 @@
> ARM_SET_SYM_BRANCH_TYPE (dst->st_target_internal,
> ST_BRANCH_UNKNOWN);
>
> /* Mark CMSE special symbols. */
> - symtab_hdr = & elf_symtab_hdr (abfd);
> if (symtab_hdr->sh_size && dst->st_size != 0)
> name = bfd_elf_sym_name (abfd, symtab_hdr, dst, NULL);
> if (name && CONST_STRNEQ (name, CMSE_PREFIX))
> Index: external/gpl3/binutils/dist/bfd/elf32-i386.c
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf32-i386.c,v
> retrieving revision 1.11
> diff -u -r1.11 elf32-i386.c
> --- external/gpl3/binutils/dist/bfd/elf32-i386.c 7 Nov 2018 01:13:52 -0000
> 1.11
> +++ external/gpl3/binutils/dist/bfd/elf32-i386.c 15 Aug 2019 07:43:23 -
> 0000
> @@ -3949,7 +3949,7 @@
> if (!bed->s->swap_symbol_in (abfd,
> (htab->dynsym->contents
> + r_symndx * sizeof
> (Elf32_External_Sym)),
> - 0, &sym))
> + 0, &sym, NULL))
> abort ();
>
> if (ELF32_ST_TYPE (sym.st_info) == STT_GNU_IFUNC)
> Index: external/gpl3/binutils/dist/bfd/elf32-s390.c
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf32-s390.c,v
> retrieving revision 1.1.1.7
> diff -u -r1.1.1.7 elf32-s390.c
> --- external/gpl3/binutils/dist/bfd/elf32-s390.c 6 Nov 2018 21:18:42 -0000
> 1.1.1.7
> +++ external/gpl3/binutils/dist/bfd/elf32-s390.c 15 Aug 2019 07:43:24 -
> 0000
> @@ -3710,7 +3710,7 @@
> || !bed->s->swap_symbol_in (abfd,
> (htab->elf.dynsym->contents
> + r_symndx * bed->s->sizeof_sym),
> - 0, &sym))
> + 0, &sym, NULL))
> abort ();
>
> /* Check relocation against STT_GNU_IFUNC symbol. */
> Index: external/gpl3/binutils/dist/bfd/elf32-v850.c
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf32-v850.c,v
> retrieving revision 1.1.1.7
> diff -u -r1.1.1.7 elf32-v850.c
> --- external/gpl3/binutils/dist/bfd/elf32-v850.c 6 Nov 2018 21:18:35 -0000
> 1.1.1.7
> +++ external/gpl3/binutils/dist/bfd/elf32-v850.c 15 Aug 2019 07:43:26 -
> 0000
> @@ -3280,7 +3280,7 @@
> bfd_elf32_swap_symbol_in (abfd,
> extsyms + ELF32_R_SYM (irel->r_info),
> shndx ? shndx + ELF32_R_SYM (irel->r_info) :
> NULL,
> - & isym);
> + & isym, symtab_hdr);
>
> if (isym.st_shndx != sec_shndx)
> continue;
> @@ -3339,7 +3339,7 @@
> {
> Elf_Internal_Sym isym;
>
> - bfd_elf32_swap_symbol_in (abfd, esym, shndx, & isym);
> + bfd_elf32_swap_symbol_in (abfd, esym, shndx, & isym, symtab_hdr);
>
> if (isym.st_shndx == sec_shndx
> && isym.st_value >= addr + count
> @@ -3375,7 +3375,7 @@
> {
> Elf_Internal_Sym isym;
>
> - bfd_elf32_swap_symbol_in (abfd, esym, shndx, & isym);
> + bfd_elf32_swap_symbol_in (abfd, esym, shndx, & isym, symtab_hdr);
> sym_hash = elf_sym_hashes (abfd) [sym_index];
>
> if (isym.st_shndx == sec_shndx
> Index: external/gpl3/binutils/dist/bfd/elf64-s390.c
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf64-s390.c,v
> retrieving revision 1.1.1.7
> diff -u -r1.1.1.7 elf64-s390.c
> --- external/gpl3/binutils/dist/bfd/elf64-s390.c 6 Nov 2018 21:18:36 -0000
> 1.1.1.7
> +++ external/gpl3/binutils/dist/bfd/elf64-s390.c 15 Aug 2019 07:43:27 -
> 0000
> @@ -3507,7 +3507,7 @@
> || !bed->s->swap_symbol_in (abfd,
> (htab->elf.dynsym->contents
> + r_symndx * bed->s->sizeof_sym),
> - 0, &sym))
> + 0, &sym, NULL))
> abort ();
>
> /* Check relocation against STT_GNU_IFUNC symbol. */
> Index: external/gpl3/binutils/dist/bfd/elf64-x86-64.c
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf64-x86-64.c,v
> retrieving revision 1.9
> diff -u -r1.9 elf64-x86-64.c
> --- external/gpl3/binutils/dist/bfd/elf64-x86-64.c 7 Nov 2018 01:13:52 -
> 0000 1.9
> +++ external/gpl3/binutils/dist/bfd/elf64-x86-64.c 15 Aug 2019 07:43:29 -
> 0000
> @@ -4375,7 +4375,7 @@
> if (!bed->s->swap_symbol_in (abfd,
> (htab->elf.dynsym->contents
> + r_symndx * bed->s->sizeof_sym),
> - 0, &sym))
> + 0, &sym, NULL))
> abort ();
>
> if (ELF_ST_TYPE (sym.st_info) == STT_GNU_IFUNC)
> Index: external/gpl3/binutils/dist/bfd/elfcode.h
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elfcode.h,v
> retrieving revision 1.1.1.7
> diff -u -r1.1.1.7 elfcode.h
> --- external/gpl3/binutils/dist/bfd/elfcode.h 6 Nov 2018 21:18:37 -0000
> 1.1.1.7
> +++ external/gpl3/binutils/dist/bfd/elfcode.h 15 Aug 2019 07:43:29 -0000
> @@ -174,7 +174,8 @@
> elf_swap_symbol_in (bfd *abfd,
> const void *psrc,
> const void *pshn,
> - Elf_Internal_Sym *dst)
> + Elf_Internal_Sym *dst,
> + Elf_Internal_Shdr *symtab_hdr ATTRIBUTE_UNUSED)
> {
> const Elf_External_Sym *src = (const Elf_External_Sym *) psrc;
> const Elf_External_Sym_Shndx *shndx = (const Elf_External_Sym_Shndx *)
> pshn;
> Index: external/gpl3/binutils/dist/bfd/elfnn-aarch64.c
> ==========================================================
> =========
> RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elfnn-aarch64.c,v
> retrieving revision 1.1.1.5
> diff -u -r1.1.1.5 elfnn-aarch64.c
> --- external/gpl3/binutils/dist/bfd/elfnn-aarch64.c 6 Nov 2018 21:18:35 -
> 0000 1.1.1.5
> +++ external/gpl3/binutils/dist/bfd/elfnn-aarch64.c 15 Aug 2019 07:43:36 -
> 0000
> @@ -7822,7 +7822,7 @@
> if (!bed->s->swap_symbol_in (abfd,
> (htab->root.dynsym->contents
> + r_symndx * bed->s->sizeof_sym),
> - 0, &sym))
> + 0, &sym, NULL))
> {
> /* xgettext:c-format */
> _bfd_error_handler (_("%pB symbol number %lu references"
>
> --fdj2RfSjLxBAspz7--
> --
> Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
> http://www.kergis.com/
> http://www.sbfa.fr/ Key fingerprint = 0FF7 E906 FBAF FE95 FD89
> 250D 52B1 AE95 6006 F40C