This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH] MIPS/readelf: Simplify GOT[1] data availability check
- From: "Maciej W. Rozycki" <macro at imgtec dot com>
- To: Nick Clifton <nickc at redhat dot com>, <binutils at sourceware dot org>
- Date: Wed, 12 Apr 2017 00:03:41 +0100
- Subject: [PATCH] MIPS/readelf: Simplify GOT[1] data availability check
- Authentication-results: sourceware.org; auth=none
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");