PR24891, objdump memory leaks when parsing malformed archive

BFD was leaking memory in bfd_check_format_matches.  As part of
deciding the proper format of an archive, BFD looks at the format of
the first file stored.  That file's bfd was left open for reasons
given in a comment removed in git commit 0e71e4955cd1 that said:
             /* We ought to close `first' here, but we can't, because
                we have no way to remove it from the archive cache.
                It's close to impossible to figure out when we can
                release bfd_ardata.  FIXME.  */
Well, things have changed since that comment was true and we now can
remove files from the archive cache.  Closing the first file is good
and cures some of the leaks.  Other leaks are caused by
bfd_check_format_matches throwing away bfd tdata before trying a new
match.  That lost the element cache set up when format checking the
first element in the archive.  The easiest and cleanest fix is to
simply disable the caching when checking the first element.

	PR 24891
	* bfd.c (struct bfd): Add no_element_cache.
	* archive.c (_bfd_get_elt_at_filepos): Don't add element to
	archive cache when no_element_cache.
	(bfd_generic_archive_p): Set no_element_cache when opening first
	element to check format.  Close first element too.
	(do_slurp_bsd_armap): Don't zero ardata->cache here.
	* bfd-in2.h: Regenerate.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 9f7a8cfc6afc96bd3348d8ffbc44c079cfb6514c..fe7f8eee6a88ff58eca32dc9b0a3302387f91058 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,14 @@
+2019-08-29  Alan Modra  <amodra@gmail.com>
+
+       PR 24891
+       * bfd.c (struct bfd): Add no_element_cache.
+       * archive.c (_bfd_get_elt_at_filepos): Don't add element to
+       archive cache when no_element_cache.
+       (bfd_generic_archive_p): Set no_element_cache when opening first
+       element to check format.  Close first element too.
+       (do_slurp_bsd_armap): Don't zero ardata->cache here.
+       * bfd-in2.h: Regenerate.
+
 2019-08-24  Alan Modra  <amodra@gmail.com>
 
        * elf64-ppc.c (ppc64_elf_edit_toc): Exclude undefined weak

diff --git a/bfd/archive.c b/bfd/archive.c
index 0a7da3a0cb9c839657aa615107abc67d2d103800..3baf83d40ca18689283b4132468d72c1835aa7f4 100644 (file)
--- a/bfd/archive.c
+++ b/bfd/archive.c
@@ -734,7 +734,8 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
   /* Copy is_linker_input.  */
   n_bfd->is_linker_input = archive->is_linker_input;
 
-  if (_bfd_add_bfd_to_archive_cache (archive, filepos, n_bfd))
+  if (archive->no_element_cache
+      || _bfd_add_bfd_to_archive_cache (archive, filepos, n_bfd))
     return n_bfd;
 
   free (new_areldata);
@@ -885,6 +886,7 @@ bfd_generic_archive_p (bfd *abfd)
   if (abfd->target_defaulted && bfd_has_map (abfd))
     {
       bfd *first;
+      unsigned int save;
 
       /* This archive has a map, so we may presume that the contents
         are object files.  Make sure that if the first file in the
@@ -897,14 +899,17 @@ bfd_generic_archive_p (bfd *abfd)
         normal archive, regardless of the format of the object files.
         We do accept an empty archive.  */
 
+      save = abfd->no_element_cache;
+      abfd->no_element_cache = 1;
       first = bfd_openr_next_archived_file (abfd, NULL);
+      abfd->no_element_cache = save;
       if (first != NULL)
        {
          first->target_defaulted = FALSE;
          if (bfd_check_format (first, bfd_object)
              && first->xvec != abfd->xvec)
            bfd_set_error (bfd_error_wrong_object_format);
-         /* And we ought to close `first' here too.  */
+         bfd_close (first);
        }
     }
 
@@ -974,7 +979,6 @@ do_slurp_bsd_armap (bfd *abfd)
       goto byebye;
     }
 
-  ardata->cache = 0;
   rbase = raw_armap + BSD_SYMDEF_COUNT_SIZE;
   stringbase = ((char *) rbase
                + ardata->symdef_count * BSD_SYMDEF_SIZE

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index cf44bd07514cc9943f571a0cb6542236e03cd4c6..63b4792818ce9c9ee0b17607df1ec60e06aa22a7 100644 (file)
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -7180,6 +7180,9 @@ struct bfd
   /* Set if this is a thin archive.  */
   unsigned int is_thin_archive : 1;
 
+  /* Set if this archive should not cache element positions.  */
+  unsigned int no_element_cache : 1;
+
   /* Set if only required symbols should be added in the link hash table for
      this object.  Used by VMS linkers.  */
   unsigned int selective_search : 1;

diff --git a/bfd/bfd.c b/bfd/bfd.c
index a9b224b925ae1f1166a19644ee5a6fec7a9e1c9e..f56a8d342fba07d3f8e992f06361c6f221dc0338 100644 (file)
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -220,6 +220,9 @@ CODE_FRAGMENT
 .  {* Set if this is a thin archive.  *}
 .  unsigned int is_thin_archive : 1;
 .
+.  {* Set if this archive should not cache element positions.  *}
+.  unsigned int no_element_cache : 1;
+.
 .  {* Set if only required symbols should be added in the link hash table for
 .     this object.  Used by VMS linkers.  *}
 .  unsigned int selective_search : 1;