OOM in setup_group

We alloc, seek and read using section sizes in object files.  Fuzzed
objects can have silly sizes, but that's OK if the system supports
memory over-commit.  The read fails because we hit EOF and that
usually results in a graceful exit.

But if we memset before the read then the invalid size results in
attempting to write to a huge number of memory pages, and an eventual
Out Of Memory after probably swapping like crazy.  So don't memset.
There really isn't a need to clear the section contents anyway.  All
bytes are written with a good object file by the read and following
loop converting section index in target order to ELF section header
pointer, and the only untidy bytes are the 4 bytes past the group
flags when pointers are 8 bytes.  Those don't matter but the patch
clears them for anyone poking around in a debugger.  On error paths
it's as good to free section contents as it is to clear them.

Noticed when looking at PR4110 fourth test case.

	PR 4110
	* elf.c (setup_group): Don't clear entire section contents,
	just the padding after group flags.  Release alloc'd memory
	after a seek or read failure.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 30766c113e3fd4ba2e679558b777431875f9499d..0061b6f355705aff11ffe31c67ed95d27e6ff2b4 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,10 @@
+2020-01-31  Alan Modra  <amodra@gmail.com>
+
+       PR 4110
+       * elf.c (setup_group): Don't clear entire section contents,
+       just the padding after group flags.  Release alloc'd memory
+       after a seek or read failure.
+
 2020-01-16  Jon Turney  <jon.turney@dronecode.org.uk>
 
        * peXXigen.c (pe_is_repro): New function.

diff --git a/bfd/elf.c b/bfd/elf.c
index a8d98a60f4eac8b670ac52900c5f8887f49f2ba4..5e6b9a0f4160c15f5e503d5d5c5e905e32b0e11b 100644 (file)
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -676,8 +676,6 @@ setup_group (bfd *abfd, Elf_Internal_Shdr *hdr, asection *newsect)
                      continue;
                    }
 
-                 memset (shdr->contents, 0, amt);
-
                  if (bfd_seek (abfd, shdr->sh_offset, SEEK_SET) != 0
                      || (bfd_bread (shdr->contents, shdr->sh_size, abfd)
                          != shdr->sh_size))
@@ -692,7 +690,8 @@ setup_group (bfd *abfd, Elf_Internal_Shdr *hdr, asection *newsect)
                      /* PR 17510: If the group contents are even
                         partially corrupt, do not allow any of the
                         contents to be used.  */
-                     memset (shdr->contents, 0, amt);
+                     bfd_release (abfd, shdr->contents);
+                     shdr->contents = NULL;
                      continue;
                    }
 
@@ -712,6 +711,7 @@ setup_group (bfd *abfd, Elf_Internal_Shdr *hdr, asection *newsect)
                      idx = H_GET_32 (abfd, src);
                      if (src == shdr->contents)
                        {
+                         dest->shdr = NULL;
                          dest->flags = idx;
                          if (shdr->bfd_section != NULL && (idx & GRP_COMDAT))
                            shdr->bfd_section->flags