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
Hello Tamar,
On Mon, Aug 19, 2019 at 02:30:01PM +0000, Tamar Christina wrote:
> Hi Thierry,
>
> Before I take a look at the patch I was wondering if you already have a copyright assignment with the FSF?
>
I have made one long years ago for contributions to GRUB. If this
is considered obsolete or if the assignment has to be made by package,
I have no problem making an assignment with the
FSF for contributions to binutils.
Best regards,
T. Laronde
> > -----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
--
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