Add even more checks for corrupt input when processing relocations for ELF files.

PR 32643

1 files changed, 106 insertions, 100 deletions

diff --git a/bfd/elflink.c b/bfd/elflink.c
index eafbd13..bf94094 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -96,15 +96,17 @@ _bfd_elf_link_keep_memory (struct bfd_link_info *info)
   return true;
 }
 
-struct elf_link_hash_entry *
-_bfd_elf_get_link_hash_entry (struct elf_link_hash_entry **  sym_hashes,
-			      unsigned int                   symndx,
-			      Elf_Internal_Shdr *            symtab_hdr)
+static struct elf_link_hash_entry *
+get_link_hash_entry (struct elf_link_hash_entry **  sym_hashes,
+		     unsigned int                   symndx,
+		     unsigned int                   ext_sym_start)
 {
-  if (symndx < symtab_hdr->sh_info)
+  if (sym_hashes == NULL
+      /* Guard against corrupt input.  See PR 32636 for an example.  */
+      || symndx < ext_sym_start)
     return NULL;
 
-  struct elf_link_hash_entry *h = sym_hashes[symndx - symtab_hdr->sh_info];
+  struct elf_link_hash_entry *h = sym_hashes[symndx - ext_sym_start];
 
   /* The hash might be empty.  See PR 32641 for an example of this.  */
   if (h == NULL)
@@ -117,27 +119,28 @@ _bfd_elf_get_link_hash_entry (struct elf_link_hash_entry **  sym_hashes,
   return h;
 }
 
-static struct elf_link_hash_entry *
-get_ext_sym_hash (struct elf_reloc_cookie *cookie, unsigned long r_symndx)
+struct elf_link_hash_entry *
+_bfd_elf_get_link_hash_entry (struct elf_link_hash_entry **  sym_hashes,
+			      unsigned int                   symndx,
+			      Elf_Internal_Shdr *            symtab_hdr)
 {
-  struct elf_link_hash_entry *h = NULL;
-
-  if ((r_symndx >= cookie->locsymcount
-       || ELF_ST_BIND (cookie->locsyms[r_symndx].st_info) != STB_LOCAL)
-      /* Guard against corrupt input.  See PR 32636 for an example.  */
-      && r_symndx >= cookie->extsymoff)
-    {
-      h = cookie->sym_hashes[r_symndx - cookie->extsymoff];
+  if (symtab_hdr == NULL)
+    return NULL;
 
-      if (h == NULL)
-	return NULL;
+  return get_link_hash_entry (sym_hashes, symndx, symtab_hdr->sh_info);
+}
 
-      while (h->root.type == bfd_link_hash_indirect
-	     || h->root.type == bfd_link_hash_warning)
-	h = (struct elf_link_hash_entry *) h->root.u.i.link;
-    }
+static struct elf_link_hash_entry *
+get_ext_sym_hash_from_cookie (struct elf_reloc_cookie *cookie, unsigned long r_symndx)
+{
+  if (cookie == NULL || cookie->sym_hashes == NULL)
+    return NULL;
+  
+  if (r_symndx >= cookie->locsymcount
+      || ELF_ST_BIND (cookie->locsyms[r_symndx].st_info) != STB_LOCAL)
+    return get_link_hash_entry (cookie->sym_hashes, r_symndx, cookie->extsymoff);
 
-  return h;
+  return NULL;
 }
 
 asection *
@@ -147,7 +150,7 @@ _bfd_elf_section_for_symbol (struct elf_reloc_cookie *cookie,
 {
   struct elf_link_hash_entry *h;
 
-  h = get_ext_sym_hash (cookie, r_symndx);
+  h = get_ext_sym_hash_from_cookie (cookie, r_symndx);
   
   if (h != NULL)
     {
@@ -9108,7 +9111,6 @@ set_symbol_value (bfd *bfd_with_globals,
 		  size_t symidx,
 		  bfd_vma val)
 {
-  struct elf_link_hash_entry **sym_hashes;
   struct elf_link_hash_entry *h;
   size_t extsymoff = locsymcount;
 
@@ -9131,12 +9133,12 @@ set_symbol_value (bfd *bfd_with_globals,
 
   /* It is a global symbol: set its link type
      to "defined" and give it a value.  */
-
-  sym_hashes = elf_sym_hashes (bfd_with_globals);
-  h = sym_hashes [symidx - extsymoff];
-  while (h->root.type == bfd_link_hash_indirect
-	 || h->root.type == bfd_link_hash_warning)
-    h = (struct elf_link_hash_entry *) h->root.u.i.link;
+  h = get_link_hash_entry (elf_sym_hashes (bfd_with_globals), symidx, extsymoff);
+  if (h == NULL)
+    {
+      /* FIXMEL What should we do ?  */
+      return;
+    }
   h->root.type = bfd_link_hash_defined;
   h->root.u.def.value = val;
   h->root.u.def.section = bfd_abs_section_ptr;
@@ -11614,10 +11616,19 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
 	      || (elf_bad_symtab (input_bfd)
 		  && flinfo->sections[symndx] == NULL))
 	    {
-	      struct elf_link_hash_entry *h = sym_hashes[symndx - extsymoff];
-	      while (h->root.type == bfd_link_hash_indirect
-		     || h->root.type == bfd_link_hash_warning)
-		h = (struct elf_link_hash_entry *) h->root.u.i.link;
+	      struct elf_link_hash_entry *h;
+
+	      h = get_link_hash_entry (sym_hashes, symndx, extsymoff);
+	      if (h == NULL)
+		{
+		  _bfd_error_handler
+		    /* xgettext:c-format */
+		    (_("error: %pB: unable to create group section symbol"),
+		     input_bfd);
+		  bfd_set_error (bfd_error_bad_value);
+		  return false;
+		}	      
+
 	      /* Arrange for symbol to be output.  */
 	      h->indx = -2;
 	      elf_section_data (osec)->this_hdr.sh_info = -2;
@@ -11752,7 +11763,7 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
 		  || (elf_bad_symtab (input_bfd)
 		      && flinfo->sections[r_symndx] == NULL))
 		{
-		  h = sym_hashes[r_symndx - extsymoff];
+		  h = get_link_hash_entry (sym_hashes, r_symndx, extsymoff);
 
 		  /* Badly formatted input files can contain relocs that
 		     reference non-existant symbols.  Check here so that
@@ -11761,17 +11772,13 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
 		    {
 		      _bfd_error_handler
 			/* xgettext:c-format */
-			(_("error: %pB contains a reloc (%#" PRIx64 ") for section %pA "
+			(_("error: %pB contains a reloc (%#" PRIx64 ") for section '%pA' "
 			   "that references a non-existent global symbol"),
 			 input_bfd, (uint64_t) rel->r_info, o);
 		      bfd_set_error (bfd_error_bad_value);
 		      return false;
 		    }
 
-		  while (h->root.type == bfd_link_hash_indirect
-			 || h->root.type == bfd_link_hash_warning)
-		    h = (struct elf_link_hash_entry *) h->root.u.i.link;
-
 		  s_type = h->type;
 
 		  /* If a plugin symbol is referenced from a non-IR file,
@@ -11987,7 +11994,6 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
 			  && flinfo->sections[r_symndx] == NULL))
 		    {
 		      struct elf_link_hash_entry *rh;
-		      unsigned long indx;
 
 		      /* This is a reloc against a global symbol.  We
 			 have not yet output all the local symbols, so
@@ -11996,15 +12002,16 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
 			 reloc to point to the global hash table entry
 			 for this symbol.  The symbol index is then
 			 set at the end of bfd_elf_final_link.  */
-		      indx = r_symndx - extsymoff;
-		      rh = elf_sym_hashes (input_bfd)[indx];
-		      while (rh->root.type == bfd_link_hash_indirect
-			     || rh->root.type == bfd_link_hash_warning)
-			rh = (struct elf_link_hash_entry *) rh->root.u.i.link;
-
-		      /* Setting the index to -2 tells
-			 elf_link_output_extsym that this symbol is
-			 used by a reloc.  */
+		      rh = get_link_hash_entry (elf_sym_hashes (input_bfd),
+						r_symndx, extsymoff);
+		      if (rh == NULL)
+			{
+			  /* FIXME: Generate an error ?  */
+			  continue;
+			}
+
+		      /* Setting the index to -2 tells elf_link_output_extsym
+			 that this symbol is used by a reloc.  */
 		      BFD_ASSERT (rh->indx < 0);
 		      rh->indx = -2;
 		      *rel_hash = rh;
@@ -13968,25 +13975,21 @@ _bfd_elf_gc_mark_hook (asection *sec,
 		       struct elf_link_hash_entry *h,
 		       Elf_Internal_Sym *sym)
 {
-  if (h != NULL)
+  if (h == NULL)
+    return bfd_section_from_elf_index (sec->owner, sym->st_shndx);
+
+  switch (h->root.type)
     {
-      switch (h->root.type)
-	{
-	case bfd_link_hash_defined:
-	case bfd_link_hash_defweak:
-	  return h->root.u.def.section;
+    case bfd_link_hash_defined:
+    case bfd_link_hash_defweak:
+      return h->root.u.def.section;
 
-	case bfd_link_hash_common:
-	  return h->root.u.c.p->section;
+    case bfd_link_hash_common:
+      return h->root.u.c.p->section;
 
-	default:
-	  break;
-	}
+    default:
+      return NULL;
     }
-  else
-    return bfd_section_from_elf_index (sec->owner, sym->st_shndx);
-
-  return NULL;
 }
 
 /* Return the debug definition section.  */
@@ -14035,46 +14038,49 @@ _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
   if (r_symndx == STN_UNDEF)
     return NULL;
 
-  h = get_ext_sym_hash (cookie, r_symndx);
-  
-  if (h != NULL)
+  h = get_ext_sym_hash_from_cookie (cookie, r_symndx);
+  if (h == NULL)
     {
-      bool was_marked;
+      /* A corrup tinput file can lead to a situation where the index
+	 does not reference either a local or an external symbol.  */
+      if (r_symndx >= cookie->locsymcount)
+	return NULL;
 
-      was_marked = h->mark;
-      h->mark = 1;
-      /* Keep all aliases of the symbol too.  If an object symbol
-	 needs to be copied into .dynbss then all of its aliases
-	 should be present as dynamic symbols, not just the one used
-	 on the copy relocation.  */
-      hw = h;
-      while (hw->is_weakalias)
-	{
-	  hw = hw->u.alias;
-	  hw->mark = 1;
-	}
+      return (*gc_mark_hook) (sec, info, cookie->rel, NULL,
+			      &cookie->locsyms[r_symndx]);
+    }
 
-      if (!was_marked && h->start_stop && !h->root.ldscript_def)
-	{
-	  if (info->start_stop_gc)
-	    return NULL;
+  bool was_marked = h->mark;
 
-	  /* To work around a glibc bug, mark XXX input sections
-	     when there is a reference to __start_XXX or __stop_XXX
-	     symbols.  */
-	  else if (start_stop != NULL)
-	    {
-	      asection *s = h->u2.start_stop_section;
-	      *start_stop = true;
-	      return s;
-	    }
-	}
+  h->mark = 1;
+  /* Keep all aliases of the symbol too.  If an object symbol
+     needs to be copied into .dynbss then all of its aliases
+     should be present as dynamic symbols, not just the one used
+     on the copy relocation.  */
+  hw = h;
+  while (hw->is_weakalias)
+    {
+      hw = hw->u.alias;
+      hw->mark = 1;
+    }
 
-      return (*gc_mark_hook) (sec, info, cookie->rel, h, NULL);
+  if (!was_marked && h->start_stop && !h->root.ldscript_def)
+    {
+      if (info->start_stop_gc)
+	return NULL;
+
+      /* To work around a glibc bug, mark XXX input sections
+	 when there is a reference to __start_XXX or __stop_XXX
+	 symbols.  */
+      else if (start_stop != NULL)
+	{
+	  asection *s = h->u2.start_stop_section;
+	  *start_stop = true;
+	  return s;
+	}
     }
 
-  return (*gc_mark_hook) (sec, info, cookie->rel, NULL,
-			  &cookie->locsyms[r_symndx]);
+  return (*gc_mark_hook) (sec, info, cookie->rel, h, NULL);
 }
 
 /* COOKIE->rel describes a relocation against section SEC, which is
@@ -15097,7 +15103,7 @@ bfd_elf_reloc_symbol_deleted_p (bfd_vma offset, void *cookie)
 
       struct elf_link_hash_entry *h;
 
-      h = get_ext_sym_hash (rcookie, r_symndx);
+      h = get_ext_sym_hash_from_cookie (rcookie, r_symndx);
       
       if (h != NULL)
 	{