Put bfd_section_from_shdr loop detection array in elf_tdata

The static variables used by bfd_section_from_shdr to detect loops
in ELF sections have a problem:  Comparing a BFD pointer doesn't
guarantee that the current bfd is the same as the one previously used
to allocate the sections_being_created array.  For example, doing
   size bad_elf_1 bad_elf_2
with two corrupted ELF files containing section loops will leave the
section_being_created array allocated for the first file and since
bfd_close is called for bad_elf_1 before bfd_elf_2 is opened, it is
possible that the BFD for the second file is allocated in the same
memory as the first file.  If bad_elf_2 has more sections than
bad_elf_1 then we might write beyond the end of the array.

So this patch implements the FIXME Nick put in a comment about
attaching the array to the BFD.

	* elf-bfd.h (struct elf_obj_tdata): Add being_created.
	* elf.c (bfd_section_from_shdr): Delete static vars for loop
	detection.  Use new tdata variable instead.
	* elfcode.h (elf_object_p): Allocate being_created.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 8be401858260ebd0cc92c46217614199bcae6512..e3a5c8a6eeb40ecac69b53936d5d706b0f4359ed 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,10 @@
+2020-12-14  Alan Modra  <amodra@gmail.com>
+
+       * elf-bfd.h (struct elf_obj_tdata): Add being_created.
+       * elf.c (bfd_section_from_shdr): Delete static vars for loop
+       detection.  Use new tdata variable instead.
+       * elfcode.h (elf_object_p): Allocate being_created.
+
 2020-12-10  Nelson Chu  <nelson.chu@sifive.com>
 
        * elfxx-riscv.c (riscv_ext_dont_care_version): New function.  Return

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index e9c890f6f16bb751304cacbde8d81d833172d2d0..53b5ffc33d5092046c34c7d1c559585a3fe28c76 100644 (file)
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1942,6 +1942,7 @@ struct elf_obj_tdata
   bfd_vma gp;                          /* The gp value */
   unsigned int gp_size;                        /* The gp size */
   unsigned int num_elf_sections;       /* elf_sect_ptr size */
+  unsigned char *being_created;
 
   /* A mapping from external symbols to entries in the linker hash
      table, used when linking.  This is indexed by the symbol index

diff --git a/bfd/elf.c b/bfd/elf.c
index 419c5f4420cc8f6d539523f354fda9397510d379..7f2237655d3e9c16fe158ad1bb2866de7dd0b7e2 100644 (file)
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -2045,49 +2045,20 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex)
   const struct elf_backend_data *bed;
   const char *name;
   bfd_boolean ret = TRUE;
-  static bfd_boolean * sections_being_created = NULL;
-  static bfd * sections_being_created_abfd = NULL;
-  static unsigned int nesting = 0;
 
   if (shindex >= elf_numsections (abfd))
     return FALSE;
 
-  if (++ nesting > 3)
+  /* PR17512: A corrupt ELF binary might contain a loop of sections via
+     sh_link or sh_info.  Detect this here, by refusing to load a
+     section that we are already in the process of loading.  */
+  if (elf_tdata (abfd)->being_created[shindex])
     {
-      /* PR17512: A corrupt ELF binary might contain a recursive group of
-        sections, with each the string indices pointing to the next in the
-        loop.  Detect this here, by refusing to load a section that we are
-        already in the process of loading.  We only trigger this test if
-        we have nested at least three sections deep as normal ELF binaries
-        can expect to recurse at least once.
-
-        FIXME: It would be better if this array was attached to the bfd,
-        rather than being held in a static pointer.  */
-
-      if (sections_being_created_abfd != abfd)
-       {
-         free (sections_being_created);
-         sections_being_created = NULL;
-       }
-      if (sections_being_created == NULL)
-       {
-         size_t amt = elf_numsections (abfd) * sizeof (bfd_boolean);
-
-         /* PR 26005: Do not use bfd_zalloc here as the memory might
-            be released before the bfd has been fully scanned.  */
-         sections_being_created = (bfd_boolean *) bfd_zmalloc (amt);
-         if (sections_being_created == NULL)
-           return FALSE;
-         sections_being_created_abfd = abfd;
-       }
-      if (sections_being_created [shindex])
-       {
-         _bfd_error_handler
-           (_("%pB: warning: loop in section dependencies detected"), abfd);
-         return FALSE;
-       }
-      sections_being_created [shindex] = TRUE;
+      _bfd_error_handler
+       (_("%pB: warning: loop in section dependencies detected"), abfd);
+      return FALSE;
     }
+  elf_tdata (abfd)->being_created[shindex] = TRUE;
 
   hdr = elf_elfsections (abfd)[shindex];
   ehdr = elf_elfheader (abfd);
@@ -2611,14 +2582,7 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex)
  fail:
   ret = FALSE;
  success:
-  if (sections_being_created && sections_being_created_abfd == abfd)
-    sections_being_created [shindex] = FALSE;
-  if (-- nesting == 0)
-    {
-      free (sections_being_created);
-      sections_being_created = NULL;
-      sections_being_created_abfd = NULL;
-    }
+  elf_tdata (abfd)->being_created[shindex] = FALSE;
   return ret;
 }
 

diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index c7da8f6c07424ec1f067af620f5b2ae3f20873d4..072220e220adc6fc94cc47e2be4ed6e25757207d 100644 (file)
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -704,6 +704,9 @@ elf_object_p (bfd *abfd)
       elf_elfsections (abfd) = (Elf_Internal_Shdr **) bfd_alloc (abfd, amt);
       if (!elf_elfsections (abfd))
        goto got_no_match;
+      elf_tdata (abfd)->being_created = bfd_zalloc (abfd, num_sec);
+      if (!elf_tdata (abfd)->being_created)
+       goto got_no_match;
 
       memcpy (i_shdrp, &i_shdr, sizeof (*i_shdrp));
       for (shdrp = i_shdrp, shindex = 0; shindex < num_sec; shindex++)