[PATCH] MIPS/readelf: Simplify GOT[1] data availability check

[PATCH] MIPS/readelf: Simplify GOT[1] data availability check

[Maciej W. Rozycki]

Unavailable data is handled gracefully in MIPS GOT processing done by 
`print_mips_got_entry', so all that is needed in special GOT[1] handling 
is to verify whether data can be retrieved for the purpose of the GNU 
marker check done with `byte_get'.  Remove the extra error reporting 
code then, introduced with commit 75ec1fdbb797 ("Fix runtime seg-fault 
in readelf when parsing a corrupt MIPS binary.") in the course of 
addressing PR binutils/21344, and defer the error case to regular local
GOT entry processing.

	binutils/
	* readelf.c (process_mips_specific): Remove error reporting from
	GOT[1] processing.
---
 Verified not to crash with the PR binutils/21344 test case provided.

 This test case is actually interesting in that its GOT is consistent: the 
size is 8 and the ELF file is 64-bit, so the table simply consists of the 
GOT[0] entry, and GOT[1] is absent.  This is valid according to the MIPS 
psABI as only GOT[0] is required, and the special meaning of GOT[1] is 
merely a GNU extension.  So there should be no warning message produced, 
with GOT[1] interpreted as what would be the first local GOT entry if it 
was there.

 Output without the patch applied is as follows:

$ readelf -A 00258-binutils-readelf-heapoverflow2-byte_get_little_endian
readelf: Error: the PHDR segment is not covered by a LOAD segment

Primary GOT:
 Canonical gp value: 0000000000609ff0

 Reserved entries:
           Address     Access          Initial Purpose
  0000000000602000 -32752(gp) 0000000000601e28 Lazy resolver
readelf: Error: Invalid got entry - 0x602008 - overflows GOT table
$

updated hereby to:

$ readelf -A 00258-binutils-readelf-heapoverflow2-byte_get_little_endian
readelf: Error: the PHDR segment is not covered by a LOAD segment

Primary GOT:
 Canonical gp value: 0000000000609ff0

 Reserved entries:
           Address     Access          Initial Purpose
  0000000000602000 -32752(gp) 0000000000601e28 Lazy resolver

$

Should there be a partial GOT[1] entry, `print_mips_got_entry' would 
handle it gracefully, producing output like:

Primary GOT:
 Canonical gp value: 0000000000609ff0

 Reserved entries:
           Address     Access          Initial Purpose
  0000000000602000 -32752(gp) 0000000000601e28 Lazy resolver

 Local entries:
           Address     Access          Initial
  0000000000602008 -32744(gp) readelf: Warning: MIPS GOT entry extends beyond the end of available data
<corrupt>

so there's no need to bail out in `process_mips_specific'.

[NB this is output from stdout and stderr interspersed.  Output formatting 
for each of these streams remains consistent.  Maybe it could be improved, 
however the way how this code has been structured would make it a larger 
change to make, so I'll defer it to unspecified future.]

 I'll push this change shortly then unless someone objects.

  Maciej

binutils-mips-readelf-arch-got-data.diff
Index: binutils/binutils/readelf.c
===================================================================
--- binutils.orig/binutils/readelf.c	2017-04-10 23:37:11.221995243 +0100
+++ binutils/binutils/readelf.c	2017-04-10 23:42:33.459706758 +0100
@@ -15485,24 +15485,20 @@ process_mips_specific (FILE * file)
       if (ent == (bfd_vma) -1)
 	goto got_print_fail;
 
-      if (data)
+      /* Check for the MSB of GOT[1] being set, denoting a GNU object.
+	 This entry will be used by some runtime loaders, to store the
+	 module pointer.  Otherwise this is an ordinary local entry.
+	 PR 21344: Check for the entry being fully available before
+	 fetching it.  */
+      if (data
+	  && data + ent - pltgot + addr_size <= data_end
+	  && (byte_get (data + ent - pltgot, addr_size)
+	      >> (addr_size * 8 - 1)) != 0)
 	{
-	  /* PR 21344 */
-	  if (data + ent - pltgot > data_end - addr_size)
-	    {
-	      error (_("Invalid got entry - %#lx - overflows GOT table\n"),
-		     (long) ent);
-	      goto got_print_fail;
-	    }
-	  
-	  if (byte_get (data + ent - pltgot, addr_size)
-	      >> (addr_size * 8 - 1) != 0)
-	    {
-	      ent = print_mips_got_entry (data, pltgot, ent, data_end);
-	      printf (_(" Module pointer (GNU extension)\n"));
-	      if (ent == (bfd_vma) -1)
-		goto got_print_fail;
-	    }
+	  ent = print_mips_got_entry (data, pltgot, ent, data_end);
+	  printf (_(" Module pointer (GNU extension)\n"));
+	  if (ent == (bfd_vma) -1)
+	    goto got_print_fail;
 	}
       printf ("\n");
 

Re: [PATCH] MIPS/readelf: Simplify GOT[1] data availability check

[Nick Clifton]

Hi Maciej,

>  I'll push this change shortly then unless someone objects.

No objections - please apply.

Cheers
  Nick

Re: [PATCH] MIPS/readelf: Simplify GOT[1] data availability check

[Maciej W. Rozycki]

On Wed, 12 Apr 2017, Nick Clifton wrote:

> >  I'll push this change shortly then unless someone objects.
> 
> No objections - please apply.

 Committed.

  Maciej