* [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy
@ 2023-05-08 14:59 Simon Marchi
2023-05-09 14:21 ` Tom Tromey
2023-05-10 9:12 ` Andrew Burgess
0 siblings, 2 replies; 6+ messages in thread
From: Simon Marchi @ 2023-05-08 14:59 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
Commit 7a8de0c33019 ("Remove ALL_BREAKPOINTS_SAFE") introduced a
use-after-free in the breakpoints iterations (see below for full ASan
report). This makes gdb.base/stale-infcall.exp fail when GDB is build
with ASan.
check_longjmp_breakpoint_for_call_dummy iterates on all breakpoints,
possibly deleting the current breakpoint as well as related breakpoints.
The problem arises when a breakpoint in the B->related_breakpoint chain
is also B->next. In that case, deleting that related breakpoint frees
the breakpoint that all_breakpoints_safe has saved.
The old code worked around that by manually changing B_TMP, which was
the next breakpoint saved by the "safe iterator":
while (b->related_breakpoint != b)
{
if (b_tmp == b->related_breakpoint)
b_tmp = b->related_breakpoint->next;
delete_breakpoint (b->related_breakpoint);
}
(Note that this seemed to assume that b->related_breakpoint->next was
the same as b->next->next, not sure this is guaranteed.)
The new code kept the B_TMP variable, but it's not useful in that
context. We can't go change the next breakpoint as saved by the safe
iterator, like we did before. I suggest fixing that by saving the
breakpoints to delete in a map and deleting them all at the end.
Here's the full ASan report:
(gdb) PASS: gdb.base/stale-infcall.exp: continue to breakpoint: break-run1
print infcall ()
=================================================================
==47472==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000034980 at pc 0x563f7012c7bc bp 0x7ffdf3804d70 sp 0x7ffdf3804d60
READ of size 8 at 0x611000034980 thread T0
#0 0x563f7012c7bb in next_iterator<breakpoint>::operator++() /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/next-iterator.h:66
#1 0x563f702ce8c0 in basic_safe_iterator<next_iterator<breakpoint> >::operator++() /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/safe-iterator.h:84
#2 0x563f7021522a in check_longjmp_breakpoint_for_call_dummy(thread_info*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7611
#3 0x563f714567b1 in process_event_stop_test /home/smarchi/src/binutils-gdb/gdb/infrun.c:6881
#4 0x563f71454e07 in handle_signal_stop /home/smarchi/src/binutils-gdb/gdb/infrun.c:6769
#5 0x563f7144b680 in handle_inferior_event /home/smarchi/src/binutils-gdb/gdb/infrun.c:6023
#6 0x563f71436165 in fetch_inferior_event() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4387
#7 0x563f7136ff51 in inferior_event_handler(inferior_event_type) /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:42
#8 0x563f7168038d in handle_target_event /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4219
#9 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
#10 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
#11 0x563f72fcaf2b in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:217
#12 0x563f7262b9bb in wait_sync_command_done() /home/smarchi/src/binutils-gdb/gdb/top.c:426
#13 0x563f7137a7c3 in run_inferior_call /home/smarchi/src/binutils-gdb/gdb/infcall.c:650
#14 0x563f71381295 in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1332
#15 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
#16 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
#17 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
#18 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
#19 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
#20 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
#21 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
#22 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
#23 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
#24 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
#25 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
#26 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
#27 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
#28 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
#29 0x563f7101014b in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/smarchi/src/binutils-gdb/gdb/event-top.c:779
#30 0x563f72777942 in tui_command_line_handler /home/smarchi/src/binutils-gdb/gdb/tui/tui-interp.c:104
#31 0x563f7100d059 in gdb_rl_callback_handler /home/smarchi/src/binutils-gdb/gdb/event-top.c:250
#32 0x7f5a80418246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)
#33 0x563f7100ca06 in gdb_rl_callback_read_char_wrapper_noexcept /home/smarchi/src/binutils-gdb/gdb/event-top.c:192
#34 0x563f7100cc5e in gdb_rl_callback_read_char_wrapper /home/smarchi/src/binutils-gdb/gdb/event-top.c:225
#35 0x563f728c70db in stdin_event_handler /home/smarchi/src/binutils-gdb/gdb/ui.c:155
#36 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
#37 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
#38 0x563f72fcb15c in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:264
#39 0x563f7177ec1c in start_event_loop /home/smarchi/src/binutils-gdb/gdb/main.c:412
#40 0x563f7177f12e in captured_command_loop /home/smarchi/src/binutils-gdb/gdb/main.c:476
#41 0x563f717846e4 in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1320
#42 0x563f71784821 in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1339
#43 0x563f6fcedfbd in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32
#44 0x7f5a7e43984f (/usr/lib/libc.so.6+0x2384f) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
#45 0x7f5a7e439909 in __libc_start_main (/usr/lib/libc.so.6+0x23909) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
#46 0x563f6fcedd84 in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0xafb0d84) (BuildId: 50bd32e6e9d5e84543e9897b8faca34858ca3995)
0x611000034980 is located 0 bytes inside of 208-byte region [0x611000034980,0x611000034a50)
freed by thread T0 here:
#0 0x7f5a7fce312a in operator delete(void*, unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:164
#1 0x563f702bd1fa in momentary_breakpoint::~momentary_breakpoint() /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:304
#2 0x563f702771c5 in delete_breakpoint(breakpoint*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:12404
#3 0x563f702150a7 in check_longjmp_breakpoint_for_call_dummy(thread_info*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7673
#4 0x563f714567b1 in process_event_stop_test /home/smarchi/src/binutils-gdb/gdb/infrun.c:6881
#5 0x563f71454e07 in handle_signal_stop /home/smarchi/src/binutils-gdb/gdb/infrun.c:6769
#6 0x563f7144b680 in handle_inferior_event /home/smarchi/src/binutils-gdb/gdb/infrun.c:6023
#7 0x563f71436165 in fetch_inferior_event() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4387
#8 0x563f7136ff51 in inferior_event_handler(inferior_event_type) /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:42
#9 0x563f7168038d in handle_target_event /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4219
#10 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
#11 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
#12 0x563f72fcaf2b in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:217
#13 0x563f7262b9bb in wait_sync_command_done() /home/smarchi/src/binutils-gdb/gdb/top.c:426
#14 0x563f7137a7c3 in run_inferior_call /home/smarchi/src/binutils-gdb/gdb/infcall.c:650
#15 0x563f71381295 in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1332
#16 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
#17 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
#18 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
#19 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
#20 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
#21 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
#22 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
#23 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
#24 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
#25 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
#26 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
#27 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
#28 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
#29 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
previously allocated by thread T0 here:
#0 0x7f5a7fce2012 in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
#1 0x563f7029a9a3 in new_momentary_breakpoint<program_space*&, frame_id&, int&> /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:8129
#2 0x563f702212f6 in momentary_breakpoint_from_master /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:8169
#3 0x563f70212db1 in set_longjmp_breakpoint_for_call_dummy() /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7582
#4 0x563f713804db in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1260
#5 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
#6 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
#7 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
#8 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
#9 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
#10 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
#11 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
#12 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
#13 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
#14 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
#15 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
#16 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
#17 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
#18 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
#19 0x563f7101014b in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/smarchi/src/binutils-gdb/gdb/event-top.c:779
#20 0x563f72777942 in tui_command_line_handler /home/smarchi/src/binutils-gdb/gdb/tui/tui-interp.c:104
#21 0x563f7100d059 in gdb_rl_callback_handler /home/smarchi/src/binutils-gdb/gdb/event-top.c:250
#22 0x7f5a80418246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)
Change-Id: Id00c17ab677f847fbf4efdf0f4038373668d3d88
---
gdb/breakpoint.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1cc9e84c11f3..dcb00bffdbe8 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -69,6 +69,7 @@
#include "tid-parse.h"
#include "cli/cli-style.h"
#include "cli/cli-decode.h"
+#include <unordered_set>
/* readline include files */
#include "readline/tilde.h"
@@ -7608,9 +7609,13 @@ set_longjmp_breakpoint_for_call_dummy (void)
void
check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
{
- for (struct breakpoint *b : all_breakpoints_safe ())
+ /* We would need to delete breakpoints other than the current one while
+ iterating, so all_breakpoints_safe is not sufficient to make that safe.
+ Save all breakpoints to delete in that set and delete them at the end. */
+ std::unordered_set<breakpoint *> to_delete;
+
+ for (struct breakpoint *b : all_breakpoints ())
{
- struct breakpoint *b_tmp = b->next;
if (b->type == bp_longjmp_call_dummy && b->thread == tp->global_num)
{
struct breakpoint *dummy_b = b->related_breakpoint;
@@ -7666,15 +7671,17 @@ check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
dummy_frame_discard (dummy_b->frame_id, tp);
- while (b->related_breakpoint != b)
- {
- if (b_tmp == b->related_breakpoint)
- b_tmp = b->related_breakpoint->next;
- delete_breakpoint (b->related_breakpoint);
- }
- delete_breakpoint (b);
+ for (breakpoint *related_breakpoint = b->related_breakpoint;
+ related_breakpoint != b;
+ related_breakpoint = related_breakpoint->related_breakpoint)
+ to_delete.insert (b->related_breakpoint);
+
+ to_delete.insert (b);
}
}
+
+ for (breakpoint *b : to_delete)
+ delete_breakpoint (b);
}
void
--
2.40.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy
2023-05-08 14:59 [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy Simon Marchi
@ 2023-05-09 14:21 ` Tom Tromey
2023-05-09 17:48 ` Simon Marchi
2023-05-10 9:12 ` Andrew Burgess
1 sibling, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2023-05-09 14:21 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi
>>>>> Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> Commit 7a8de0c33019 ("Remove ALL_BREAKPOINTS_SAFE") introduced a
> use-after-free in the breakpoints iterations (see below for full ASan
> report). This makes gdb.base/stale-infcall.exp fail when GDB is build
> with ASan.
Sorry about that.
> The new code kept the B_TMP variable, but it's not useful in that
> context. We can't go change the next breakpoint as saved by the safe
> iterator, like we did before. I suggest fixing that by saving the
> breakpoints to delete in a map and deleting them all at the end.
Looks good.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy
2023-05-09 14:21 ` Tom Tromey
@ 2023-05-09 17:48 ` Simon Marchi
0 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2023-05-09 17:48 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: Simon Marchi
On 5/9/23 10:21, Tom Tromey wrote:
>>>>>> Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> Commit 7a8de0c33019 ("Remove ALL_BREAKPOINTS_SAFE") introduced a
>> use-after-free in the breakpoints iterations (see below for full ASan
>> report). This makes gdb.base/stale-infcall.exp fail when GDB is build
>> with ASan.
>
> Sorry about that.
>
>> The new code kept the B_TMP variable, but it's not useful in that
>> context. We can't go change the next breakpoint as saved by the safe
>> iterator, like we did before. I suggest fixing that by saving the
>> breakpoints to delete in a map and deleting them all at the end.
>
> Looks good.
>
> Approved-By: Tom Tromey <tom@tromey.com>
>
> Tom
Thanks, pushed.
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy
2023-05-08 14:59 [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy Simon Marchi
2023-05-09 14:21 ` Tom Tromey
@ 2023-05-10 9:12 ` Andrew Burgess
2023-05-10 11:50 ` Simon Marchi
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Burgess @ 2023-05-10 9:12 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches, gdb-patches; +Cc: Simon Marchi
Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> Commit 7a8de0c33019 ("Remove ALL_BREAKPOINTS_SAFE") introduced a
> use-after-free in the breakpoints iterations (see below for full ASan
> report). This makes gdb.base/stale-infcall.exp fail when GDB is build
> with ASan.
>
> check_longjmp_breakpoint_for_call_dummy iterates on all breakpoints,
> possibly deleting the current breakpoint as well as related breakpoints.
> The problem arises when a breakpoint in the B->related_breakpoint chain
> is also B->next. In that case, deleting that related breakpoint frees
> the breakpoint that all_breakpoints_safe has saved.
>
> The old code worked around that by manually changing B_TMP, which was
> the next breakpoint saved by the "safe iterator":
>
> while (b->related_breakpoint != b)
> {
> if (b_tmp == b->related_breakpoint)
> b_tmp = b->related_breakpoint->next;
> delete_breakpoint (b->related_breakpoint);
> }
>
> (Note that this seemed to assume that b->related_breakpoint->next was
> the same as b->next->next, not sure this is guaranteed.)
>
> The new code kept the B_TMP variable, but it's not useful in that
> context. We can't go change the next breakpoint as saved by the safe
> iterator, like we did before. I suggest fixing that by saving the
> breakpoints to delete in a map and deleting them all at the end.
>
> Here's the full ASan report:
>
> (gdb) PASS: gdb.base/stale-infcall.exp: continue to breakpoint: break-run1
> print infcall ()
> =================================================================
> ==47472==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000034980 at pc 0x563f7012c7bc bp 0x7ffdf3804d70 sp 0x7ffdf3804d60
> READ of size 8 at 0x611000034980 thread T0
> #0 0x563f7012c7bb in next_iterator<breakpoint>::operator++() /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/next-iterator.h:66
> #1 0x563f702ce8c0 in basic_safe_iterator<next_iterator<breakpoint> >::operator++() /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/safe-iterator.h:84
> #2 0x563f7021522a in check_longjmp_breakpoint_for_call_dummy(thread_info*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7611
> #3 0x563f714567b1 in process_event_stop_test /home/smarchi/src/binutils-gdb/gdb/infrun.c:6881
> #4 0x563f71454e07 in handle_signal_stop /home/smarchi/src/binutils-gdb/gdb/infrun.c:6769
> #5 0x563f7144b680 in handle_inferior_event /home/smarchi/src/binutils-gdb/gdb/infrun.c:6023
> #6 0x563f71436165 in fetch_inferior_event() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4387
> #7 0x563f7136ff51 in inferior_event_handler(inferior_event_type) /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:42
> #8 0x563f7168038d in handle_target_event /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4219
> #9 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
> #10 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
> #11 0x563f72fcaf2b in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:217
> #12 0x563f7262b9bb in wait_sync_command_done() /home/smarchi/src/binutils-gdb/gdb/top.c:426
> #13 0x563f7137a7c3 in run_inferior_call /home/smarchi/src/binutils-gdb/gdb/infcall.c:650
> #14 0x563f71381295 in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1332
> #15 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
> #16 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
> #17 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
> #18 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
> #19 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
> #20 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
> #21 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
> #22 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
> #23 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
> #24 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
> #25 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
> #26 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
> #27 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
> #28 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
> #29 0x563f7101014b in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/smarchi/src/binutils-gdb/gdb/event-top.c:779
> #30 0x563f72777942 in tui_command_line_handler /home/smarchi/src/binutils-gdb/gdb/tui/tui-interp.c:104
> #31 0x563f7100d059 in gdb_rl_callback_handler /home/smarchi/src/binutils-gdb/gdb/event-top.c:250
> #32 0x7f5a80418246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)
> #33 0x563f7100ca06 in gdb_rl_callback_read_char_wrapper_noexcept /home/smarchi/src/binutils-gdb/gdb/event-top.c:192
> #34 0x563f7100cc5e in gdb_rl_callback_read_char_wrapper /home/smarchi/src/binutils-gdb/gdb/event-top.c:225
> #35 0x563f728c70db in stdin_event_handler /home/smarchi/src/binutils-gdb/gdb/ui.c:155
> #36 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
> #37 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
> #38 0x563f72fcb15c in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:264
> #39 0x563f7177ec1c in start_event_loop /home/smarchi/src/binutils-gdb/gdb/main.c:412
> #40 0x563f7177f12e in captured_command_loop /home/smarchi/src/binutils-gdb/gdb/main.c:476
> #41 0x563f717846e4 in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1320
> #42 0x563f71784821 in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1339
> #43 0x563f6fcedfbd in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32
> #44 0x7f5a7e43984f (/usr/lib/libc.so.6+0x2384f) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
> #45 0x7f5a7e439909 in __libc_start_main (/usr/lib/libc.so.6+0x23909) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
> #46 0x563f6fcedd84 in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0xafb0d84) (BuildId: 50bd32e6e9d5e84543e9897b8faca34858ca3995)
>
> 0x611000034980 is located 0 bytes inside of 208-byte region [0x611000034980,0x611000034a50)
> freed by thread T0 here:
> #0 0x7f5a7fce312a in operator delete(void*, unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:164
> #1 0x563f702bd1fa in momentary_breakpoint::~momentary_breakpoint() /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:304
> #2 0x563f702771c5 in delete_breakpoint(breakpoint*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:12404
> #3 0x563f702150a7 in check_longjmp_breakpoint_for_call_dummy(thread_info*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7673
> #4 0x563f714567b1 in process_event_stop_test /home/smarchi/src/binutils-gdb/gdb/infrun.c:6881
> #5 0x563f71454e07 in handle_signal_stop /home/smarchi/src/binutils-gdb/gdb/infrun.c:6769
> #6 0x563f7144b680 in handle_inferior_event /home/smarchi/src/binutils-gdb/gdb/infrun.c:6023
> #7 0x563f71436165 in fetch_inferior_event() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4387
> #8 0x563f7136ff51 in inferior_event_handler(inferior_event_type) /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:42
> #9 0x563f7168038d in handle_target_event /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4219
> #10 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
> #11 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
> #12 0x563f72fcaf2b in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:217
> #13 0x563f7262b9bb in wait_sync_command_done() /home/smarchi/src/binutils-gdb/gdb/top.c:426
> #14 0x563f7137a7c3 in run_inferior_call /home/smarchi/src/binutils-gdb/gdb/infcall.c:650
> #15 0x563f71381295 in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1332
> #16 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
> #17 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
> #18 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
> #19 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
> #20 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
> #21 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
> #22 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
> #23 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
> #24 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
> #25 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
> #26 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
> #27 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
> #28 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
> #29 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
>
> previously allocated by thread T0 here:
> #0 0x7f5a7fce2012 in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
> #1 0x563f7029a9a3 in new_momentary_breakpoint<program_space*&, frame_id&, int&> /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:8129
> #2 0x563f702212f6 in momentary_breakpoint_from_master /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:8169
> #3 0x563f70212db1 in set_longjmp_breakpoint_for_call_dummy() /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7582
> #4 0x563f713804db in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1260
> #5 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
> #6 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
> #7 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
> #8 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
> #9 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
> #10 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
> #11 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
> #12 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
> #13 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
> #14 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
> #15 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
> #16 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
> #17 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
> #18 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
> #19 0x563f7101014b in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/smarchi/src/binutils-gdb/gdb/event-top.c:779
> #20 0x563f72777942 in tui_command_line_handler /home/smarchi/src/binutils-gdb/gdb/tui/tui-interp.c:104
> #21 0x563f7100d059 in gdb_rl_callback_handler /home/smarchi/src/binutils-gdb/gdb/event-top.c:250
> #22 0x7f5a80418246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)
>
> Change-Id: Id00c17ab677f847fbf4efdf0f4038373668d3d88
> ---
> gdb/breakpoint.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 1cc9e84c11f3..dcb00bffdbe8 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -69,6 +69,7 @@
> #include "tid-parse.h"
> #include "cli/cli-style.h"
> #include "cli/cli-decode.h"
> +#include <unordered_set>
>
> /* readline include files */
> #include "readline/tilde.h"
> @@ -7608,9 +7609,13 @@ set_longjmp_breakpoint_for_call_dummy (void)
> void
> check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
> {
> - for (struct breakpoint *b : all_breakpoints_safe ())
> + /* We would need to delete breakpoints other than the current one while
> + iterating, so all_breakpoints_safe is not sufficient to make that safe.
> + Save all breakpoints to delete in that set and delete them at the end. */
> + std::unordered_set<breakpoint *> to_delete;
Hi,
For my own education: why did you choose a std::unordered_set here? I
would assume that we will never find the same related breakpoint more
than once. Indeed, if we did then I suspect the old code would have
resulted in a double free.
So why choose a set over a vector?
Thanks,
Andrew
> +
> + for (struct breakpoint *b : all_breakpoints ())
> {
> - struct breakpoint *b_tmp = b->next;
> if (b->type == bp_longjmp_call_dummy && b->thread == tp->global_num)
> {
> struct breakpoint *dummy_b = b->related_breakpoint;
> @@ -7666,15 +7671,17 @@ check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
>
> dummy_frame_discard (dummy_b->frame_id, tp);
>
> - while (b->related_breakpoint != b)
> - {
> - if (b_tmp == b->related_breakpoint)
> - b_tmp = b->related_breakpoint->next;
> - delete_breakpoint (b->related_breakpoint);
> - }
> - delete_breakpoint (b);
> + for (breakpoint *related_breakpoint = b->related_breakpoint;
> + related_breakpoint != b;
> + related_breakpoint = related_breakpoint->related_breakpoint)
> + to_delete.insert (b->related_breakpoint);
> +
> + to_delete.insert (b);
> }
> }
> +
> + for (breakpoint *b : to_delete)
> + delete_breakpoint (b);
> }
>
> void
> --
> 2.40.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy
2023-05-10 9:12 ` Andrew Burgess
@ 2023-05-10 11:50 ` Simon Marchi
2023-05-12 10:19 ` Andrew Burgess
0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2023-05-10 11:50 UTC (permalink / raw)
To: Andrew Burgess, Simon Marchi via Gdb-patches; +Cc: Simon Marchi
On 5/10/23 05:12, Andrew Burgess via Gdb-patches wrote:
>> @@ -7608,9 +7609,13 @@ set_longjmp_breakpoint_for_call_dummy (void)
>> void
>> check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
>> {
>> - for (struct breakpoint *b : all_breakpoints_safe ())
>> + /* We would need to delete breakpoints other than the current one while
>> + iterating, so all_breakpoints_safe is not sufficient to make that safe.
>> + Save all breakpoints to delete in that set and delete them at the end. */
>> + std::unordered_set<breakpoint *> to_delete;
>
> Hi,
>
> For my own education: why did you choose a std::unordered_set here? I
> would assume that we will never find the same related breakpoint more
> than once. Indeed, if we did then I suspect the old code would have
> resulted in a double free.
>
> So why choose a set over a vector?
We look for bp_longjmp_call_dummy breakpoints, which are documented like
this:
/* Breakpoint placed to the same location(s) like bp_longjmp but used to
protect against stale DUMMY_FRAME. Multiple bp_longjmp_call_dummy and
one bp_call_dummy are chained together by related_breakpoint for each
DUMMY_FRAME. */
I can imagine this happening: suppose X and Y are two related
bp_longjmp_call_dummy breakpoints, following each other in
breakpoint_chain. When looking at X, we will insert X and Y in
to_delete. We will then look at X, and we will try to insert X and Y
again in to_delete.
The old code wouldn't double free or use-after-free, because of its
special handling of B_TMP. When looking at X, we would delete Y and
then X. And if Y happened to be the next iteration value (saved in the
B_TMP variable), we would modify B_TMP to avoid iterating on Y.
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy
2023-05-10 11:50 ` Simon Marchi
@ 2023-05-12 10:19 ` Andrew Burgess
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Burgess @ 2023-05-12 10:19 UTC (permalink / raw)
To: Simon Marchi, Simon Marchi via Gdb-patches; +Cc: Simon Marchi
Simon Marchi <simark@simark.ca> writes:
> On 5/10/23 05:12, Andrew Burgess via Gdb-patches wrote:
>>> @@ -7608,9 +7609,13 @@ set_longjmp_breakpoint_for_call_dummy (void)
>>> void
>>> check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
>>> {
>>> - for (struct breakpoint *b : all_breakpoints_safe ())
>>> + /* We would need to delete breakpoints other than the current one while
>>> + iterating, so all_breakpoints_safe is not sufficient to make that safe.
>>> + Save all breakpoints to delete in that set and delete them at the end. */
>>> + std::unordered_set<breakpoint *> to_delete;
>>
>> Hi,
>>
>> For my own education: why did you choose a std::unordered_set here? I
>> would assume that we will never find the same related breakpoint more
>> than once. Indeed, if we did then I suspect the old code would have
>> resulted in a double free.
>>
>> So why choose a set over a vector?
>
> We look for bp_longjmp_call_dummy breakpoints, which are documented like
> this:
>
> /* Breakpoint placed to the same location(s) like bp_longjmp but used to
> protect against stale DUMMY_FRAME. Multiple bp_longjmp_call_dummy and
> one bp_call_dummy are chained together by related_breakpoint for each
> DUMMY_FRAME. */
>
> I can imagine this happening: suppose X and Y are two related
> bp_longjmp_call_dummy breakpoints, following each other in
> breakpoint_chain. When looking at X, we will insert X and Y in
> to_delete. We will then look at X, and we will try to insert X and Y
> again in to_delete.
>
> The old code wouldn't double free or use-after-free, because of its
> special handling of B_TMP. When looking at X, we would delete Y and
> then X. And if Y happened to be the next iteration value (saved in the
> B_TMP variable), we would modify B_TMP to avoid iterating on Y.
Thanks for the explanation.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-12 10:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-08 14:59 [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy Simon Marchi
2023-05-09 14:21 ` Tom Tromey
2023-05-09 17:48 ` Simon Marchi
2023-05-10 9:12 ` Andrew Burgess
2023-05-10 11:50 ` Simon Marchi
2023-05-12 10:19 ` Andrew Burgess
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).