From patchwork Mon Jan 6 14:27:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shahab Vahedi X-Patchwork-Id: 37211 Received: (qmail 62850 invoked by alias); 6 Jan 2020 14:27:59 -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 62842 invoked by uid 89); 6 Jan 2020 14:27:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=filled, threw, HX-Spam-Relays-External:209.85.128.66, H*RU:209.85.128.66 X-HELO: mail-wm1-f66.google.com Received: from mail-wm1-f66.google.com (HELO mail-wm1-f66.google.com) (209.85.128.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 06 Jan 2020 14:27:57 +0000 Received: by mail-wm1-f66.google.com with SMTP id p17so15468180wma.1 for ; Mon, 06 Jan 2020 06:27:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=o8eTQ9dql7dOehFXltCnEk1rJtiVLPgn2VfzAbqj5fY=; b=kWSZmZioiPiD/VKchQ/MqQzvV+9KKcieawNYTtgumNvlXo3OnZouiv7TgyRiJGDcVf 9MZEnndIeOeebLnTjOWhfYdlghUc4b8UTl3lWkRg5T8A7ZaYXxN1LXAPUvWDBuhSq5hw qB47qrBzC2R76a+CWpwLOXXfEAh1/3U1LUyQYL/TEOjgoXPA+EeskS9FMwD6Z7haPfd4 XuIKVd9O7pXxt5wZhdTX0OU+dIpSsuYHXDjG5J5MUaTpYYVDHnTUyiQmGLwiKbxyRT9Q jkZ9LuCI0099+hh3NDhOuI7ZlM/IMlQQE7EuRhGRFksBRNFt/EzW440fFSle4mfJYn4q pJzA== Return-Path: Received: from localhost.localdomain (ip-217-103-128-141.ip.prioritytelecom.net. [217.103.128.141]) by smtp.gmail.com with ESMTPSA id r62sm24150255wma.32.2020.01.06.06.27.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Jan 2020 06:27:54 -0800 (PST) From: Shahab Vahedi To: gdb-patches@sourceware.org Cc: Shahab Vahedi , Pedro Alves , Andrew Burgess , Claudiu Zissulescu , Francois Bedard Subject: [PATCH v3] GDB: Fix the overflow in addr/line_is_displayed() Date: Mon, 6 Jan 2020 15:27:32 +0100 Message-Id: <20200106142732.32733-1-shahab.vahedi@gmail.com> In-Reply-To: <45a718f7-e905-e7b1-1596-6ef6c4204176@redhat.com> References: <45a718f7-e905-e7b1-1596-6ef6c4204176@redhat.com> MIME-Version: 1.0 From: Shahab Vahedi In tui_disasm_window::addr_is_displayed(), there can be situations where "content" is empty. For instance, it can happen when the "content" was not filled in tui_disasm_window::set_contents(), because tui_disassemble() threw an exception. Usually this exception is the result of fetching invalid PC addresses like the ones beyond the end of the program. Having "content.size ()" zero leads to an overflow in this condition check inside tui_disasm_window::addr_is_displayed(): int i = 0; while (i < content.size () - threshold ...) { ... content[i] ... } "threshold" is 2 and there are times that "content.size ()" is 0. This results into an overflow and the loop is entered whereas it should have been skipped. Finally, "content[i]" access leads to a segmentation fault. Same problem applies to tui_source_window::line_is_displayed(). The issue has been discussed at length in bug 25345: https://sourceware.org/bugzilla/show_bug.cgi?id=25345 This commit avoids the segmentation faults with an early check: if (contet.size () < SCROLL_THRESHOLD) return false; Moreover, those functions have been overhauled to a leaner code. gdb/ChangeLog: 2020-01-06 Shahab Vahedi * tui/tui-disasm.c (tui_disasm_window::addr_is_displayed): Avoid overflow by an early check of content vs threshold. * tui/tui-source.c (tui_source_window::line_is_displayed): Likewise. --- gdb/tui/tui-disasm.c | 16 +++++++--------- gdb/tui/tui-source.c | 17 ++++++++--------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c index ebd0ba317f5..98c691f3387 100644 --- a/gdb/tui/tui-disasm.c +++ b/gdb/tui/tui-disasm.c @@ -348,19 +348,17 @@ tui_disasm_window::location_matches_p (struct bp_location *loc, int line_no) bool tui_disasm_window::addr_is_displayed (CORE_ADDR addr) const { - bool is_displayed = false; - int threshold = SCROLL_THRESHOLD; + if (content.size () < SCROLL_THRESHOLD) + return false; - int i = 0; - while (i < content.size () - threshold && !is_displayed) + for (size_t i = 0; i < content.size () - SCROLL_THRESHOLD; ++i) { - is_displayed - = (content[i].line_or_addr.loa == LOA_ADDRESS - && content[i].line_or_addr.u.addr == addr); - i++; + if (content[i].line_or_addr.loa == LOA_ADDRESS + && content[i].line_or_addr.u.addr == addr) + return true; } - return is_displayed; + return false; } void diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c index e028b724d23..1503cd4c636 100644 --- a/gdb/tui/tui-source.c +++ b/gdb/tui/tui-source.c @@ -174,18 +174,17 @@ tui_source_window::location_matches_p (struct bp_location *loc, int line_no) bool tui_source_window::line_is_displayed (int line) const { - bool is_displayed = false; - int threshold = SCROLL_THRESHOLD; - int i = 0; - while (i < content.size () - threshold && !is_displayed) + if (content.size () < SCROLL_THRESHOLD) + return false; + + for (size_t i = 0; i < content.size () - SCROLL_THRESHOLD; ++i) { - is_displayed - = (content[i].line_or_addr.loa == LOA_LINE - && content[i].line_or_addr.u.line_no == line); - i++; + if (content[i].line_or_addr.loa == LOA_LINE + && content[i].line_or_addr.u.line_no == line) + return true; } - return is_displayed; + return false; } void