From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id UCeuB0EqzWLmLBEAWB0awg (envelope-from ) for ; Tue, 12 Jul 2022 04:01:05 -0400 Received: by simark.ca (Postfix, from userid 112) id 0B6001E5EA; Tue, 12 Jul 2022 04:01:05 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=QZYEXomA; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id B57311E21F for ; Tue, 12 Jul 2022 04:01:02 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E71EF385800B for ; Tue, 12 Jul 2022 08:00:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E71EF385800B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1657612858; bh=4l+n7Pe0A+xqGFdq6aKVeH8VaZ9rwgnrhDK+EAZS/M8=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=QZYEXomAtxvS6aZDk84iJRwj8+9e8gZOz/K8Loq9rSMi1Gr/6n+ujWWyn0706MpPa dxz5LYgprkhdBf08nPjvBPh+02LTUttenc5usQjmaw9hJlOuaNxbRgWIp0d11l8e3e qYELa91+ak8zBTuluhJuIvpvOy+8Bp6t+DF11FIU= Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 58B383858413 for ; Tue, 12 Jul 2022 08:00:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 58B383858413 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 898FF1F9CE for ; Tue, 12 Jul 2022 08:00:35 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 76FB013638 for ; Tue, 12 Jul 2022 08:00:35 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id YRfwGyMqzWIVJgAAMHmgww (envelope-from ) for ; Tue, 12 Jul 2022 08:00:35 +0000 Date: Tue, 12 Jul 2022 10:00:34 +0200 To: gdb-patches@sourceware.org Subject: [PATCH][gdb/symtab] Fix out-of-bounds in objfile::section_offset Message-ID: <20220712080032.GA18344@delia.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) 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: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Hi, Using this patch in objfile::section_offset that checks that idx is within bounds: ... int idx = gdb_bfd_section_index (this->obfd, section); + gdb_assert (idx < section_offsets.size ()); return this->section_offsets[idx]; ... we run into the assert in test-cases: - gdb.base/readline-ask.exp - gdb.base/symbol-without-target_section.exp - gdb.dwarf2/dw2-icc-opaque.exp These were previously reported as -fsanitize-threads issues (PR25724, PR25723). In the case of the latter test-case the problem happens as follows. - We start out with bfd_count_sections () == 6, so gdb_bfd_count_sections () == 10. The difference of 4 is due to the 4 'special sections' named *ABS*, *UND*, *COM* and *IND*. - According to gdb_bfd_section_index, the *IND* has section index bfd_count_sections () + 3, so 9. - default_symfile_relocate gets called, which calls bfd_simple_get_relocated_section_contents and eventually this results in bfd_make_section_old_way being called for a section named COMMON, meaning now we have bfd_count_sections () == 7 - consequently the next time we call objfile::section_offset for *IND*, gdb_bfd_section_index assigns it section index 10. - the assert fails because idx == 10 and section_offsets.size () == 10. Fix this in a minimal and contained way, by: - adding a side-table orig_bfd_count_sections_map, in which we store the original bfd_count_sections () value, and - using this value in gdb_bfd_count_sections and gdb_bfd_section_index, ensuring that the creation of the new section doesn't interfere with accessing the unchanged objfile::sections and objfile::section_offsets. In case we call gdb_bfd_section_index with the new section, we assert. However, in case gdb_bfd_section_index is not used, and the bfd section index of the new section is used to access objfile::sections or objfile::section_offsets, we return some unrelated element, which might fail in some difficult to understand manner. It's hard to check whether this can happen or not without having distinct types for the different section indices (bfd vs. gdb_bfd). Anyway, if this does occur, it's a pre-existing bug. This patch focuses on getting things right for the original sections. Tested on x86_64-linux, with and without -fsanitize=threads. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29295 Any comments? Thanks, - Tom [gdb/symtab] Fix out-of-bounds in objfile::section_offset --- gdb/gdb_bfd.c | 32 +++++++++++++++++++++++++++----- gdb/objfiles.h | 2 ++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c index 22828482d5b..66b6ad1c0de 100644 --- a/gdb/gdb_bfd.c +++ b/gdb/gdb_bfd.c @@ -34,6 +34,10 @@ #include "inferior.h" #include "cli/cli-style.h" +/* The quantity bfd_count_sections can change over time. Store + the original value here. */ +static std::unordered_map orig_bfd_count_sections_map; + /* An object of this type is stored in the section's user data when mapping a section. */ @@ -730,6 +734,7 @@ gdb_bfd_unref (struct bfd *abfd) bfd_set_usrdata (abfd, NULL); /* Paranoia. */ htab_remove_elt (all_bfds, abfd); + orig_bfd_count_sections_map.erase (abfd); gdb_bfd_close_or_warn (abfd); @@ -996,21 +1001,37 @@ gdb_bfd_record_inclusion (bfd *includer, bfd *includee) gdb_static_assert (ARRAY_SIZE (_bfd_std_section) == 4); +/* The quantity bfd_count_sections can change over time. Return + the original value here. */ + +static int +get_orig_bfd_count_sections (bfd *abfd) +{ + auto it = orig_bfd_count_sections_map.find (abfd); + if (it != orig_bfd_count_sections_map.end ()) + return it->second; + int idx = bfd_count_sections (abfd); + orig_bfd_count_sections_map[abfd] = idx; + return idx; +} + /* See gdb_bfd.h. */ int gdb_bfd_section_index (bfd *abfd, asection *section) { + int count = get_orig_bfd_count_sections (abfd); if (section == NULL) return -1; else if (section == bfd_com_section_ptr) - return bfd_count_sections (abfd); + return count; else if (section == bfd_und_section_ptr) - return bfd_count_sections (abfd) + 1; + return count + 1; else if (section == bfd_abs_section_ptr) - return bfd_count_sections (abfd) + 2; + return count + 2; else if (section == bfd_ind_section_ptr) - return bfd_count_sections (abfd) + 3; + return count + 3; + gdb_assert (section->index < count); return section->index; } @@ -1019,7 +1040,8 @@ gdb_bfd_section_index (bfd *abfd, asection *section) int gdb_bfd_count_sections (bfd *abfd) { - return bfd_count_sections (abfd) + 4; + int count = get_orig_bfd_count_sections (abfd); + return count + 4; } /* See gdb_bfd.h. */ diff --git a/gdb/objfiles.h b/gdb/objfiles.h index a7098b46279..faaf97feb1f 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -598,6 +598,7 @@ struct objfile gdb_assert (section->owner == nullptr || section->owner == this->obfd); int idx = gdb_bfd_section_index (this->obfd, section); + gdb_assert (idx != -1); return this->section_offsets[idx]; } @@ -609,6 +610,7 @@ struct objfile gdb_assert (section->owner == nullptr || section->owner == this->obfd); int idx = gdb_bfd_section_index (this->obfd, section); + gdb_assert (idx != -1); this->section_offsets[idx] = offset; }