Re: readelf: Save and dump the original section header values

This started as removing the imperative "Ignore the.." from
validate_section_info warnings, and making them a little less work for
translators.  I also decided it wasn't worth reporting specifics about
a malloc failure when allocating a relatively small fixed size buffer.
It would be different if some large chunk of file data caused a malloc
fail.

The patch also fixes a potential orig_section_headers buffer overflow,
caused by allocating a single element array when probing, hitting some
early error in process_file_header, then attempting to access it as an
array of e_shnum entries in free_filedata.

	* readelf.c (save_original_section_header_values): Use xmalloc
	rather than reporting a more specific error.  Drop now unused
	function parameters.
	(validate_section_info): Delete "dynamic" and "probe" params.
	Redo warning messages.  Don't save orig header for fake
	section header passing relr dynamic tags.  Avoid possible
	arithmetic overflow when checking sh_size.
	(get_32bit_section_headers): Don't allocate orig_section_headers
	or call validate_section_info when probing.
	(get_64bit_section_headers): Likewise.
	(process_relocs): Update validate_section_info call when
	handling relr.

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 0938fe64b225e11196e4d2a00db810454aa5e462..f18fbfbe8b9506e87284c053f99b5a786d481413 100644 (file)
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -7853,79 +7853,45 @@ offset_from_vma (Filedata * filedata, uint64_t vma, uint64_t size)
 
 static void
 save_original_section_header_values (Elf_Internal_Shdr *internal,
-                                    Elf_Internal_Shdr **orig_internal,
-                                    const char *dynamic_tag,
-                                    unsigned int i)
+                                    Elf_Internal_Shdr **orig_internal)
 {
-  /* Return if the original section header values have been saved.  */
+  /* Nothing to do if they have already been saved.  */
   if (*orig_internal != NULL)
     return;
 
-  *orig_internal = (Elf_Internal_Shdr *)
-    malloc (sizeof (Elf_Internal_Shdr));
-  if (*orig_internal == NULL)
-    {
-      if (dynamic_tag)
-       error (_("Out of memory reading dynamic tag %s\n"),
-              dynamic_tag);
-      else
-       error (_("Out of memory reading %u section headers\n"), i);
-      return;
-    }
-
-  /* Save the original section header values.  */
+  *orig_internal = xmalloc (sizeof (**orig_internal));
   **orig_internal = *internal;
 }
 
 
-/* Valid section info and clear the invalid fields.  */
+/* Warn about and clear any invalid ELF section fields.  */
 
 static void
 validate_section_info (Elf_Internal_Shdr *internal,
                       Elf_Internal_Shdr **orig_internal,
-                      unsigned int i, Filedata *filedata, bool dynamic,
-                      bool probe)
+                      unsigned int i, Filedata *filedata)
 {
-  const char *dynamic_tag = NULL;
-  const char *dynamicsz_tag = NULL;
-  const char *dynamicent_tag = NULL;
-  if (probe)
-    return;
-
-  if (dynamic)
-    switch (i)
-      {
-      case DT_RELR:
-       dynamic_tag = "DT_RELR";
-       dynamicsz_tag = "DT_RELRSZ";
-       dynamicent_tag = "DT_RELRENT";
-       break;
-
-      default:
-       abort ();
-      }
-  else
+  bool dynamic = orig_internal == NULL;
+  if (!dynamic)
     {
       if (internal->sh_link >= filedata->file_header.e_shnum
          && !special_defined_section_index (filedata,
                                             internal->sh_link))
        {
-         warn (_("Ignore the out of range sh_link value of %u for "
-                 "section %u\n"), internal->sh_link, i);
+         warn (_("out of range %s (%" PRIu64 ") for section %u\n"),
+               "sh_link", (uint64_t) internal->sh_link, i);
          /* Save the original section header values before garbage
             values are cleared.  */
-         save_original_section_header_values (internal, orig_internal,
-                                              NULL, i);
+         save_original_section_header_values (internal, orig_internal);
          internal->sh_link = 0;
        }
 
       if ((internal->sh_flags & SHF_INFO_LINK) != 0
          && internal->sh_info > filedata->file_header.e_shnum)
        {
-         warn (_("Ignore the out of range sh_info value of %u for "
-                 "section %u\n"), internal->sh_info, i);
-         save_original_section_header_values (internal, orig_internal,
-                                              NULL, i);
+         warn (_("out of range %s (%" PRIu64 ") for section %u\n"),
+               "sh_info", (uint64_t) internal->sh_info, i);
+         save_original_section_header_values (internal, orig_internal);
          internal->sh_info = 0;
        }
     }
@@ -7933,47 +7899,45 @@ validate_section_info (Elf_Internal_Shdr *internal,
   if (internal->sh_entsize > filedata->file_size)
     {
       if (dynamic)
-       warn (_("Ignore the out of range value of %" PRIu64 " for "
-               "dynamic tag %s\n"), (uint64_t) internal->sh_entsize,
-             dynamicent_tag);
+       warn (_("out of range %s (%" PRIu64 ")\n"),
+             "DT_RELRENT", (uint64_t) internal->sh_entsize);
       else
-       warn (_("Ignore the out of range sh_entsize value of %"
-               PRIu64 " for section %u\n"),
-             (uint64_t) internal->sh_entsize, i);
-      save_original_section_header_values (internal, orig_internal,
-                                          dynamicent_tag, i);
+       {
+         warn (_("out of range %s (%" PRIu64 ") for section %u\n"),
+               "sh_entsize", (uint64_t) internal->sh_entsize, i);
+         save_original_section_header_values (internal, orig_internal);
+       }
       internal->sh_entsize = 0;
     }
 
   if (internal->sh_type != SHT_NOBITS)
     {
-      int64_t sh_offset = internal->sh_offset;
-      if (sh_offset < 0 || (uint64_t) sh_offset > filedata->file_size)
+      uint64_t sh_offset = internal->sh_offset;
+      if (sh_offset > filedata->file_size)
        {
          if (dynamic)
-           warn (_("Ignore the out of range value of %" PRId64 " for "
-                   "dynamic tag %s\n"), sh_offset, dynamic_tag);
+           warn (_("out of range %s (%" PRIu64 ")\n"),
+                 "DT_RELR", sh_offset);
          else
-           warn (_("Ignore the out of range sh_offset value of %"
-                   PRId64 " for section %u\n"), sh_offset, i);
-         save_original_section_header_values (internal, orig_internal,
-                                              dynamic_tag, i);
+           {
+             warn (_("out of range %s (%" PRIu64 ") for section %u\n"),
+                   "sh_offset", sh_offset, i);
+             save_original_section_header_values (internal, orig_internal);
+           }
          internal->sh_offset = 0;
+         internal->sh_size = 0;
        }
-
-      if (sh_offset + internal->sh_size > filedata->file_size)
+      else if (internal->sh_size > filedata->file_size - sh_offset)
        {
          if (dynamic)
-           warn (_("Ignore the out of range value of %" PRIu64 " for "
-                   "dynamic tag %s\n"), (uint64_t) internal->sh_size,
-                 dynamicsz_tag);
+           warn (_("out of range %s (%" PRIu64 ")\n"),
+                 "DT_RELRSZ", (uint64_t) internal->sh_size);
          else
-           warn (_("Ignore the out of range sh_size value of %"
-                   PRIu64 " for section %u with sh_offset value of %"
-                   PRId64 "\n"), (uint64_t) internal->sh_size, i,
-                 sh_offset);
-         save_original_section_header_values (internal, orig_internal,
-                                              dynamicsz_tag, i);
+           {
+             warn (_("out of range %s (%" PRIu64 ") for section %u\n"),
+                   "sh_size", (uint64_t) internal->sh_size, i);
+             save_original_section_header_values (internal, orig_internal);
+           }
          internal->sh_size = 0;
        }
     }
@@ -8028,8 +7992,9 @@ get_32bit_section_headers (Filedata * filedata, bool probe)
       return false;
     }
 
-  filedata->orig_section_headers = (Elf_Internal_Shdr **)
-    xcalloc2 (num, sizeof (Elf_Internal_Shdr *));
+  if (!probe)
+    filedata->orig_section_headers = xcalloc2 (num,
+                                              sizeof (Elf_Internal_Shdr *));
 
   orig_internal = filedata->orig_section_headers;
   for (i = 0, internal = filedata->section_headers;
@@ -8046,8 +8011,8 @@ get_32bit_section_headers (Filedata * filedata, bool probe)
       internal->sh_info      = BYTE_GET (shdrs[i].sh_info);
       internal->sh_addralign = BYTE_GET (shdrs[i].sh_addralign);
       internal->sh_entsize   = BYTE_GET (shdrs[i].sh_entsize);
-      validate_section_info (internal, orig_internal, i, filedata,
-                            false, probe);
+      if (!probe)
+       validate_section_info (internal, orig_internal, i, filedata);
     }
 
   free (shdrs);
@@ -8103,8 +8068,9 @@ get_64bit_section_headers (Filedata * filedata, bool probe)
       return false;
     }
 
-  filedata->orig_section_headers = (Elf_Internal_Shdr **)
-    xcalloc2 (num, sizeof (Elf_Internal_Shdr *));
+  if (!probe)
+    filedata->orig_section_headers = xcalloc2 (num,
+                                              sizeof (Elf_Internal_Shdr *));
 
   orig_internal = filedata->orig_section_headers;
   for (i = 0, internal = filedata->section_headers;
@@ -8121,8 +8087,8 @@ get_64bit_section_headers (Filedata * filedata, bool probe)
       internal->sh_info      = BYTE_GET (shdrs[i].sh_info);
       internal->sh_offset    = BYTE_GET (shdrs[i].sh_offset);
       internal->sh_addralign = BYTE_GET (shdrs[i].sh_addralign);
-      validate_section_info (internal, orig_internal, i, filedata,
-                            false, probe);
+      if (!probe)
+       validate_section_info (internal, orig_internal, i, filedata);
     }
 
   free (shdrs);
@@ -10261,18 +10227,15 @@ process_relocs (Filedata * filedata)
                  uint64_t num_reloc;
                  uint64_t *relrs = NULL;
                  Elf_Internal_Shdr section = {};
-                 Elf_Internal_Shdr *orig_section = NULL;
                  section.sh_offset
                    = filedata->dynamic_info[DT_RELR];
                  section.sh_size = rel_size;
                  section.sh_entsize = rel_entsz;
                  section.sh_type = SHT_RELR;
-                 validate_section_info (&section, &orig_section, DT_RELR,
-                                        filedata, true, false);
+                 validate_section_info (&section, NULL, 0, filedata);
                  num_reloc = count_relr_relocations (filedata,
                                                      &section,
                                                      &relrs);
-                 free (orig_section);
                  free (relrs);
                  if (num_reloc == 0)
                    continue;