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.

4 files changed, 20 insertions, 45 deletions

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 8be4018..e3a5c8a 100644
--- 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 e9c890f..53b5ffc 100644
--- 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 419c5f4..7f22376 100644
--- 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 c7da8f6..072220e 100644
--- 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++)