From 3097045a18a8878bc40548bd0995f9bad5609c43 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Thu, 23 Jan 2025 10:24:46 +1030 Subject: [PATCH] ld plugin bfd_make_readable leak bfd_make_readable leaks memory that could be freed by _free_cached_info except that does too much in releasing all bfd memory. (The fact that we had to hack around keeping the bfd filename also indicates that releasing all bfd memory was too much.) So this patch moves code releasing bfd_alloc'd memory to the COFF _free_cached_info, where the syms and suchlike are released. This is the memory that archive handling wants to release in the call there to bfd_free_cached_info. * coffgen.c (_bfd_coff_free_cached_info): Release syms. * opncls.c (_bfd_new_bfd): Correct error return path. (_bfd_free_cached_info): Don't kill all abfd->memory. (_bfd_delete_bfd): Adjust fallback for bfd_free_cached_info. (bfd_make_readable): Call target bfd_free_cached_info and _bfd_free_cached_info plus reinstate section_htab. --- bfd/coffgen.c | 8 +++++ bfd/opncls.c | 88 ++++++++++++++++++--------------------------------- 2 files changed, 39 insertions(+), 57 deletions(-) diff --git a/bfd/coffgen.c b/bfd/coffgen.c index d8c60e26342..c4026e96afb 100644 --- a/bfd/coffgen.c +++ b/bfd/coffgen.c @@ -3296,6 +3296,14 @@ _bfd_coff_free_cached_info (bfd *abfd) These may have been set by pe_ILF_build_a_bfd() indicating that the syms and strings pointers are not to be freed. */ _bfd_coff_free_symbols (abfd); + + /* Free raw syms, and any other data bfd_alloc'd after raw syms + are read. */ + if (obj_raw_syments (abfd)) + bfd_release (abfd, obj_raw_syments (abfd)); + obj_raw_syments (abfd) = NULL; + obj_symbols (abfd) = NULL; + obj_convert (abfd) = NULL; } return _bfd_generic_bfd_free_cached_info (abfd); diff --git a/bfd/opncls.c b/bfd/opncls.c index 45f774c2c92..47a74512efc 100644 --- a/bfd/opncls.c +++ b/bfd/opncls.c @@ -73,20 +73,16 @@ _bfd_new_bfd (void) return NULL; if (!bfd_lock ()) - return NULL; + goto loser; nbfd->id = bfd_id_counter++; if (!bfd_unlock ()) - { - free (nbfd); - return NULL; - } + goto loser; nbfd->memory = objalloc_create (); if (nbfd->memory == NULL) { bfd_set_error (bfd_error_no_memory); - free (nbfd); - return NULL; + goto loser; } nbfd->arch_info = &bfd_default_arch_struct; @@ -94,14 +90,16 @@ _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; + objalloc_free (nbfd->memory); + goto loser; } nbfd->archive_plugin_fd = -1; - return nbfd; + + loser: + free (nbfd); + return NULL; } static const struct bfd_iovec opncls_iovec; @@ -153,13 +151,10 @@ _bfd_delete_bfd (bfd *abfd) bfd_free_cached_info (abfd); /* The target _bfd_free_cached_info may not have done anything.. */ + if (abfd->section_htab.memory) + bfd_hash_table_free (&abfd->section_htab); if (abfd->memory) - { - bfd_hash_table_free (&abfd->section_htab); - objalloc_free ((struct objalloc *) abfd->memory); - } - else - free ((char *) bfd_get_filename (abfd)); + objalloc_free (abfd->memory); #ifdef USE_MMAP struct bfd_mmapped *mmapped, *next; @@ -191,41 +186,15 @@ DESCRIPTION bool _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); - - abfd->sections = NULL; - abfd->section_last = NULL; - abfd->outsymbols = NULL; - abfd->tdata.any = NULL; - abfd->usrdata = NULL; - abfd->memory = NULL; - } + if (abfd->section_htab.memory) + bfd_hash_table_free (&abfd->section_htab); + + abfd->sections = NULL; + abfd->section_last = NULL; + abfd->section_count = 0; + abfd->outsymbols = NULL; + abfd->tdata.any = NULL; + abfd->usrdata = NULL; return true; } @@ -1043,10 +1012,18 @@ bfd_make_readable (bfd *abfd) return false; } - if (! BFD_SEND_FMT (abfd, _bfd_write_contents, (abfd))) + if (!BFD_SEND_FMT (abfd, _bfd_write_contents, (abfd))) return false; - if (! BFD_SEND (abfd, _close_and_cleanup, (abfd))) + if (!BFD_SEND (abfd, _bfd_free_cached_info, (abfd))) + return false; + + if (!BFD_SEND (abfd, _close_and_cleanup, (abfd))) + return false; + + _bfd_free_cached_info (abfd); + if (!bfd_hash_table_init_n (&abfd->section_htab, bfd_section_hash_newfunc, + sizeof (struct section_hash_entry), 13)) return false; abfd->arch_info = &bfd_default_arch_struct; @@ -1057,20 +1034,17 @@ bfd_make_readable (bfd *abfd) abfd->origin = 0; abfd->opened_once = false; abfd->output_has_begun = false; - abfd->section_count = 0; abfd->usrdata = NULL; abfd->cacheable = false; abfd->mtime_set = false; abfd->target_defaulted = true; abfd->direction = read_direction; - abfd->sections = 0; abfd->symcount = 0; abfd->outsymbols = 0; abfd->tdata.any = 0; abfd->size = 0; - bfd_section_list_clear (abfd); bfd_check_format (abfd, bfd_object); return true; -- 2.39.5