From 0490dd41ae89e66efd8b3cee122c189a481269de Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Thu, 21 May 2020 23:34:58 +0930 Subject: 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. --- bfd/ChangeLog | 8 ++++++++ bfd/opncls.c | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index bc51d862ea9..3dc0356928a 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,11 @@ +2020-05-21 Alan Modra + + 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 * aoutx.h: Replace "if (x) free (x)" with "free (x)" throughout. diff --git a/bfd/opncls.c b/bfd/opncls.c index 794cf99e7ce..c2a1d2fa4df 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); -- cgit v1.2.1