From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102d.google.com (mail-pj1-x102d.google.com [IPv6:2607:f8b0:4864:20::102d]) by sourceware.org (Postfix) with ESMTPS id BB44A3858033 for ; Thu, 9 Feb 2023 10:14:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BB44A3858033 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-x102d.google.com with SMTP id v18-20020a17090ae99200b00230f079dcd9so5490396pjy.1 for ; Thu, 09 Feb 2023 02:14:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=rqdWiLJeuqAvSkfhyOJgi48VW7E9zdY4qfgAVmnDnE4=; b=gOZlUMgWAhgXn0YYf7KB4kntl44oAB0NWTg0J3+MPx3XcdjRuhrLSEwXH6s0ORHMDh 66t54afEvxfs4rbK3RbRe8HcXu2JDAJkLe/MNDu98SGfs7G1KI8ZDj1gFVzVXfTrVFbb viMKv6yeoAOzzTg9ouzJJ6Abyti1UC1H244qKoOavlAkuUgEYsSdUUKq8pltfXTBnUqt sFgO4Zt0eZliDWuR9iVRPTFePlGoGTJfXS7dnQ6qoqXjyyozMol5hCnD7/bCu4/oEue/ U9wMvuI1Wkh3GNtI7XMwF1gz5SNY3zI8YhIm5uToxvnt2M7NCIGB8y0o0aoKngJhg/re ypYQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=rqdWiLJeuqAvSkfhyOJgi48VW7E9zdY4qfgAVmnDnE4=; b=cqkPmAnJcXvDahXBNxUzHdTIN31HlHHXr1VRGNOHTjs37x3Ql9sejvg9fg938CxenJ C4KDYoiLsM9GWbPXkEnvoEAjv4IexryMLQyRP51S2ZjfqEMQ58cbD4cKkkKi9HH8zzmL MYr0Cl0RAfBd6QDSB7cYk2Pjg2j6gkcorDhCYVL9ZVDLv84em3Zu0dt4DRkEH5Tu4YCe CeSxwT45CNJy/RuPcbXVn9VQRHPnjibwJihIw03k6zBPGimBurfF5jp6QfHe3+G0ZANn MJFkZy0i52xH23TtfQGtaqyhfIAtS96rMCKvbW8ra5Ab3TYIofRfjyACJhnj/MwlaRe9 jEiA== X-Gm-Message-State: AO0yUKWR7fg5FfCj//IfElnM9g5AY+BmE0hj3EayRdc4ZdRdwUQ4cyOF b1DMDKtsehV38pOXtObemYZJdY9/Zks= X-Google-Smtp-Source: AK7set9+Nb52S3Ge9N9wbnH9uV7WMqOtxDDJhkLeoIkd3T8mR+/6TwHI17IGTNzSo9rdsfF6ZzK3Sw== X-Received: by 2002:a17:902:ebcf:b0:199:2236:ae63 with SMTP id p15-20020a170902ebcf00b001992236ae63mr9014025plg.2.1675937651749; Thu, 09 Feb 2023 02:14:11 -0800 (PST) Received: from squeak.grove.modra.org ([2406:3400:51d:8cc0:2c19:e063:cb3b:da4]) by smtp.gmail.com with ESMTPSA id c1-20020a170902d48100b001966308ca0dsm1084071plg.167.2023.02.09.02.14.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Feb 2023 02:14:11 -0800 (PST) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id 0D46B11404B1; Thu, 9 Feb 2023 20:44:08 +1030 (ACDT) Date: Thu, 9 Feb 2023 20:44:08 +1030 From: Alan Modra To: "Maciej W. Rozycki" Cc: binutils@sourceware.org, Chenghua Xu Subject: Re: Protect mips_hi16_list from fuzzed debug info Message-ID: References: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="5lMWLy6u4HkqbMgH" Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-3035.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,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 List-Id: --5lMWLy6u4HkqbMgH Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Feb 09, 2023 at 01:26:11AM +0000, Maciej W. Rozycki wrote: > On Thu, 9 Feb 2023, Alan Modra wrote: > > If it is really true that hi16/lo16 pairs are always in the same > > section, then we wouldn't need freeze_mips_hi16_list. > > It is, by definition[1]: > > "The AHL addend is a composite computed from the addends of two > consecutive relocation entries. Each relocation type of R_MIPS_HI16 must > have an associated R_MIPS_LO16 entry immediately following it in the list > of relocations. > > "These relocation entries are always processed as a pair and both addend > fields contribute to the AHL addend. If AHI and ALO are the addends from > the paired R_MIPS_HI16 and R_MIPS_LO16 entries, then the addend AHL is com > puted as (AHI << 16) + (short)ALO. R_MIPS_LO16 entries without an > R_MIPS_HI16 entry immediately preceding are orphaned and the previously > defined R_MIPS_HI16 is used for computing the addend." > > Two consecutive entries must come from a single .rel.* section or they > wouldn't be consecutive (there's no relationship between different .rel.* > sections). Yes, but see the comment before _bfd_mips_elf_hi16_reloc The ABI requires that the *LO16 immediately follow the *HI16. However, as a GNU extension, we permit an arbitrary number of *HI16s to be associated with a single *LO16. This significantly simplies the relocation handling in gcc. > While not explicitly stated I don't think anyone considered reusing an > R_MIPS_HI16 reloc defined in another relocation section for an orphaned > R_MIPS_LO16 reloc. That would IMO make no semantic sense (for instance > you can shuffle sections via a linker script in a relocatable link) and I > think we ought not strive for doing so (i.e. there is not supposed to be > any "previously defined R_MIPS_HI16" at the beginning of any given reloc > section). > > Of course this consideration applies to the REL format only (i.e. o32); > there's no need to track HI16/LO16 pairing with RELA objects as the addend > is readily available. AFAICT we don't get this right either: for > `!partial_inplace' howtos we need not use `_bfd_mips_elf_hi16_reloc' or > `_bfd_mips_elf_lo16_reloc' and just `_bfd_mips_elf_generic_reloc' will do. > > > Instead it > > would be much better to attach the list to mips_elf_section_data. I > > wasn't sure enough to do that, given things like gcc's hot/cold > > section partitioning, when I moved the mip_hi16_list to be per-bfd. > > That sounds like a plan to me, but it's unrealistic for me to commit to > it in the next two days (and then I head out to California for a week). Patch attached. Note the FIXME. > References: > > [1] "SYSTEM V APPLICATION BINARY INTERFACE, MIPS RISC Processor > Supplement, 3rd Edition", Section "Relocation", pp. 4-17, 4-18 -- Alan Modra Australia Development Lab, IBM --5lMWLy6u4HkqbMgH Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0002-Move-mips_hi16_list-to-mips_elf_section_data.patch" >From 4febdc8426f3b834bc5bb5a9e6cc67f2315018c4 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Thu, 9 Feb 2023 14:24:49 +1030 Subject: Move mips_hi16_list to mips_elf_section_data * elfxx-mips.c (struct mips_hi16): Move earlier, deleting input_section and data fields. (struct _mips_elf_section_data): Add mips_hi16_list. (struct mips_elf_obj_tdata): Delete mips_hi16_list. (free_mips_hi16_list): New function. (_bfd_mips_elf_close_and_cleanup): Adjust to suit new location of mips_hi16_list. (_bfd_mips_elf_hi16_reloc, _bfd_mips_elf_lo16_reloc): Likewise. (_bfd_elf_mips_get_relocated_section_contents): Likewise. diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c index e9fb61ff9e7..8dcfba3c27e 100644 --- a/bfd/elfxx-mips.c +++ b/bfd/elfxx-mips.c @@ -222,6 +222,15 @@ struct mips_elf_traverse_got_arg int value; }; +/* Used to store a REL high-part relocation such as R_MIPS_HI16 or + R_MIPS_GOT16. */ + +struct mips_hi16 +{ + struct mips_hi16 *next; + arelent rel; +}; + struct _mips_elf_section_data { struct bfd_elf_section_data elf; @@ -229,6 +238,8 @@ struct _mips_elf_section_data { bfd_byte *tdata; } u; + + struct mips_hi16 *mips_hi16_list; }; #define mips_elf_section_data(sec) \ @@ -549,19 +560,6 @@ struct mips_htab_traverse_info bool error; }; -/* Used to store a REL high-part relocation such as R_MIPS_HI16 or - R_MIPS_GOT16. REL is the relocation, INPUT_SECTION is the section - that contains the relocation field and DATA points to the start of - INPUT_SECTION. */ - -struct mips_hi16 -{ - struct mips_hi16 *next; - bfd_byte *data; - asection *input_section; - arelent rel; -}; - /* MIPS ELF private object data. */ struct mips_elf_obj_tdata @@ -597,8 +595,6 @@ struct mips_elf_obj_tdata asymbol *elf_text_symbol; asection *elf_data_section; asection *elf_text_section; - - struct mips_hi16 *mips_hi16_list; }; /* Get MIPS ELF private object data from BFD's tdata. */ @@ -1380,19 +1376,37 @@ _bfd_mips_elf_mkobject (bfd *abfd) MIPS_ELF_DATA); } +/* Free the mips_hi16_list attached to S. Return true if there were + unmatched hi16 relocs. */ + +static bool +free_mips_hi16_list (asection *s) +{ + struct mips_hi16 *hi; + struct mips_hi16 **hip = &mips_elf_section_data (s)->mips_hi16_list; + bool ret = *hip != NULL; + + while ((hi = *hip) != NULL) + { + *hip = hi->next; + free (hi); + } + return ret; +} + bool _bfd_mips_elf_close_and_cleanup (bfd *abfd) { - struct mips_elf_obj_tdata *tdata = mips_elf_tdata (abfd); - if (tdata != NULL && bfd_get_format (abfd) == bfd_object) + if (bfd_get_format (abfd) == bfd_object) { - BFD_ASSERT (tdata->root.object_id == MIPS_ELF_DATA); - while (tdata->mips_hi16_list != NULL) - { - struct mips_hi16 *hi = tdata->mips_hi16_list; - tdata->mips_hi16_list = hi->next; - free (hi); - } + for (asection *s = abfd->sections; s; s = s->next) + if (free_mips_hi16_list (s) && 0) + /* FIXME: Disabled until _bfd_mips_elf_got16_reloc is + taught that vxworks does not pair the got16 relocs with + lo16 relocs, and thus should not put them on this list. + See gas/config/tc-mips.c reloc_needs_lo_p. */ + _bfd_error_handler + (_("%pB(%pA): unmatched hi16 reloc"), abfd, s); } return _bfd_elf_close_and_cleanup (abfd); } @@ -2524,26 +2538,22 @@ _bfd_mips_elf_gprel16_with_gp (bfd *abfd, asymbol *symbol, bfd_reloc_status_type _bfd_mips_elf_hi16_reloc (bfd *abfd, arelent *reloc_entry, - asymbol *symbol ATTRIBUTE_UNUSED, void *data, + asymbol *symbol ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED, asection *input_section, bfd *output_bfd, char **error_message ATTRIBUTE_UNUSED) { - struct mips_hi16 *n; - struct mips_elf_obj_tdata *tdata; - if (reloc_entry->address > bfd_get_section_limit (abfd, input_section)) return bfd_reloc_outofrange; - n = bfd_malloc (sizeof *n); + struct mips_hi16 *n = bfd_malloc (sizeof *n); if (n == NULL) return bfd_reloc_outofrange; - tdata = mips_elf_tdata (abfd); - n->next = tdata->mips_hi16_list; - n->data = data; - n->input_section = input_section; + struct _mips_elf_section_data *sdata = mips_elf_section_data (input_section); + n->next = sdata->mips_hi16_list; n->rel = *reloc_entry; - tdata->mips_hi16_list = n; + sdata->mips_hi16_list = n; if (output_bfd != NULL) reloc_entry->address += input_section->output_offset; @@ -2583,7 +2593,6 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol, { bfd_vma vallo; bfd_byte *location = (bfd_byte *) data + reloc_entry->address; - struct mips_elf_obj_tdata *tdata; if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd, input_section, reloc_entry->address)) @@ -2595,13 +2604,11 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol, _bfd_mips_elf_reloc_shuffle (abfd, reloc_entry->howto->type, false, location); - tdata = mips_elf_tdata (abfd); - while (tdata->mips_hi16_list != NULL) + struct _mips_elf_section_data *sdata = mips_elf_section_data (input_section); + while (sdata->mips_hi16_list != NULL) { bfd_reloc_status_type ret; - struct mips_hi16 *hi; - - hi = tdata->mips_hi16_list; + struct mips_hi16 *hi = sdata->mips_hi16_list; /* R_MIPS*_GOT16 relocations are something of a special case. We want to install the addend in the same way as for a R_MIPS*_HI16 @@ -2619,13 +2626,13 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol, carry or borrow will induce a change of +1 or -1 in the high part. */ hi->rel.addend += (vallo + 0x8000) & 0xffff; - ret = _bfd_mips_elf_generic_reloc (abfd, &hi->rel, symbol, hi->data, - hi->input_section, output_bfd, + ret = _bfd_mips_elf_generic_reloc (abfd, &hi->rel, symbol, data, + input_section, output_bfd, error_message); if (ret != bfd_reloc_ok) return ret; - tdata->mips_hi16_list = hi->next; + sdata->mips_hi16_list = hi->next; free (hi); } @@ -13314,24 +13321,8 @@ _bfd_elf_mips_get_relocated_section_contents reloc_vector = (arelent **) bfd_malloc (reloc_size); if (reloc_vector == NULL) { - struct mips_elf_obj_tdata *tdata; - struct mips_hi16 **hip, *hi; error_return: - /* If we are going to return an error, remove entries on - mips_hi16_list that point into this section's data. Data - will typically be freed on return from this function. */ - tdata = mips_elf_tdata (abfd); - hip = &tdata->mips_hi16_list; - while ((hi = *hip) != NULL) - { - if (hi->input_section == input_section) - { - *hip = hi->next; - free (hi); - } - else - hip = &hi->next; - } + free_mips_hi16_list (input_section); if (orig_data == NULL) free (data); data = NULL; --5lMWLy6u4HkqbMgH--