Re: readelf reporting of e_shstrndx is slightly wrong
On Tue, Aug 21, 2018 at 11:09:04PM +, Mike Murphy wrote:
> I think an example would help explain this. Below is part of the
> output from readelf -h on an elf object I have which has 210016
> sections, but puts the section header string table at section 1. So
> e_shstrndx is 1, but e_shnum is 0. My reading of the elf standard
> is that this is legal,
I agree.
> but readelf complains due to the check
> header->e_shstrndx >= header->e_shnum:
Oh, right, header->e_shnum hasn't been updated to the actual number of
sections at that point. It's still zero.
> Size of this header: 64 (bytes)
> Size of program headers: 56 (bytes)
> Number of program headers: 0
> Size of section headers: 64 (bytes)
> Number of section headers: 0 (210016)
> Section header string table index: 1
>
> The code in readelf seems to assume that if there are > 0xff00
> sections then shstrndx will be one of those sections that are
> > 0xff00.
Yes, the range checking is wrong too, even if header->e_shnum had been
updated to the value retrieved from section_header[0].sh_size. It
doesn't range check a value from section_header[0].sh_link.
* readelf.c (process_file_header): Assign updated values from
section_header[0] fields to e_phnum, e_shnum and e_shstrndx
during printing of header. Correct e_shstrndx range check.
Remove unnecessary casts and use %u rather than %ld for
unsigned int header fields. Don't print a random %lx when
reporting an unknown EI_VERSION.
diff --git a/binutils/readelf.c b/binutils/readelf.c
index d330484663..4627da20ab 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -4765,12 +4765,12 @@ process_file_header (Filedata * filedata)
get_elf_class (header->e_ident[EI_CLASS]));
printf (_(" Data: %s\n"),
get_data_encoding (header->e_ident[EI_DATA]));
- printf (_(" Version: %d %s\n"),
+ printf (_(" Version: %d%s\n"),
header->e_ident[EI_VERSION],
(header->e_ident[EI_VERSION] == EV_CURRENT
- ? "(current)"
+ ? _(" (current)")
: (header->e_ident[EI_VERSION] != EV_NONE
- ? _("")
+ ? _(" ")
: "")));
printf (_(" OS/ABI:%s\n"),
get_osabi_name (filedata, header->e_ident[EI_OSABI]));
@@ -4781,45 +4781,57 @@ process_file_header (Filedata * filedata)
printf (_(" Machine: %s\n"),
get_machine_name (header->e_machine));
printf (_(" Version: 0x%lx\n"),
- (unsigned long) header->e_version);
+ header->e_version);
printf (_(" Entry point address: "));
- print_vma ((bfd_vma) header->e_entry, PREFIX_HEX);
+ print_vma (header->e_entry, PREFIX_HEX);
printf (_("\n Start of program headers: "));
- print_vma ((bfd_vma) header->e_phoff, DEC);
+ print_vma (header->e_phoff, DEC);
printf (_(" (bytes into file)\n Start of section headers: "));
- print_vma ((bfd_vma) header->e_shoff, DEC);
+ print_vma (header->e_shoff, DEC);
printf (_(" (bytes into file)\n"));
printf (_(" Flags: 0x%lx%s\n"),
- (unsigned long) header->e_flags,
+ header->e_flags,
get_machine_flags (filedata, header->e_flags, header->e_machine));
- printf (_(" Size of this header: %ld (bytes)\n"),
- (long) header->e_ehsize);
- printf (_(" Size of program headers: %ld (bytes)\n"),
- (long) header->e_phentsize);
- printf (_(" Number of program headers: %ld"),
- (long) header->e_phnum);
+ printf (_(" Size of this header: %u (bytes)\n"),
+ header->e_ehsize);
+ printf (_(" Size of program headers: %u (bytes)\n"),
+ header->e_phentsize);
+ printf (_(" Number of program headers: %u"),
+ header->e_phnum);
if (filedata->section_headers != NULL
&& header->e_phnum == PN_XNUM
&& filedata->section_headers[0].sh_info != 0)
- printf (" (%ld)", (long) filedata->section_headers[0].sh_info);
+ {
+ header->e_phnum = filedata->section_headers[0].sh_info;
+ printf (" (%u)", header->e_phnum);
+ }
putc ('\n', stdout);
- printf (_(" Size of section headers: %ld (bytes)\n"),
- (long) header->e_shentsize);
- printf (_(" Number of section headers: %ld"),
- (long) header->e_shnum);
+ printf (_(" Size of section headers: %u (bytes)\n"),
+ header->e_shentsize);
+ printf (_(" Number of section headers:
RE: readelf reporting of e_shstrndx is slightly wrong
I think an example would help explain this. Below is part of the output from
readelf -h on an elf object I have which has 210016 sections, but puts the
section header string table at section 1. So e_shstrndx is 1, but e_shnum is
0. My reading of the elf standard is that this is legal, but readelf complains
due to the check header->e_shstrndx >= header->e_shnum:
Size of this header: 64 (bytes)
Size of program headers: 56 (bytes)
Number of program headers: 0
Size of section headers: 64 (bytes)
Number of section headers: 0 (210016)
Section header string table index: 1
The code in readelf seems to assume that if there are > 0xff00 sections then
shstrndx will be one of those sections that are > 0xff00.
-Original Message-
From: Nick Clifton
Sent: Tuesday, August 21, 2018 8:32 AM
To: Mike Murphy ; bug-binutils@gnu.org
Subject: Re: readelf reporting of e_shstrndx is slightly wrong
Hi Mike,
> If the file has no section name string table, this member holds the value
> |SHN_UNDEF|.
> If the section name string table section index is greater than or
> equal to |SHN_LORESERVE| (|0xff00|), this member has the value
> |SHN_XINDEX| (|0x|)
> The current readelf -h seems to assume that if there are more than 0xff00
> sections, then the shstrndx will also be past that. But there is nothing to
> prevent the section name string table from being section 1, in which case
> e_shstrndx should just be 1. But the readelf implementation has:
>
> else if (elf_header.e_shstrndx != SHN_UNDEF &&
> elf_header.e_shstrndx >= elf_header.e_shnum)
>
> printf(_(""));
I disagree. The readelf code actually looks like this:
if (filedata->section_headers != NULL
&& header->e_shstrndx == (SHN_XINDEX & 0x))
printf (" (%u)", filedata->section_headers[0].sh_link);
else if (header->e_shstrndx != SHN_UNDEF
&& header->e_shstrndx >= header->e_shnum)
printf (_(" "));
There is no check that the file itself has more than 0xff00 sections.
Instead it checks to see if the e_shstrndx field is SHN_XINDEX and if so it
follows the link. Otherwise it checks that the index is either SHN_UNDEF or a
valid section number.
Note - the use if "& 0x" in the above code is confusing, and looks surplus
to me, but I do not think that it makes any difference to the behaviour.
Cheers
Nick
___
bug-binutils mailing list
bug-binutils@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-binutils
Re: readelf reporting of e_shstrndx is slightly wrong
Hi,
On 08/21/2018 05:31 PM, Nick Clifton wrote:
if (filedata->section_headers != NULL
&& header->e_shstrndx == (SHN_XINDEX & 0x))
printf (" (%u)", filedata->section_headers[0].sh_link);
else if (header->e_shstrndx != SHN_UNDEF
&& header->e_shstrndx >= header->e_shnum)
printf (_(" "));
[...]
Note - the use if "& 0x" in the above code is confusing, and looks
surplus to me, but I do not think that it makes any difference to the
behaviour.
I haven't checked the location of this quoted code; however, the use of
"& 0x" is probably due to a clever hack made internally:
Elf[32|64]_External_Ehdr in include/elf/external.h declares e_shstrndx
as 2 bytes,
while Elf_Internal_Ehdr in include/elf/internal.h declares it as 4
bytes because we would like to be able to keep the overflowing values
within the same variable without extra checks.
We would like to be able to store values that are reserved or higher
than 0x as valid indices internally in the 4-byte e_shstrndx.
Therefore, some constants including SHN_XINDEX are re-defined in
elf/internal.h as 4 byte values. These redefinitions hold the new bounds
on such indices. Because of this internal simplification, though, we
need to use "& 0x" to get the 2-byte value that's used when reading
values in or outputting them.
Regards,
Egeyar
___
bug-binutils mailing list
bug-binutils@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-binutils
Re: readelf reporting of e_shstrndx is slightly wrong
Hi Mike,
> If the file has no section name string table, this member holds the value
> |SHN_UNDEF|.
> If the section name string table section index is greater than or equal to
> |SHN_LORESERVE| (|0xff00|), this member has the value |SHN_XINDEX| (|0x|)
> The current readelf -h seems to assume that if there are more than 0xff00
> sections, then the shstrndx will also be past that. But there is nothing to
> prevent the section name string table from being section 1, in which case
> e_shstrndx should just be 1. But the readelf implementation has:
>
> else if (elf_header.e_shstrndx != SHN_UNDEF &&
> elf_header.e_shstrndx >= elf_header.e_shnum)
>
> printf(_(“”));
I disagree. The readelf code actually looks like this:
if (filedata->section_headers != NULL
&& header->e_shstrndx == (SHN_XINDEX & 0x))
printf (" (%u)", filedata->section_headers[0].sh_link);
else if (header->e_shstrndx != SHN_UNDEF
&& header->e_shstrndx >= header->e_shnum)
printf (_(" "));
There is no check that the file itself has more than 0xff00 sections.
Instead it checks to see if the e_shstrndx field is SHN_XINDEX and if
so it follows the link. Otherwise it checks that the index is either
SHN_UNDEF or a valid section number.
Note - the use if "& 0x" in the above code is confusing, and looks
surplus to me, but I do not think that it makes any difference to the
behaviour.
Cheers
Nick
___
bug-binutils mailing list
bug-binutils@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-binutils
