public inbox for binutils-cvs@sourceware.org
 help / color / mirror / Atom feed
* [binutils-gdb] PR30326, uninitialised value in objdump compare_relocs
@ 2023-04-12  5:11 Alan Modra
  0 siblings, 0 replies; only message in thread
From: Alan Modra @ 2023-04-12  5:11 UTC (permalink / raw)
  To: bfd-cvs

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=93c6e8c3c14bf81020ca7571fe752250a34f5bc9

commit 93c6e8c3c14bf81020ca7571fe752250a34f5bc9
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Apr 12 11:00:42 2023 +0930

    PR30326, uninitialised value in objdump compare_relocs
    
    This is a fuzzing PR, with a testcase involving a SHF_ALLOC and
    SHF_COMPRESSED SHT_RELA section, ie. a compressed dynamic reloc
    section.  BFD doesn't handle compressed relocation sections, with most
    of the code reading relocs using sh_size (often no bfd section is
    created) but in the case of SHF_ALLOC dynamic relocs we had some code
    using the bfd section size.  This led to a mismatch, sh_size is
    compressed, size is uncompressed, and from that some uninitialised
    memory.  Consistently using sh_size is enough to fix this PR, but I've
    also added tests to exclude SHF_COMPRESSED reloc sections from
    consideration.
    
            PR 30362
            * elf.c (bfd_section_from_shdr): Exclude reloc sections with
            SHF_COMPRESSED flag from normal reloc processing.
            (_bfd_elf_get_dynamic_reloc_upper_bound): Similarly exclude
            SHF_COMPRESSED sections from consideration.  Use sh_size when
            sizing to match slurp_relocs.
            (_bfd_elf_canonicalize_dynamic_reloc): Likewise.
            (_bfd_elf_get_synthetic_symtab): Use NUM_SHDR_ENTRIES to size
            plt relocs.
            * elf32-arm.c (elf32_arm_get_synthetic_symtab): Likewise.
            * elf32-ppc.c (ppc_elf_get_synthetic_symtab): Likewise.
            * elf64-ppc.c (ppc64_elf_get_synthetic_symtab): Likewise.
            * elfxx-mips.c (_bfd_mips_elf_get_synthetic_symtab): Likewise.

Diff:
---
 bfd/elf.c        | 17 ++++++++++-------
 bfd/elf32-arm.c  |  2 +-
 bfd/elf32-ppc.c  |  2 +-
 bfd/elf64-ppc.c  |  2 +-
 bfd/elfxx-mips.c |  2 +-
 5 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 0e2ae6dae1c..fa7c25ad9dc 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -2381,6 +2381,7 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex)
 	   its sh_link points to the null section.  */
 	if (((abfd->flags & (DYNAMIC | EXEC_P)) != 0
 	     && (hdr->sh_flags & SHF_ALLOC) != 0)
+	    || (hdr->sh_flags & SHF_COMPRESSED) != 0
 	    || hdr->sh_type == SHT_RELR
 	    || hdr->sh_link == SHN_UNDEF
 	    || hdr->sh_link != elf_onesymtab (abfd)
@@ -8728,15 +8729,16 @@ _bfd_elf_get_dynamic_reloc_upper_bound (bfd *abfd)
   for (s = abfd->sections; s != NULL; s = s->next)
     if (elf_section_data (s)->this_hdr.sh_link == elf_dynsymtab (abfd)
 	&& (elf_section_data (s)->this_hdr.sh_type == SHT_REL
-	    || elf_section_data (s)->this_hdr.sh_type == SHT_RELA))
+	    || elf_section_data (s)->this_hdr.sh_type == SHT_RELA)
+	&& (elf_section_data (s)->this_hdr.sh_flags & SHF_COMPRESSED) == 0)
       {
-	ext_rel_size += s->size;
-	if (ext_rel_size < s->size)
+	ext_rel_size += elf_section_data (s)->this_hdr.sh_size;
+	if (ext_rel_size < elf_section_data (s)->this_hdr.sh_size)
 	  {
 	    bfd_set_error (bfd_error_file_truncated);
 	    return -1;
 	  }
-	count += s->size / elf_section_data (s)->this_hdr.sh_entsize;
+	count += NUM_SHDR_ENTRIES (&elf_section_data (s)->this_hdr);
 	if (count > LONG_MAX / sizeof (arelent *))
 	  {
 	    bfd_set_error (bfd_error_file_too_big);
@@ -8785,14 +8787,15 @@ _bfd_elf_canonicalize_dynamic_reloc (bfd *abfd,
     {
       if (elf_section_data (s)->this_hdr.sh_link == elf_dynsymtab (abfd)
 	  && (elf_section_data (s)->this_hdr.sh_type == SHT_REL
-	      || elf_section_data (s)->this_hdr.sh_type == SHT_RELA))
+	      || elf_section_data (s)->this_hdr.sh_type == SHT_RELA)
+	  && (elf_section_data (s)->this_hdr.sh_flags & SHF_COMPRESSED) == 0)
 	{
 	  arelent *p;
 	  long count, i;
 
 	  if (! (*slurp_relocs) (abfd, s, syms, true))
 	    return -1;
-	  count = s->size / elf_section_data (s)->this_hdr.sh_entsize;
+	  count = NUM_SHDR_ENTRIES (&elf_section_data (s)->this_hdr);
 	  p = s->relocation;
 	  for (i = 0; i < count; i++)
 	    *storage++ = p++;
@@ -12936,7 +12939,7 @@ _bfd_elf_get_synthetic_symtab (bfd *abfd,
   if (! (*slurp_relocs) (abfd, relplt, dynsyms, true))
     return -1;
 
-  count = relplt->size / hdr->sh_entsize;
+  count = NUM_SHDR_ENTRIES (hdr);
   size = count * sizeof (asymbol);
   p = relplt->relocation;
   for (i = 0; i < count; i++, p += bed->s->int_rels_per_ext_rel)
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index e07e12226a5..70413668e5a 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -20067,7 +20067,7 @@ elf32_arm_get_synthetic_symtab (bfd *abfd,
       plt->flags |= SEC_IN_MEMORY;
     }
 
-  count = relplt->size / hdr->sh_entsize;
+  count = NUM_SHDR_ENTRIES (hdr);
   size = count * sizeof (asymbol);
   p = relplt->relocation;
   for (i = 0; i < count; i++, p += elf32_arm_size_info.int_rels_per_ext_rel)
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index bb77ba2d5c7..2cff158a5f5 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -1920,7 +1920,7 @@ ppc_elf_get_synthetic_symtab (bfd *abfd, long symcount, asymbol **syms,
 	    }
     }
 
-  count = relplt->size / sizeof (Elf32_External_Rela);
+  count = NUM_SHDR_ENTRIES (&elf_section_data (relplt)->this_hdr);
   /* If the stubs are those for -shared/-pie then we might have
      multiple stubs for each plt entry.  If that is the case then
      there is no way to associate stubs with their plt entries short
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 069bd758aec..daa6deef728 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -2576,7 +2576,7 @@ ppc64_elf_get_synthetic_symtab (bfd *abfd,
 	      if (!(*slurp_relocs) (abfd, relplt, dyn_syms, true))
 		goto free_contents_and_exit_err;
 
-	      plt_count = relplt->size / sizeof (Elf64_External_Rela);
+	      plt_count = NUM_SHDR_ENTRIES (&elf_section_data (relplt)->this_hdr);
 	      size += plt_count * sizeof (asymbol);
 
 	      p = relplt->relocation;
diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
index d34a755807b..751deede887 100644
--- a/bfd/elfxx-mips.c
+++ b/bfd/elfxx-mips.c
@@ -16595,7 +16595,7 @@ _bfd_mips_elf_get_synthetic_symtab (bfd *abfd,
   /* Calculating the exact amount of space required for symbols would
      require two passes over the PLT, so just pessimise assuming two
      PLT slots per relocation.  */
-  count = relplt->size / hdr->sh_entsize;
+  count = NUM_SHDR_ENTRIES (hdr);
   counti = count * bed->s->int_rels_per_ext_rel;
   size = 2 * count * sizeof (asymbol);
   size += count * (sizeof (mipssuffix) +

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-04-12  5:11 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-12  5:11 [binutils-gdb] PR30326, uninitialised value in objdump compare_relocs Alan Modra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).