Re: [PATCH] gdb: fix post-hook execution for remote targets

["Jan Vraný" <Jan.Vrany@labware.com>]

Polite ping 2.

Jan


On Wed, 2023-05-03 at 15:40 +0100, Jan Vrany wrote:
> Polite ping. 
> 
> Jan
> 
> On Fri, 2023-04-14 at 17:17 +0200, Jan Vrany wrote:
> > Commit b5661ff2 ("gdb: fix possible use-after-free when
> > executing commands") attempted to fix possible use-after-free
> > in case command redefines itself.
> > 
> > Commit 37e5833d ("gdb: fix command lookup in execute_command ()")
> > updated the previous fix to handle subcommands as well by using the
> > original command string to lookup the command again after its execution.
> > 
> > This fixed the test in gdb.base/define.exp but it turned out that it
> > does not work (at least) for "target remote" and "target extended-remote".
> > 
> > The problem is that the command buffer P passed to execute_command ()
> > gets overwritten in dont_repeat () while executing "target remote"
> > command itself:
> > 
> > 	#0  dont_repeat () at top.c:822
> > 	#1  0x000055555730982a in target_preopen (from_tty=1) at target.c:2483
> > 	#2  0x000055555711e911 in remote_target::open_1 (name=0x55555881c7fe ":1234", from_tty=1, extended_p=0)
> > 	    at remote.c:5946
> > 	#3  0x000055555711d577 in remote_target::open (name=0x55555881c7fe ":1234", from_tty=1) at remote.c:5272
> > 	#4  0x00005555573062f2 in open_target (args=0x55555881c7fe ":1234", from_tty=1, command=0x5555589d0490)
> > 	    at target.c:853
> > 	#5  0x0000555556ad22fa in cmd_func (cmd=0x5555589d0490, args=0x55555881c7fe ":1234", from_tty=1)
> > 	    at cli/cli-decode.c:2737
> > 	#6  0x00005555573487fd in execute_command (p=0x55555881c802 "4", from_tty=1) at top.c:688
> > 
> > Therefore the second call to lookup_cmd () at line 697 fails to find
> > command because the original command string is gone.
> > 
> > This commit addresses this particular problem by creating a *copy* of
> > original command string for the sole purpose of using it after command
> > execution to lookup the command again. It may not be the most efficient
> > way but it's safer given that command buffer is shared and overwritten
> > in hard-to-foresee situations.
> > 
> > Tested on x86_64-linux.
> > 
> > PR 30249
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30249
> > ---
> >  gdb/top.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gdb/top.c b/gdb/top.c
> > index 81f74f72f61..63798789553 100644
> > --- a/gdb/top.c
> > +++ b/gdb/top.c
> > @@ -575,6 +575,7 @@ execute_command (const char *p, int from_tty)
> >    struct cmd_list_element *c;
> >    const char *line;
> >    const char *cmd_start = p;
> > +  std::string cmd_copy = p;
> >  
> >    auto cleanup_if_error = make_scope_exit (bpstat_clear_actions);
> >    scoped_value_mark cleanup = prepare_execute_command ();
> > @@ -692,7 +693,7 @@ execute_command (const char *p, int from_tty)
> >  	 We need to lookup the command again since during its execution,
> >  	 a command may redefine itself.  In this case, C pointer
> >  	 becomes invalid so we need to look it up again.  */
> > -      const char *cmd2 = cmd_start;
> > +      const char *cmd2 = cmd_copy.c_str ();
> >        c = lookup_cmd (&cmd2, cmdlist, "", nullptr, 1, 1);
> >        if (c != nullptr)
> >  	execute_cmd_post_hook (c);
>