espressif/binutils-gdb
1
0
Fork 0
mirror of https://github.com/espressif/binutils-gdb.git synced 2025-06-25 21:41:47 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix use after free introduced by $_hit_bpnum/$_hit_locno variables.

Browse Source 
If the commands of the bpstat bs contain commands such as step or next or
continue, the BS and its commands are freed by execute_control_command.

So, we cannot remember the BS that was printed. Instead, remember
the bpnum and locno.

Regtested on debian/amd64 and re-run a few tests under valgrind.
This commit is contained in:
Philippe Waroquiers
2022-11-20 11:11:27 +01:00
parent e66c9edea3
commit 83f350830e
1 changed files with 36 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
gdb/breakpoint.c
View File

@ -4574,18 +4574,17 @@ command_line_is_silent (struct command_line *cmd)
return cmd && (strcmp ("silent", cmd->line) == 0); return cmd && (strcmp ("silent", cmd->line) == 0);
} }
/* Sets the $_hit_bpnum and $_hit_locno to the bpnum and locno of bs. */ /* Sets the $_hit_bpnum and $_hit_locno to bpnum and locno.
A locno 0 is changed to 1 to e.g. let the user do
(gdb) disable $_hit_bpnum.$_hit_locno
for a single location breakpoint. */
static void static void
set_hit_convenience_vars (bpstat *bs) set_hit_convenience_vars (int bpnum, int locno)
{ {
const struct breakpoint *b = bs->breakpoint_at; set_internalvar_integer (lookup_internalvar ("_hit_bpnum"), bpnum);
if (b != nullptr) set_internalvar_integer (lookup_internalvar ("_hit_locno"),
{ (locno > 0 ? locno : 1));
int locno = bpstat_locno (bs);
set_internalvar_integer (lookup_internalvar ("_hit_bpnum"), b->number);
set_internalvar_integer (lookup_internalvar ("_hit_locno"),
(locno > 0 ? locno : 1));
}
} }
/* Execute all the commands associated with all the breakpoints at /* Execute all the commands associated with all the breakpoints at
@ -4602,7 +4601,6 @@ bpstat_do_actions_1 (bpstat **bsp)
{ {
bpstat *bs; bpstat *bs;
bool again = false; bool again = false;
bpstat *bs_print_hit_var;
/* Avoid endless recursion if a `source' command is contained /* Avoid endless recursion if a `source' command is contained
in bs->commands. */ in bs->commands. */
@ -4618,29 +4616,39 @@ bpstat_do_actions_1 (bpstat **bsp)
bs = *bsp; bs = *bsp;
/* The $_hit_* convenience variables are set before running the /* The $_hit_* convenience variables are set before running the
commands of bs. In case we have several bs, after the loop, commands of BS. In case we have several bs, after the loop,
we set again the variables to the first bs to print. */ we set again the variables to the first printed bpnum and locno.
bs_print_hit_var = nullptr; For multiple breakpoints, this ensures the variables are set to the
breakpoint printed for the user. */
int printed_hit_bpnum = -1;
int printed_hit_locno = -1;
breakpoint_proceeded = 0; breakpoint_proceeded = 0;
for (; bs != NULL; bs = bs->next) for (; bs != NULL; bs = bs->next)
{ {
struct command_line *cmd = NULL; struct command_line *cmd = NULL;
/* Set the _hit_* convenience variables before running the commands of /* Set the _hit_* convenience variables before running BS's commands. */
each bs. If this is the first bs to be printed, remember it so as to {
set the convenience variable again to this bs after the loop so that in const struct breakpoint *b = bs->breakpoint_at;
case of multiple breakpoints, the variables are set to the breakpoint if (b != nullptr)
printed for the user. */ {
set_hit_convenience_vars (bs); int locno = bpstat_locno (bs);
if (bs_print_hit_var == nullptr && bs->print)
bs_print_hit_var = bs; set_hit_convenience_vars (b->number, locno);
if (printed_hit_locno == -1 && bs->print)
{
printed_hit_bpnum = b->number;
printed_hit_locno = locno;
}
}
}
/* Take ownership of the BSP's command tree, if it has one. /* Take ownership of the BSP's command tree, if it has one.
The command tree could legitimately contain commands like The command tree could legitimately contain commands like
'step' and 'next', which call clear_proceed_status, which 'step' and 'next', which call clear_proceed_status, which
frees stop_bpstat's command tree. To make sure this doesn't frees the bpstat BS and its command tree. To make sure this doesn't
free the tree we're executing out from under us, we need to free the tree we're executing out from under us, we need to
take ownership of the tree ourselves. Since a given bpstat's take ownership of the tree ourselves. Since a given bpstat's
commands are only executed once, we don't need to copy it; we commands are only executed once, we don't need to copy it; we
@ -4659,6 +4667,8 @@ bpstat_do_actions_1 (bpstat **bsp)
while (cmd != NULL) while (cmd != NULL)
{ {
execute_control_command (cmd); execute_control_command (cmd);
/* After execute_control_command, if breakpoint_proceeded is true,
BS has been freed and cannot be accessed anymore. */
if (breakpoint_proceeded) if (breakpoint_proceeded)
break; break;
@ -4693,9 +4703,9 @@ bpstat_do_actions_1 (bpstat **bsp)
} }
/* Now that we have executed the commands of all bs, set the _hit_* /* Now that we have executed the commands of all bs, set the _hit_*
convenience variables to the printed bs. */ convenience variables to the printed values. */
if (bs_print_hit_var != nullptr) if (printed_hit_locno != -1)
set_hit_convenience_vars (bs_print_hit_var); set_hit_convenience_vars (printed_hit_bpnum, printed_hit_locno);
return again; return again;
} }