From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id E0CA63858D35 for ; Wed, 15 Feb 2023 18:01:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E0CA63858D35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 1968033A97; Wed, 15 Feb 2023 18:01:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1676484113; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UvvYXBKDbYeAaeFJSw2sbmMUgXCMs8nLK7hC02IoEZ8=; b=VdmKIDsq0MlI1jj+HGvOoMY6oSmMoJoSy9rGKI/MhBwjyRtm24dR1Yjty5mQi1/VXgKtoI ZnXr9NRfN0lPMtHopw9Knp5zdixlC5v4mLHdNl1xci9a/6G9ELTX5oj54lkoZhwyENyziv 3Sfoj4SHiCMaLPho7N/7FvcgWoThspk= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1676484113; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UvvYXBKDbYeAaeFJSw2sbmMUgXCMs8nLK7hC02IoEZ8=; b=gtRGI/NxRHSczFOMbYaPRysWaEdFYYUqsc82t5Fv5pHBwTl41an5lVn8s/AksLrl8J2ifT tOnSjOz7HJfU3zCA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 0637A134BA; Wed, 15 Feb 2023 18:01:53 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id AtGGABEe7WPbZgAAMHmgww (envelope-from ); Wed, 15 Feb 2023 18:01:53 +0000 Message-ID: <5be9ed74-04cf-b909-9e8c-d0f38cf28501@suse.de> Date: Wed, 15 Feb 2023 19:01:52 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH] gdb: fix dealloc function not being called for frame 0 Content-Language: en-US To: Simon Marchi , gdb-patches@sourceware.org References: <20230209195037.100368-1-simon.marchi@efficios.com> From: Tom de Vries In-Reply-To: <20230209195037.100368-1-simon.marchi@efficios.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_NUMSUBJECT,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2/9/23 20:50, Simon Marchi wrote: > Tom de Vries reported [1] a regression in gdb.btrace/record_goto.exp > caused by 6d3717d4c4 ("gdb: call frame unwinders' dealloc_cache methods > through destroying the frame cache"). This issue is caught by ASan. On > a non-ASan build, it may or may not cause a crash or some other issue, I > haven't tried. > > I managed to narrow it down to: > > $ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.btrace/record_goto/record_goto -ex "start" -ex "record btrace" -ex "next" > > ... and then doing repeatedly "record goto 19" and "record goto 27". > Eventually, I get: > > (gdb) record goto 27 > ================================================================= > ==1527735==ERROR: AddressSanitizer: heap-use-after-free on address 0x6210003392a8 at pc 0x55e4c26eef86 bp 0x7ffd229f24e0 sp 0x7ffd229f24d8 > READ of size 8 at 0x6210003392a8 thread T0 > #0 0x55e4c26eef85 in bfcache_eq /home/simark/src/binutils-gdb/gdb/record-btrace.c:1639 > #1 0x55e4c37cdeff in htab_find_slot_with_hash /home/simark/src/binutils-gdb/libiberty/hashtab.c:659 > #2 0x55e4c37ce24a in htab_find_slot /home/simark/src/binutils-gdb/libiberty/hashtab.c:703 > #3 0x55e4c26ef0c6 in bfcache_new /home/simark/src/binutils-gdb/gdb/record-btrace.c:1653 > #4 0x55e4c26f1242 in record_btrace_frame_sniffer /home/simark/src/binutils-gdb/gdb/record-btrace.c:1820 > #5 0x55e4c1b926a1 in frame_unwind_try_unwinder /home/simark/src/binutils-gdb/gdb/frame-unwind.c:136 > #6 0x55e4c1b930d7 in frame_unwind_find_by_frame(frame_info_ptr, void**) /home/simark/src/binutils-gdb/gdb/frame-unwind.c:196 > #7 0x55e4c1bb867f in get_frame_type(frame_info_ptr) /home/simark/src/binutils-gdb/gdb/frame.c:2925 > #8 0x55e4c2ae6798 in print_frame_info(frame_print_options const&, frame_info_ptr, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:1049 > #9 0x55e4c2ade3e1 in print_stack_frame(frame_info_ptr, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:367 > #10 0x55e4c26fda03 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2779 > #11 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843 > #12 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169 > #13 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372 > #14 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383 > > 0x6210003392a8 is located 424 bytes inside of 4064-byte region [0x621000339100,0x62100033a0e0) > freed by thread T0 here: > #0 0x7f6ca34a5b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123 > #1 0x55e4c38a4c17 in rpl_free /home/simark/src/binutils-gdb/gnulib/import/free.c:44 > #2 0x55e4c1bbd378 in xfree /home/simark/src/binutils-gdb/gdb/../gdbsupport/gdb-xfree.h:37 > #3 0x55e4c37d1b63 in call_freefun /home/simark/src/binutils-gdb/libiberty/obstack.c:103 > #4 0x55e4c37d25a2 in _obstack_free /home/simark/src/binutils-gdb/libiberty/obstack.c:280 > #5 0x55e4c1bad701 in reinit_frame_cache() /home/simark/src/binutils-gdb/gdb/frame.c:2112 > #6 0x55e4c27705a3 in registers_changed_ptid(process_stratum_target*, ptid_t) /home/simark/src/binutils-gdb/gdb/regcache.c:564 > #7 0x55e4c27708c7 in registers_changed_thread(thread_info*) /home/simark/src/binutils-gdb/gdb/regcache.c:573 > #8 0x55e4c26fd922 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2772 > #9 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843 > #10 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169 > #11 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372 > #12 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383 > > previously allocated by thread T0 here: > #0 0x7f6ca34a5e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145 > #1 0x55e4c0b55c60 in xmalloc /home/simark/src/binutils-gdb/gdb/alloc.c:57 > #2 0x55e4c37d1a6d in call_chunkfun /home/simark/src/binutils-gdb/libiberty/obstack.c:94 > #3 0x55e4c37d1c20 in _obstack_begin_worker /home/simark/src/binutils-gdb/libiberty/obstack.c:141 > #4 0x55e4c37d1ed7 in _obstack_begin /home/simark/src/binutils-gdb/libiberty/obstack.c:164 > #5 0x55e4c1bad728 in reinit_frame_cache() /home/simark/src/binutils-gdb/gdb/frame.c:2113 > #6 0x55e4c27705a3 in registers_changed_ptid(process_stratum_target*, ptid_t) /home/simark/src/binutils-gdb/gdb/regcache.c:564 > #7 0x55e4c27708c7 in registers_changed_thread(thread_info*) /home/simark/src/binutils-gdb/gdb/regcache.c:573 > #8 0x55e4c26fd922 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2772 > #9 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843 > #10 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169 > #11 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372 > #12 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383 > > The problem is a stale entry in the bfcache hash table (in > record-btrace.c), left across a reinit_frame_cache. This entry points > to something that used to be allocated on the frame obstack, that has > since been wiped by reinit_frame_cache. > > Before the aforementioned, unwinder deallocation functions were called > by iterating on the frame chain, starting with the sentinel frame, like > so: > > /* Tear down all frame caches. */ > for (frame_info *fi = sentinel_frame; fi != NULL; fi = fi->prev) > { > if (fi->prologue_cache && fi->unwind->dealloc_cache) > fi->unwind->dealloc_cache (fi, fi->prologue_cache); > if (fi->base_cache && fi->base->unwind->dealloc_cache) > fi->base->unwind->dealloc_cache (fi, fi->base_cache); > } > > After that patch, we relied on the fact that all frames are (supposedly) > in the frame_stash. A deletion function was added to the frame_stash > hash table, so that dealloc functions would be called when emptying the > frame stash. There is one case, however, where a frame_info is not in > the frame stash. That is when we create the frame_info for the current > frame (level 0, unwound from the sentinel frame), but don't computed its computed -> compute > frame id. The computation of the frame id for that frame (and only that > frame, AFAIK) is done lazily. And putting a frame_info in the frame stash > requires knowing its id. So a frame 0 whose frame id is not computed > yet is necessarily not in the frame stash. > > When replaying with btrace, record_btrace_frame_sniffer insert entries > corresponding to frames in the "bfcache" hash table. It then relies on > record_btrace_frame_dealloc_cache being called for each frame to remove > all those entries when the frames get invalidated. If a frame reinit > happens while frame 0's id is not computed (and thefore that frame is thefore -> therefore > not in frame_stash), record_btrace_frame_dealloc_cache does not get > called for it, and it leaves a stale entry in bfcache. That then leads > to a use-after-free when that entry is accessed later, which ASan > catches. > > The proposed solution is to explicitly call frame_info_del on frame 0, > if it exists, and if its frame id is not computed. If it's frame id is it's -> its Thanks, - Tom > computed, it is expected that it will be in the frame stash, so it will > be "deleted" through that. > > Reported-By: Tom de Vries