[binutils-gdb] Re: PR25993, read of freed memory

[gdb-buildbot@sergiodj.net]

*** TEST RESULTS FOR COMMIT 0490dd41ae89e66efd8b3cee122c189a481269de ***

commit 0490dd41ae89e66efd8b3cee122c189a481269de
Author:     Alan Modra <amodra@gmail.com>
AuthorDate: Thu May 21 23:34:58 2020 +0930
Commit:     Alan Modra <amodra@gmail.com>
CommitDate: Thu May 21 23:39:36 2020 +0930

    Re: PR25993, read of freed memory
    
    git commit 7b958a48e132 put the bfd filename in the bfd objalloc
    memory.  That means the filename is freed by _bfd_free_cached_info.
    Which is called by _bfd_compute_and_write_armap to tidy up symbol
    tables after they are done with.
    
    Unfortunately, _bfd_write_archive_contents wants to seek and read from
    archive elements after that point, and if the number of elements
    exceeds max_open_files in cache.c then some of those elements will
    have their files closed.  To reopen, you need the filename.
    
            PR 25993
            * opncls.c (_bfd_free_cached_info): Keep a copy of the bfd
            filename.
            (_bfd_delete_bfd): Free the copy.
            (_bfd_new_bfd): Free nbfd->memory on error.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index bc51d862ea..3dc0356928 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-21  Alan Modra  <amodra@gmail.com>
+
+	PR 25993
+	* opncls.c (_bfd_free_cached_info): Keep a copy of the bfd
+	filename.
+	(_bfd_delete_bfd): Free the copy.
+	(_bfd_new_bfd): Free nbfd->memory on error.
+
 2020-05-21  Alan Modra  <amodra@gmail.com>
 
 	* aoutx.h: Replace "if (x) free (x)" with "free (x)" throughout.
diff --git a/bfd/opncls.c b/bfd/opncls.c
index 794cf99e7c..c2a1d2fa4d 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -84,6 +84,7 @@ _bfd_new_bfd (void)
   if (!bfd_hash_table_init_n (& nbfd->section_htab, bfd_section_hash_newfunc,
 			      sizeof (struct section_hash_entry), 13))
     {
+      objalloc_free ((struct objalloc *) nbfd->memory);
       free (nbfd);
       return NULL;
     }
@@ -125,6 +126,8 @@ _bfd_delete_bfd (bfd *abfd)
       bfd_hash_table_free (&abfd->section_htab);
       objalloc_free ((struct objalloc *) abfd->memory);
     }
+  else
+    free ((char *) bfd_get_filename (abfd));
 
   free (abfd->arelt_data);
   free (abfd);
@@ -137,6 +140,29 @@ _bfd_free_cached_info (bfd *abfd)
 {
   if (abfd->memory)
     {
+      const char *filename = bfd_get_filename (abfd);
+      if (filename)
+	{
+	  /* We can't afford to lose the bfd filename when freeing
+	     abfd->memory, because that would kill the cache.c scheme
+	     of closing and reopening files in order to limit the
+	     number of open files.  To reopen, you need the filename.
+	     And indeed _bfd_compute_and_write_armap calls
+	     _bfd_free_cached_info to free up space used by symbols
+	     and by check_format_matches.  Which we want to continue
+	     doing to handle very large archives.  Later the archive
+	     elements are copied, which might require reopening files.
+	     We also want to keep using objalloc memory for the
+	     filename since that allows the name to be updated
+	     without either leaking memory or implementing some sort
+	     of reference counted string for copies of the filename.  */
+	  size_t len = strlen (filename) + 1;
+	  char *copy = bfd_malloc (len);
+	  if (copy == NULL)
+	    return FALSE;
+	  memcpy (copy, filename, len);
+	  abfd->filename = copy;
+	}
       bfd_hash_table_free (&abfd->section_htab);
       objalloc_free ((struct objalloc *) abfd->memory);