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 CFB4A3858CDB for ; Wed, 21 Sep 2022 15:52:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CFB4A3858CDB 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) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 43AED1E0D5; Wed, 21 Sep 2022 11:52:30 -0400 (EDT) Message-ID: Date: Wed, 21 Sep 2022 11:52:29 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v2] gdb/source.c: Fix undefined behaviour dereferencing empty string Content-Language: en-US To: Magne Hov , gdb-patches@sourceware.org References: <20220915183141.3484234-1-mhov@undo.io> <20220921145834.837969-1-mhov@undo.io> From: Simon Marchi In-Reply-To: <20220921145834.837969-1-mhov@undo.io> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.2 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 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: Wed, 21 Sep 2022 15:52:32 -0000 On 2022-09-21 10:58, Magne Hov via Gdb-patches wrote: > When a source file's dirname is solely made up of directory separators > we end up trying to dereference the last character of an empty string > with std::string::back, which results in undefined behaviour. A typical > use case where this can happen is when the root directory "/" is used as > a compilation directory. > > With libstdc++.so.6.0.28 we get no out-of-bounds checks and the byte > preceding the storage of the empty string is returned. The character > value of this byte depends on heap implementation and usage, but when > this byte happens to hold the value of the directory separator character > we go on to call std::string::pop_back on the empty string which results > in an out_of_range exception which terminates GDB. > > Fix this by using path_join. prepare_path_for_appending ensures that the > filename component is relative. > > The testsuite has been run before and after the change and no > regressions were found. > --- > gdb/source.c | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/gdb/source.c b/gdb/source.c > index 3f498d552c4..25ad1ecb3da 100644 > --- a/gdb/source.c > +++ b/gdb/source.c > @@ -1146,15 +1146,7 @@ find_and_open_source (const char *filename, > helpful if part of the compilation directory was removed, > e.g. using gcc's -fdebug-prefix-map, and we have added the missing > prefix to source_path. */ > - std::string cdir_filename (dirname); > - > - /* Remove any trailing directory separators. */ > - while (IS_DIR_SEPARATOR (cdir_filename.back ())) > - cdir_filename.pop_back (); > - > - /* Add our own directory separator. */ > - cdir_filename.append (SLASH_STRING); > - cdir_filename.append (filename_start); > + std::string cdir_filename = path_join (dirname, filename_start); > > result = openp (path, OPF_SEARCH_IN_PATH | OPF_RETURN_REALPATH, > cdir_filename.c_str (), OPEN_MODE, fullname); Thanks, this is OK, nice cleanup. Simon