[PATCH] Fix heap-use-after-free because all_objfiles_removed triggers tui_display_main

[PATCH] Fix heap-use-after-free because all_objfiles_removed triggers tui_display_main

[Hannes Domani]

Since gdb-10 there is a heap-use-after free happening if starting the
target in TUI triggers a re-reading of symbols.

It can be reproduced with:

$ gdb -q -batch a.out -ex "tui enable" -ex "shell touch a.out" -ex start

==28392== Invalid read of size 1
==28392==    at 0x79E97E: lookup_global_or_static_symbol(char const*, block_enum, objfile*, domain_enum) (symtab.h:503)
==28392==    by 0x79F859: lookup_global_symbol(char const*, block const*, domain_enum) (symtab.c:2641)
==28392==    by 0x79F8E9: language_defn::lookup_symbol_nonlocal(char const*, block const*, domain_enum) const (symtab.c:2473)
==28392==    by 0x7A66EE: lookup_symbol_aux(char const*, symbol_name_match_type, block const*, domain_enum, language, field_of_this_result*) (symtab.c:2150)
==28392==    by 0x7A68C9: lookup_symbol_in_language(char const*, block const*, domain_enum, language, field_of_this_result*) (symtab.c:1958)
==28392==    by 0x7A6A25: lookup_symbol(char const*, block const*, domain_enum, field_of_this_result*) (symtab.c:1970)
==28392==    by 0x77120F: select_source_symtab() (source.c:319)
==28392==    by 0x7EE2D5: tui_get_begin_asm_address(gdbarch**, unsigned long*) (tui-disasm.c:401)
==28392==    by 0x807558: tui_display_main() (tui-winsource.c:55)
==28392==    by 0x7937B5: clear_symtab_users(enum_flags<symfile_add_flag>) (functional:2464)
==28392==    by 0x794F40: reread_symbols(int) (symfile.c:2690)
==28392==    by 0x6497D1: run_command_1(char const*, int, run_how) (infcmd.c:398)
==28392==  Address 0x4e67848 is 3,864 bytes inside a block of size 4,064 free'd
==28392==    at 0x4A0A430: free (vg_replace_malloc.c:446)
==28392==    by 0x936B63: _obstack_free (obstack.c:280)
==28392==    by 0x79541E: reread_symbols(int) (symfile.c:2579)
==28392==    by 0x6497D1: run_command_1(char const*, int, run_how) (infcmd.c:398)
==28392==    by 0x4FFC45: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:2735)
==28392==    by 0x7DAB50: execute_command(char const*, int) (top.c:575)
==28392==    by 0x5D2B43: command_handler(char const*) (event-top.c:552)
==28392==    by 0x5D3A50: command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) (event-top.c:788)
==28392==    by 0x5D1F4B: gdb_rl_callback_handler(char*) (event-top.c:259)
==28392==    by 0x857B3F: rl_callback_read_char (callback.c:290)
==28392==    by 0x5D215D: gdb_rl_callback_read_char_wrapper_noexcept() (event-top.c:195)
==28392==    by 0x5D232F: gdb_rl_callback_read_char_wrapper(void*) (event-top.c:234)

The problem is that tui_display_main is called by the all_objfiles_removed
hook, which tries to access the symbol cache.
This symbol cache is actually stale at this point, and would have been
flushed immediately afterwards by that same all_objfiles_removed hook.

It's not possible to tell the hook to call the observers in a specific
order, but in this case the tui_all_objfiles_removed observer is actually
not needed, since it only calls tui_display_main, and a 'main' can only
be found if objfiles are added, not removed.

So the fix is to simply remove the tui_all_objfiles_removed observer.

The clearing of the source window (if symbols were removed by e.g. 'file'
without arguments) still works, since this is done by the
tui_before_prompt observer.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31697
---
 gdb/tui/tui-hooks.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index cf48e709ed8..70874e5460a 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -48,20 +48,13 @@
 
 #include "gdb_curses.h"
 
-static void tui_on_objfiles_changed ()
+static void
+tui_new_objfile_hook (struct objfile* objfile)
 {
   if (tui_active)
     tui_display_main ();
 }
 
-static void
-tui_new_objfile_hook (struct objfile* objfile)
-{ tui_on_objfiles_changed (); }
-
-static void
-tui_all_objfiles_removed (program_space *pspace)
-{ tui_on_objfiles_changed (); }
-
 /* Observer for the register_changed notification.  */
 
 static void
@@ -281,6 +274,4 @@ _initialize_tui_hooks ()
 {
   /* Install the permanent hooks.  */
   gdb::observers::new_objfile.attach (tui_new_objfile_hook, "tui-hooks");
-  gdb::observers::all_objfiles_removed.attach (tui_all_objfiles_removed,
-					       "tui-hooks");
 }
-- 
2.35.1

Re: [PATCH] Fix heap-use-after-free because all_objfiles_removed triggers tui_display_main

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> So the fix is to simply remove the tui_all_objfiles_removed observer.

Hannes> The clearing of the source window (if symbols were removed by e.g. 'file'
Hannes> without arguments) still works, since this is done by the
Hannes> tui_before_prompt observer.

Hannes> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31697

Thank you.  This is ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH] Fix heap-use-after-free because all_objfiles_removed triggers tui_display_main

[Hannes Domani]

 Am Montag, 6. Mai 2024 um 21:59:06 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
>
> Hannes> So the fix is to simply remove the tui_all_objfiles_removed observer.
>
> Hannes> The clearing of the source window (if symbols were removed by e.g. 'file'
> Hannes> without arguments) still works, since this is done by the
> Hannes> tui_before_prompt observer.
>
> Hannes> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31697
>
> Thank you.  This is ok.
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed, thanks.


Hannes