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 085D63852C5D for ; Mon, 21 Nov 2022 15:10:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 085D63852C5D 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.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 9A9A91E0D3; Mon, 21 Nov 2022 10:10:39 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1669043439; bh=8G2CgwI29ULVkDIHYCWTxLbQBuu8hOjuarb+YNz6p70=; h=Date:Subject:To:References:From:In-Reply-To:From; b=XGU8yHrcT+e4lj+mBCyzaQeIdybcqGdWWwQi0V8uNeFfENYgcEOM+Hn/vkPiFX5CZ 7UNzrREhKDw1GkQJTGZRCziuSRtklqY29LsSOFxHiy0FXwFVfc9NbvrK3+cclYrjtJ pqzprPrxaEg+mpvRcHPNGYSHsJWyFLf0hLIoiacU= Message-ID: <24e42ada-2c7a-208f-4783-972b3cb852ce@simark.ca> Date: Mon, 21 Nov 2022 10:10:39 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [RFA] Fix use after free introduced by $_hit_bpnum/$_hit_locno variables. Content-Language: en-US To: Philippe Waroquiers , gdb-patches@sourceware.org References: <20221120104406.2134561-1-philippe.waroquiers@skynet.be> From: Simon Marchi In-Reply-To: <20221120104406.2134561-1-philippe.waroquiers@skynet.be> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,SPF_HELO_PASS,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 11/20/22 05:44, Philippe Waroquiers via Gdb-patches wrote: > 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. > --- > gdb/breakpoint.c | 62 ++++++++++++++++++++++++++++-------------------- > 1 file changed, 36 insertions(+), 26 deletions(-) > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index 7f6400db624..5b691673a0e 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -4574,18 +4574,17 @@ command_line_is_silent (struct command_line *cmd) > 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 > -set_hit_convenience_vars (bpstat *bs) > +set_hit_convenience_vars (int bpnum, int locno) > { > - const struct breakpoint *b = bs->breakpoint_at; > - if (b != nullptr) > - { > - 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)); > - } > + set_internalvar_integer (lookup_internalvar ("_hit_bpnum"), bpnum); > + set_internalvar_integer (lookup_internalvar ("_hit_locno"), > + (locno > 0 ? locno : 1)); I haven't had time to look at the original patch (sorry, just so many things to do), but the naming caught my attention. We have both "num" and "no" as abbreviations for "number". It looks a bit inconsistent. Was it on purpose? I don't so much about the variable names, but the names exposed to the user. But the fix LGTM: Approved-By: Simon Marchi Simon