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 05103386186A for ; Mon, 7 Sep 2020 19:19:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 05103386186A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) (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 9A3ED1E554; Mon, 7 Sep 2020 15:19:43 -0400 (EDT) Subject: Re: [PATCH v2 08/10] Avoid double free in startup_inferior To: Kamil Rytarowski , gdb-patches@sourceware.org Cc: tom@tromey.com References: <20200904002905.13616-1-n54@gmx.com> <20200904002905.13616-9-n54@gmx.com> From: Simon Marchi Message-ID: <96629e3b-c2d5-f705-f469-f5e527aab2a2@simark.ca> Date: Mon, 7 Sep 2020 15:19:43 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20200904002905.13616-9-n54@gmx.com> Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Sep 2020 19:19:46 -0000 On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote: > Do not free the last execd pathname as it will be used in > prepare_resume_reply(), after attaching a client side. Ok, so this function returns to its caller the last waitstatus. So indeed we want to clean up all the watstatus objects except the last one, which we hand over to the caller. > gdb/ChangeLog: > > * fork-inferior.c (startup_inferior): Avoid double free. > --- > gdb/ChangeLog | 4 ++++ > gdb/nat/fork-inferior.c | 5 ++++- > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index b96e7bf08e8..1013f6a0b3c 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,7 @@ > +2020-09-04 Kamil Rytarowski > + > + * fork-inferior.c (startup_inferior): Avoid double free. > + > 2020-09-04 Kamil Rytarowski > > * netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add. > diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c > index 1185ef8998b..94ab0b9cbc2 100644 > --- a/gdb/nat/fork-inferior.c > +++ b/gdb/nat/fork-inferior.c > @@ -526,7 +526,10 @@ startup_inferior (process_stratum_target *proc_target, pid_t pid, int ntraps, > > case TARGET_WAITKIND_EXECD: > /* Handle EXEC signals as if they were SIGTRAP signals. */ > - xfree (ws.value.execd_pathname); > + /* Do not free the last execd pathname as it will be used in > + prepare_resume_reply(), after attaching a client side. */ Since this is common code, let's just write it in a gdbserver or gdb agnostic way. I'd suggest something like: Free the exec'ed pathname, but only if this isn't the waitstatus we are returning to the caller. Simon