Re: readelf reporting of e_shstrndx is slightly wrong

[Alan Modra]

On Tue, Aug 21, 2018 at 11:09:04PM +0000, 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 <corrupt: out of range>
> 
> 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
-                 ? _("<unknown: %lx>")
+                 ? _(" <unknown>")
                  : "")));
       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:         %u"),
+             header->e_shnum);
       if (filedata->section_headers != NULL && header->e_shnum == SHN_UNDEF)
-       printf (" (%ld)", (long) filedata->section_headers[0].sh_size);
+       {
+         header->e_shnum = filedata->section_headers[0].sh_size;
+         printf (" (%u)", header->e_shnum);
+       }
       putc ('\n', stdout);
-      printf (_("  Section header string table index: %ld"),
-             (long) header->e_shstrndx);
+      printf (_("  Section header string table index: %u"),
+             header->e_shstrndx);
       if (filedata->section_headers != NULL
          && header->e_shstrndx == (SHN_XINDEX & 0xffff))
-       printf (" (%u)", filedata->section_headers[0].sh_link);
-      else if (header->e_shstrndx != SHN_UNDEF
-              && header->e_shstrndx >= header->e_shnum)
-       printf (_(" <corrupt: out of range>"));
+       {
+         header->e_shstrndx = filedata->section_headers[0].sh_link;
+         printf (" (%u)", header->e_shstrndx);
+       }
+      if (header->e_shstrndx != SHN_UNDEF
+         && header->e_shstrndx >= header->e_shnum)
+       {
+         header->e_shstrndx = SHN_UNDEF;
+         printf (_(" <corrupt: out of range>"));
+       }
       putc ('\n', stdout);
     }
 

-- 
Alan Modra
Australia Development Lab, IBM