Re: [PATCH] gdb: Fix use-after-free when an objfile has no symbols to load

[Guinevere Larsen <guinevere@redhat.com>]

On 12/5/24 10:19 AM, Guinevere Larsen wrote:
> The recent commit <HASH> moved an initialization of an objfile_holder in
Oops, I meant to go back and add the hash of the commit there.... it is 
commit 32e3f1a0aa0
> syms_from_objfile_1 much earlier in the function, to better deal with
> when GDB is unable to read the objfile format.
>
> However, there is an early exit from syms_from_objfile_1 when the
> objfile can be understood, but has no symbols. That was not releasing
> the objfile_holder, so the objfile was being unlinked from the program
> space, but the process of reading the objfile was being continued,
> leading to use-after-frees flagged by the Address Sanitizer.
>
> This commit fixes that UAF by making the objfile_holder release the
> objfile right before the early exit.
>
> This commit also changes the test gdb.base/dump.exp since that was the
> original test that flagged the UAF, but at the end of the test the
> generated files were being deleted, meaning we couldn't redo the test
> manually after teh fact. That final deletion was removed
>
> Reported-by: Simon Marchi <simark@simark.ca>
> ---
>   gdb/symfile.c                   | 4 ++++
>   gdb/testsuite/gdb.base/dump.exp | 4 ----
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 3fd6c8d73a2..28c0d46ab54 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -901,6 +901,10 @@ syms_from_objfile_1 (struct objfile *objfile,
>         int num_sections = gdb_bfd_count_sections (objfile->obfd.get ());
>   
>         objfile->section_offsets.assign (num_sections, 0);
> +
> +      /* Release the objfile unique pointer, since nothing went wrong
> +	 in reading it.  */
> +      objfile_holder.release ();
>         return;
>       }
>   
> diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp
> index 3c7bee5ff30..58fedb1d36b 100644
> --- a/gdb/testsuite/gdb.base/dump.exp
> +++ b/gdb/testsuite/gdb.base/dump.exp
> @@ -564,7 +564,3 @@ if {![string compare $is64bitonly "no"]} {
>         "reload struct as memory, tekhex" \
>   	$struct_val "\*$struct_ptr_type"
>   }
> -
> -# clean up files
> -
> -remote_exec host "rm -f $filenames"


-- 
Cheers,
Guinevere Larsen
She/Her/Hers