From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x52c.google.com (mail-pg1-x52c.google.com [IPv6:2607:f8b0:4864:20::52c]) by sourceware.org (Postfix) with ESMTPS id F03223858439 for ; Tue, 19 Dec 2023 09:36:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F03223858439 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org F03223858439 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::52c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702978577; cv=none; b=oU9LqYzaCP7i/M1MmOnFeqIjJhIlY1Ai20VNMCzqggrfQ6CRMFKH3rAPOfulrIssQQ5ZFNPaUW1u9RRH2f+5fJpqyuMghZ/AtEEdpnqdlfWVeYKDToBIb7wWbmChJGkCbxGrwgoBlX4ER+8NhtlI/PvwKINh9Ib9WcW65OD4pTU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702978577; c=relaxed/simple; bh=F2GTCp4E1k9Xy3zc52fmUfYQND1AmurwgM2NuFMNbpc=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=FyShkzm7JWKyruQDLpdJH32PY4R1GUXRtN5kwVKQUXvI+Qskzkfo1DaZpmfdvokl48OjS4GYp7KFZsX6a55W2JpZNj8gh3HhAj3KfXU2emB7yIPSx2s5YgppiFfuvUPq4nYeQd4KMwZV+d/i0afgAWsK+il0NFCgA5gvzOxu1qs= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pg1-x52c.google.com with SMTP id 41be03b00d2f7-517ab9a4a13so3268309a12.1 for ; Tue, 19 Dec 2023 01:36:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702978573; x=1703583373; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=u1VxugszV5fclbrSa5raxzncRib3Tkh7UIvm9yXeIVE=; b=cGRn6G7EjtdvJIqjEUcfjoCyYojoYqqp+Oqhvo38++5wUPN1muIm1KknAEjbwYAVU5 O2uvo9zraxwjlBSv0p7tLqyLXir0T+7Uoj9sri9W+q6KeSHxO2nB2WlfNBPuXhnhVBag ZKz/x0YL5/fjIgO8gfBQ6Fkju/C2izZuexYdNdG6fIwBoxQI/Q6RPDcxD+/2bH6iQ/NQ UPMIPBWiqYRiIVTedFNg72C/xNJuTdhlO43ASkev7blAVJeOiffij0ZAxsHAXaDhp5nv QRGa8h4ttzRvFqH1PtbHrjzhCpPDESnxAbduOJCgDCfhH+cz7MzW5E+gkFbIB6g8vRr1 cG/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702978573; x=1703583373; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=u1VxugszV5fclbrSa5raxzncRib3Tkh7UIvm9yXeIVE=; b=Ga855Rc6I7dfX80XF2dUl0MiJa8Hc1NZ/0XhghuizAi/mRBZV279Dk/U3IRDAZ2/Cs /v7weDQAmWphR2L2sC9+50kD+4zbsNLmeYoLDahCQQPaItYMDxHTI5D6fEsFA6x3UNbi eYV9hhawSoLir2XXoWcTK861AxJUSAwq8H55czR3yIREO/ksIn5AmUUERGGdRulF9RwZ AOGBL8j9RmS5EsdfexAby/lcA74umuv1H36EFIpy8zy3r6CeYJigKhDPcxWs9OvTsb3C K9XVW0baku1b7cDh9faK76ksJQJKYzg+33yKh8RCEfAPszuZs7FmGzebIeE4oldAy9A4 EHPg== X-Gm-Message-State: AOJu0YyfVByofznhgFU0hVpD1Bm+guR9dNcbcY6RL0ZYoExrPtChe9bt 8ivL3JOfYzBSM69dFxbzCGHFTD22pCo= X-Google-Smtp-Source: AGHT+IH4590ehJbuBiBba93F6At8YrFNsJneD182DycXQTNIshmjHAZggTPwfEPEURCAxW/axrc4xA== X-Received: by 2002:a05:6a20:2926:b0:18f:97c:8a4f with SMTP id t38-20020a056a20292600b0018f097c8a4fmr18129985pzf.122.1702978572937; Tue, 19 Dec 2023 01:36:12 -0800 (PST) Received: from squeak.. ([2406:3400:51d:8cc0:f25f:eeff:1565:a817]) by smtp.gmail.com with ESMTPSA id t8-20020a1709028c8800b001c5b8087fe5sm20607075plo.94.2023.12.19.01.36.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Dec 2023 01:36:12 -0800 (PST) From: Alan Modra To: binutils@sourceware.org Cc: "Maciej W . Rozycki" , Chenghua Xu , Alan Modra Subject: Move mips_hi16_list to mips_elf_section_data Date: Tue, 19 Dec 2023 20:05:45 +1030 Message-Id: <20231219093546.2112095-2-amodra@gmail.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231219093546.2112095-1-amodra@gmail.com> References: <20231219093546.2112095-1-amodra@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3032.7 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,T_SCC_BODY_TEXT_LINE 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: This patch is in response to fuzzing testcases that manage to cause segfaults due to stale references to freed memory via mips_hi16.data. A number of the error/warning handlers in ldmain.c use %C. This can cause debug info to be parsed for the first time in order to print file/function/line. If one of those warnings is triggered after some hi16 relocs have been processed but before the matching lo16 reloc is handled, *and* the debug info is corrupted with a lo16 reloc, then the mips_hi16_list will be flushed with the result that printing a warning changes linker output. It is also possible that corrupted debug info adds to the hi16 list, with the result that when the linker handles a later lo16 reloc in a text section, ld will segfault accessing mips_hi16.data after the debug buffers have be freed. Both of these problems are fixed by keeping a per-section mips_hi16_list rather than a per-file list. * elfxx-mips.c (struct mips_hi16): Move earlier, deleting input_section field. (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_free_cached_info): 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 bae8622fd34..00111553c30 100644 --- a/bfd/elfxx-mips.c +++ b/bfd/elfxx-mips.c @@ -222,6 +222,19 @@ 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. DATA nominally points to the start of the section + contents, but note that gas may use multiple chunks of memory for a + section (with DATA + [offset,offset+frag_size) addressing a given + frag). A hi16 reloc might need a different "data" to a lo16. */ + +struct mips_hi16 +{ + struct mips_hi16 *next; + bfd_byte *data; + arelent rel; +}; + struct _mips_elf_section_data { struct bfd_elf_section_data elf; @@ -229,6 +242,8 @@ struct _mips_elf_section_data { bfd_byte *tdata; } u; + + struct mips_hi16 *mips_hi16_list; }; #define mips_elf_section_data(sec) \ @@ -549,19 +564,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 +599,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. */ @@ -1389,6 +1389,30 @@ struct mips_elf_find_line struct ecoff_find_line i; }; +/* 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 = false; + + while ((hi = *hip) != NULL) + { + *hip = hi->next; + /* See gas/config/tc-mips.c reloc_needs_lo_p. Not all hi16 + relocs need lo16 relocs. */ + if (hi->rel.howto->type == R_MIPS_HI16 + || hi->rel.howto->type == R_MIPS16_HI16 + || hi->rel.howto->type == R_MICROMIPS_HI16) + ret = true; + free (hi); + } + return ret; +} + bool _bfd_mips_elf_free_cached_info (bfd *abfd) { @@ -1399,14 +1423,12 @@ _bfd_mips_elf_free_cached_info (bfd *abfd) && (tdata = mips_elf_tdata (abfd)) != NULL) { 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); - } if (tdata->find_line_info != NULL) _bfd_ecoff_free_ecoff_debug_info (&tdata->find_line_info->d); + for (asection *s = abfd->sections; s; s = s->next) + if (free_mips_hi16_list (s)) + _bfd_error_handler + (_("%pB(%pA): unmatched hi16 reloc"), abfd, s); } return _bfd_elf_free_cached_info (abfd); } @@ -2537,22 +2559,18 @@ _bfd_mips_elf_hi16_reloc (bfd *abfd, arelent *reloc_entry, 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; + struct _mips_elf_section_data *sdata = mips_elf_section_data (input_section); + n->next = sdata->mips_hi16_list; n->data = data; - n->input_section = input_section; 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; @@ -2590,64 +2608,66 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol, void *data, asection *input_section, bfd *output_bfd, char **error_message) { - bfd_vma vallo; - bfd_byte *location = (bfd_byte *) data + reloc_entry->address; - struct mips_elf_obj_tdata *tdata; + struct _mips_elf_section_data *sdata = mips_elf_section_data (input_section); - if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd, input_section, - reloc_entry->address)) - return bfd_reloc_outofrange; + if (sdata->mips_hi16_list != NULL) + { + if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd, input_section, + reloc_entry->address)) + return bfd_reloc_outofrange; - _bfd_mips_elf_reloc_unshuffle (abfd, reloc_entry->howto->type, false, - location); - /* The high 16 bits of the addend are stored in the high insn, the - low 16 bits in the low insn, but there is a catch: You can't - just concatenate the high and low parts. The high part of the - addend is adjusted for the fact that the low part is sign - extended. For example, an addend of 0x38000 would have 0x0004 in - the high part and 0x8000 (=0xff..f8000) in the low part. - To extract the actual addend, calculate (a) - ((hi & 0xffff) << 16) + ((lo & 0xffff) ^ 0x8000) - 0x8000. - We will be applying (symbol + addend) & 0xffff to the low insn, - and we want to apply (b) (symbol + addend + 0x8000) >> 16 to the - high insn (the +0x8000 adjusting for when the applied low part is - negative). Substituting (a) into (b) and recognising that - (hi & 0xffff) is already in the high insn gives a high part - addend adjustment of (lo & 0xffff) ^ 0x8000. */ - vallo = (bfd_get_32 (abfd, location) & 0xffff) ^ 0x8000; - _bfd_mips_elf_reloc_shuffle (abfd, reloc_entry->howto->type, false, - location); + bfd_byte *location = (bfd_byte *) data + reloc_entry->address; + _bfd_mips_elf_reloc_unshuffle (abfd, reloc_entry->howto->type, false, + location); + /* The high 16 bits of the addend are stored in the high insn, the + low 16 bits in the low insn, but there is a catch: You can't + just concatenate the high and low parts. The high part of the + addend is adjusted for the fact that the low part is sign + extended. For example, an addend of 0x38000 would have 0x0004 in + the high part and 0x8000 (=0xff..f8000) in the low part. + To extract the actual addend, calculate (a) + ((hi & 0xffff) << 16) + ((lo & 0xffff) ^ 0x8000) - 0x8000. + We will be applying (symbol + addend) & 0xffff to the low insn, + and we want to apply (b) (symbol + addend + 0x8000) >> 16 to the + high insn (the +0x8000 adjusting for when the applied low part is + negative). Substituting (a) into (b) and recognising that + (hi & 0xffff) is already in the high insn gives a high part + addend adjustment of (lo & 0xffff) ^ 0x8000. */ + bfd_vma vallo = (bfd_get_32 (abfd, location) & 0xffff) ^ 0x8000; + _bfd_mips_elf_reloc_shuffle (abfd, reloc_entry->howto->type, false, + location); - tdata = mips_elf_tdata (abfd); - while (tdata->mips_hi16_list != NULL) - { - bfd_reloc_status_type ret; - struct mips_hi16 *hi; - - hi = tdata->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 - relocation (with a rightshift of 16). However, since GOT16 - relocations can also be used with global symbols, their howto - has a rightshift of 0. */ - if (hi->rel.howto->type == R_MIPS_GOT16) - hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS_HI16, false); - else if (hi->rel.howto->type == R_MIPS16_GOT16) - hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS16_HI16, false); - else if (hi->rel.howto->type == R_MICROMIPS_GOT16) - hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MICROMIPS_HI16, false); - - hi->rel.addend += vallo; - - ret = _bfd_mips_elf_generic_reloc (abfd, &hi->rel, symbol, hi->data, - hi->input_section, output_bfd, - error_message); - if (ret != bfd_reloc_ok) - return ret; - - tdata->mips_hi16_list = hi->next; - free (hi); + while (sdata->mips_hi16_list != NULL) + { + bfd_reloc_status_type ret; + 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 relocation (with a rightshift of 16). + However, since GOT16 relocations can also be used with + global symbols, their howto has a rightshift of 0. */ + if (hi->rel.howto->type == R_MIPS_GOT16) + hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS_HI16, + false); + else if (hi->rel.howto->type == R_MIPS16_GOT16) + hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS16_HI16, + false); + else if (hi->rel.howto->type == R_MICROMIPS_GOT16) + hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MICROMIPS_HI16, + false); + + hi->rel.addend += vallo; + + ret = _bfd_mips_elf_generic_reloc (abfd, &hi->rel, symbol, hi->data, + input_section, output_bfd, + error_message); + if (ret != bfd_reloc_ok) + return ret; + + sdata->mips_hi16_list = hi->next; + free (hi); + } } return _bfd_mips_elf_generic_reloc (abfd, reloc_entry, symbol, data, @@ -13333,24 +13353,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;