From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 96ACA385771A for ; Wed, 10 May 2023 11:50:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 96ACA385771A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.170] (unknown [167.248.160.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 09D811E114; Wed, 10 May 2023 07:50:05 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1683719406; bh=4g70QsIdpwEpRjhLVIOb2x28HvxUh7CW+OnPZMUeJ1I=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=apQbBp4/BhXDHrTUZ6zWmu4N9eQw00f62jmrt6J3x70h8UU3+00B1yI8n4wXOlvbW GS7bKTDZLzufI8xqUMA1wQKWUBBm/Nl0vqvo52iUMoodGeba9B3iRmxZSbEd5rXz7C TVezh9R+A4P8cUhO+EkCP9kMcO45bpo/fQ3FMeCA= Message-ID: Date: Wed, 10 May 2023 07:50:05 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 Subject: Re: [PATCH] gdb: fix use-after-free in check_longjmp_breakpoint_for_call_dummy To: Andrew Burgess , Simon Marchi via Gdb-patches Cc: Simon Marchi References: <20230508145913.29359-1-simon.marchi@efficios.com> <87h6sk7doe.fsf@redhat.com> Content-Language: fr From: Simon Marchi In-Reply-To: <87h6sk7doe.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 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 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