diff options
| author | Alan Modra <amodra@gmail.com> | 2020-05-21 23:34:58 +0930 |
|---|---|---|
| committer | Alan Modra <amodra@gmail.com> | 2020-05-21 23:39:36 +0930 |
| commit | 0490dd41ae89e66efd8b3cee122c189a481269de (patch) | |
| tree | ea591291023428f54453f894bfb51aa4daa961f2 | |
| parent | d96bf37ba8360320db4d531008634b3f61af06f5 (diff) | |
| download | binutils-gdb-0490dd41ae89e66efd8b3cee122c189a481269de.tar.gz | |
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.
| -rw-r--r-- | bfd/ChangeLog | 8 | ||||
| -rw-r--r-- | bfd/opncls.c | 26 |
2 files changed, 34 insertions, 0 deletions
diff --git a/bfd/ChangeLog b/bfd/ChangeLog index bc51d862ea9..3dc0356928a 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,5 +1,13 @@ 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. * archive.c, * bfd.c, * bfdio.c, * coff-alpha.c, * coff-ppc.c, * coff-sh.c, * coff-stgo32.c, * coffcode.h, * coffgen.c, 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); |