readelf: memory leaks in process_dynamic_section

This fixes some code that assumed only one PT_LOAD would contain
DT_SYMTAB.  Which is normally the case, but fuzzers thoroughly mess
with object files.

	* readelf.c (get_num_dynamic_syms): Check for nbuckets and nchains
	non-zero.
	(process_dynamic_section): Call get_num_dynamic_syms once rather
	than in segment loop.  Break out of segment loop on a successful
	load of dynamic symbols.  Formatting.
	(process_object): Return error status from process_dynamic_section.

binutils/ChangeLog

@@ -1,3 +1,12 @@
+2020-04-24  Alan Modra  <amodra@gmail.com>
+
+	* readelf.c (get_num_dynamic_syms): Check for nbuckets and nchains
+	non-zero.
+	(process_dynamic_section): Call get_num_dynamic_syms once rather
+	than in segment loop.  Break out of segment loop on a successful
+	load of dynamic symbols.  Formatting.
+	(process_object): Return error status from process_dynamic_section.
+
 2020-04-23  Anton Kolesov  <anton.kolesov@synopsys.com>
 
 	* elf-bfd.h (elfcore_write_arc_v2): Add prototype.

binutils/readelf.c

@@ -9993,6 +9993,8 @@ get_num_dynamic_syms (Filedata * filedata)
       filedata->nbuckets = byte_get (nb, hash_ent_size);
       filedata->nchains = byte_get (nc, hash_ent_size);
 
+      if (filedata->nbuckets != 0 && filedata->nchains != 0)
+	{
 	  filedata->buckets = get_dynamic_data (filedata, filedata->nbuckets,
 						hash_ent_size);
 	  filedata->chains  = get_dynamic_data (filedata, filedata->nchains,
@@ -10000,7 +10002,7 @@ get_num_dynamic_syms (Filedata * filedata)
 
 	  if (filedata->buckets != NULL && filedata->chains != NULL)
 	    num_of_syms = filedata->nchains;
-
+	}
     no_hash:
       if (num_of_syms == 0)
 	{
@@ -10243,6 +10245,8 @@ process_dynamic_section (Filedata * filedata)
   /* Find the appropriate symbol table.  */
   if (filedata->dynamic_symbols == NULL || do_histogram)
     {
+      unsigned long num_of_syms;
+
       for (entry = filedata->dynamic_section;
 	   entry < filedata->dynamic_section + filedata->dynamic_nent;
 	   ++entry)
@@ -10262,7 +10266,11 @@ process_dynamic_section (Filedata * filedata)
 	    filedata->dynamic_info_DT_GNU_HASH = entry->d_un.d_val;
 	  }
 
-      if (filedata->dynamic_info[DT_SYMTAB]
+      num_of_syms = get_num_dynamic_syms (filedata);
+
+      if (num_of_syms != 0
+	  && filedata->dynamic_symbols == NULL
+	  && filedata->dynamic_info[DT_SYMTAB]
 	  && filedata->dynamic_info[DT_SYMENT])
 	{
 	  Elf_Internal_Phdr *seg;
@@ -10270,7 +10278,8 @@ process_dynamic_section (Filedata * filedata)
 
 	  if (! get_program_headers (filedata))
 	    {
-		error (_("Cannot interpret virtual addresses without program headers.\n"));
+	      error (_("Cannot interpret virtual addresses "
+		       "without program headers.\n"));
 	      return FALSE;
 	    }
 
@@ -10278,13 +10287,10 @@ process_dynamic_section (Filedata * filedata)
 	       seg < filedata->program_headers + filedata->file_header.e_phnum;
 	       ++seg)
 	    {
-		unsigned long num_of_syms;
-
 	      if (seg->p_type != PT_LOAD)
 		continue;
 
-		if ((seg->p_offset + seg->p_filesz)
+	      if (seg->p_offset + seg->p_filesz > filedata->file_size)
-		    > filedata->file_size)
 		{
 		  /* See PR 21379 for a reproducer.  */
 		  error (_("Invalid PT_LOAD entry\n"));
@@ -10292,9 +10298,7 @@ process_dynamic_section (Filedata * filedata)
 		}
 
 	      if (vma >= (seg->p_vaddr & -seg->p_align)
-		    && vma <= seg->p_vaddr + seg->p_filesz
+		  && vma < seg->p_vaddr + seg->p_filesz)
-		    && (num_of_syms = get_num_dynamic_syms (filedata)) != 0
-		    && filedata->dynamic_symbols == NULL)
 		{
 		  /* Since we do not know how big the symbol table is,
 		     we default to reading in up to the end of PT_LOAD
@@ -10316,6 +10320,7 @@ process_dynamic_section (Filedata * filedata)
 		      error (_("Corrupt DT_SYMTAB dynamic entry\n"));
 		      return FALSE;
 		    }
+		  break;
 		}
 	    }
 	}
@@ -10403,14 +10408,17 @@ process_dynamic_section (Filedata * filedata)
 	  filedata->dynamic_syminfo = (Elf_Internal_Syminfo *) malloc (syminsz);
 	  if (filedata->dynamic_syminfo == NULL)
 	    {
-	      error (_("Out of memory allocating %lu byte for dynamic symbol info\n"),
+	      error (_("Out of memory allocating %lu bytes "
+		       "for dynamic symbol info\n"),
 		     (unsigned long) syminsz);
 	      return FALSE;
 	    }
 
-	  filedata->dynamic_syminfo_nent = syminsz / sizeof (Elf_External_Syminfo);
+	  filedata->dynamic_syminfo_nent
+	    = syminsz / sizeof (Elf_External_Syminfo);
 	  for (syminfo = filedata->dynamic_syminfo, extsym = extsyminfo;
-	       syminfo < filedata->dynamic_syminfo + filedata->dynamic_syminfo_nent;
+	       syminfo < (filedata->dynamic_syminfo
+			  + filedata->dynamic_syminfo_nent);
 	       ++syminfo, ++extsym)
 	    {
 	      syminfo->si_boundto = BYTE_GET (extsym->si_boundto);
@@ -20120,7 +20128,7 @@ process_object (Filedata * filedata)
 {
   bfd_boolean  have_separate_files;
   unsigned int i;
-  bfd_boolean res = TRUE;
+  bfd_boolean res;
 
   if (! get_file_header (filedata))
     {
@@ -20176,10 +20184,9 @@ process_object (Filedata * filedata)
     /* Without loaded section groups we cannot process unwind.  */
     do_unwind = FALSE;
 
-  if (process_program_headers (filedata))
+  res = process_program_headers (filedata);
-    process_dynamic_section (filedata);
+  if (res)
-  else
+    res = process_dynamic_section (filedata);
-    res = FALSE;
 
   if (! process_relocs (filedata))
     res = FALSE;