strings: remove section/file size check
authorAlan Modra <amodra@gmail.com>
Mon, 3 Jul 2017 12:30:32 +0000 (22:00 +0930)
committerAlan Modra <amodra@gmail.com>
Mon, 3 Jul 2017 12:33:49 +0000 (22:03 +0930)
This reverts most of 06803313754, 2005-07-05 Dmitry V. Levin change
adding a check that section size doesn't exceed file size.  As we've
seen recently with mmo tests, decoded section size can easily exceed
file size with formats that encode section data.

I've also changed "strings" to use bfd_malloc_and_get_section, so that
"strings" won't die on a malloc failure.  I think it's better to
continue on looking at other sections after failing to dump a section
with fuzzed size.

The testcases at https://bugzilla.altlinux.org/show_bug.cgi?id=5871
on a 32-bit host now produce
$ strings -d --target=a.out-i386 /tmp/bfdkiller.dat
strings: error: /tmp/bfdkiller.dat(.text) is too large (0xffffffff bytes)
strings: /tmp/bfdkiller.dat: Reading section .text failed: Memory exhausted
strings: /tmp/bfdkiller.dat: Reading section .data failed: File truncated
org.ec
$ strings -d --target=a.out-i386 /tmp/eclipse-state
strings: /tmp/eclipse-state: Reading section .text failed: File truncated
org.eclipse.osgi
System Bundle
[snip]

* strings.c (filename_and_size_t): Delete.
(strings_a_section): Don't check section size against file size.
Use bdf_malloc_and_get_section.  Report an error on failures.
Replace arg param with filename and got_a_section param.
(got_a_section): Move to..
(strings_object_file): ..an auto var here.  Iterate over sections
rather than calling bfd_map_over_sections.  Adjust strings_a_section
call.

binutils/ChangeLog
binutils/strings.c

index a7be3db..0c1bf1c 100644 (file)
@@ -1,3 +1,14 @@
+2017-07-03  Alan Modra  <amodra@gmail.com>
+
+       * strings.c (filename_and_size_t): Delete.
+       (strings_a_section): Don't check section size against file size.
+       Use bdf_malloc_and_get_section.  Report an error on failures.
+       Replace arg param with filename and got_a_section param.
+       (got_a_section): Move to..
+       (strings_object_file): ..an auto var here.  Iterate over sections
+       rather than calling bfd_map_over_sections.  Adjust strings_a_section
+       call.
+
 2017-07-02  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
        * dwarf.c: Include assert.h.
index 77d89eb..3954473 100644 (file)
@@ -108,9 +108,6 @@ static bfd_boolean print_filenames;
 /* TRUE means for object files scan only the data section.  */
 static bfd_boolean datasection_only;
 
-/* TRUE if we found an initialized data section in the current file.  */
-static bfd_boolean got_a_section;
-
 /* The BFD object file format.  */
 static char *target;
 
@@ -137,15 +134,6 @@ static struct option long_options[] =
   {NULL, 0, NULL, 0}
 };
 
-/* Records the size of a named file so that we
-   do not repeatedly run bfd_stat() on it.  */
-
-typedef struct
-{
-  const char *  filename;
-  bfd_size_type filesize;
-} filename_and_size_t;
-
 static bfd_boolean strings_file (char *);
 static void print_strings (const char *, FILE *, file_ptr, int, int, char *);
 static void usage (FILE *, int) ATTRIBUTE_NORETURN;
@@ -329,61 +317,33 @@ main (int argc, char **argv)
   return (exit_status);
 }
 \f
-/* Scan section SECT of the file ABFD, whose printable name is in
-   ARG->filename and whose size might be in ARG->filesize.  If it
-   contains initialized data set `got_a_section' and print the
-   strings in it.
-
-   FIXME: We ought to be able to return error codes/messages for
-   certain conditions.  */
+/* Scan section SECT of the file ABFD, whose printable name is
+   FILENAME.  If it contains initialized data set GOT_A_SECTION and
+   print the strings in it.  */
 
 static void
-strings_a_section (bfd *abfd, asection *sect, void *arg)
+strings_a_section (bfd *abfd, asection *sect, const char *filename,
+                  bfd_boolean *got_a_section)
 {
-  filename_and_size_t * filename_and_sizep;
-  bfd_size_type *filesizep;
   bfd_size_type sectsize;
-  void *mem;
+  bfd_byte *mem;
 
   if ((sect->flags & DATA_FLAGS) != DATA_FLAGS)
     return;
 
   sectsize = bfd_get_section_size (sect);
-
-  if (sectsize <= 0)
-    return;
-
-  /* Get the size of the file.  This might have been cached for us.  */
-  filename_and_sizep = (filename_and_size_t *) arg;
-  filesizep = & filename_and_sizep->filesize;
-
-  if (*filesizep == 0)
-    {
-      struct stat st;
-
-      if (bfd_stat (abfd, &st))
-       return;
-
-      /* Cache the result so that we do not repeatedly stat this file.  */
-      *filesizep = st.st_size;
-    }
-
-  /* Compare the size of the section against the size of the file.
-     If the section is bigger then the file must be corrupt and
-     we should not try dumping it.  */
-  if (sectsize >= *filesizep)
+  if (sectsize == 0)
     return;
 
-  mem = xmalloc (sectsize);
-
-  if (bfd_get_section_contents (abfd, sect, mem, (file_ptr) 0, sectsize))
+  if (!bfd_malloc_and_get_section (abfd, sect, &mem))
     {
-      got_a_section = TRUE;
-
-      print_strings (filename_and_sizep->filename, NULL, sect->filepos,
-                    0, sectsize, (char *) mem);
+      non_fatal (_("%s: Reading section %s failed: %s"),
+                filename, sect->name, bfd_errmsg (bfd_get_error ()));
+      return;
     }
 
+  *got_a_section = TRUE;
+  print_strings (filename, NULL, sect->filepos, 0, sectsize, (char *) mem);
   free (mem);
 }
 
@@ -396,8 +356,9 @@ strings_a_section (bfd *abfd, asection *sect, void *arg)
 static bfd_boolean
 strings_object_file (const char *file)
 {
-  filename_and_size_t filename_and_size;
   bfd *abfd;
+  asection *s;
+  bfd_boolean got_a_section;
 
   abfd = bfd_openr (file, target);
 
@@ -415,9 +376,8 @@ strings_object_file (const char *file)
     }
 
   got_a_section = FALSE;
-  filename_and_size.filename = file;
-  filename_and_size.filesize = 0;
-  bfd_map_over_sections (abfd, strings_a_section, & filename_and_size);
+  for (s = abfd->sections; s != NULL; s = s->next)
+    strings_a_section (abfd, s, file, &got_a_section);
 
   if (!bfd_close (abfd))
     {