From patchwork Thu May 23 19:28:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 32834 Received: (qmail 37227 invoked by alias); 23 May 2019 19:28:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 37215 invoked by uid 89); 23 May 2019 19:28:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy=H*M:a8e6 X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 May 2019 19:28:49 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 2AFB2AD47; Thu, 23 May 2019 19:28:47 +0000 (UTC) Subject: Re: [Bug-readline] heap-buffer-overflow in update_line To: chet.ramey@case.edu, bug-readline@gnu.org Cc: gdb-patches@sourceware.org, Pedro Alves References: <52f237e9-83e8-2a97-4766-e60b867ab914@suse.de> <79173bd4-f37e-c137-cf48-187047078bf0@suse.de> From: Tom de Vries Message-ID: <17c0fa1d-7df4-5204-a8e6-104239d66e3c@suse.de> Date: Thu, 23 May 2019 21:28:45 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes On 23-05-19 14:38, Chet Ramey wrote: > On 5/23/19 3:33 AM, Tom de Vries wrote: > >> Using this additional bit: >> ... >> @@ -528,6 +533,8 @@ rl_redisplay () >> init_line_structures (0); >> rl_on_new_line (); >> } >> + else if (line_size <= _rl_screenwidth) >> + init_line_structures (_rl_screenwidth + 1); >> >> /* Draw the line into the buffer. */ >> cpos_buffer_position = -1; >> ... >> I managed to fix the assert also in this scenario, and managed to run >> the entire gdb testsuite without triggering the assert. >> >> Is that a good code change? > > It looks like it will solve that problem, and perhaps more. Thanks for the > patch. I did a further test-run to see if the original problem (PR24514 - heap-buffer-overflow in update_line for utf8-identifiers.exp: https://sourceware.org/bugzilla/show_bug.cgi?id=24514 ) was fixed, which turned out not to be the case. I've analyzed this, and found it to be caused by the init_line_structures part of the patch changing line_size, which does not force a reallloc. I've fixed this by changing minsize instead. Attached patch passes gdb regression testsuite with the assert enabled, and with gdb build with -fsanitize=address. Thanks, - Tom tentative --- readline/display.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/readline/display.c b/readline/display.c index 712ca79b6d..f34ad05444 100644 --- a/readline/display.c +++ b/readline/display.c @@ -452,6 +452,9 @@ init_line_structures (minsize) { register int n; + if (minsize <= _rl_screenwidth) /* XXX - for gdb */ + minsize = _rl_screenwidth + 1; + if (invisible_line == 0) /* initialize it */ { if (line_size < minsize) @@ -528,6 +531,8 @@ rl_redisplay () init_line_structures (0); rl_on_new_line (); } + else if (line_size <= _rl_screenwidth) + init_line_structures (_rl_screenwidth + 1); /* Draw the line into the buffer. */ cpos_buffer_position = -1;