From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1034.google.com (mail-pj1-x1034.google.com [IPv6:2607:f8b0:4864:20::1034]) by sourceware.org (Postfix) with ESMTPS id ACE813858C52 for ; Sat, 20 May 2023 11:41:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ACE813858C52 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-x1034.google.com with SMTP id 98e67ed59e1d1-2533d74895bso3281855a91.0 for ; Sat, 20 May 2023 04:41:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684582872; x=1687174872; 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=WS6V+8rWxdpYyJSfmHUdwqs1siiSRdR3bQRv9LKaPtA=; b=Rvt12ujvYBMZzOFyKaGpPYU7798rkvnBdsgGmhynIw/owoNED1oIqxahU6PVRuIK0/ N/rpouVFtypTVU02Nrr6DkkoBVhvx4ZdcM+NaT4nib+rlTjbm8xyPidyFsm7/KKHbaDG vLdn0oqsPdErzLChxsvSKJO/1yIfZSwBuCjliBiBNKdUIt7R32tgwdT9lzuLPYoOZz4h DIai+LQWvAaRGhUneWD7MZxEv8f0fjeKtrf6kczZVFY5hvAeZCmlA1tCFw04gaqe69a0 M5WVYYMh5GYU2ua3p+LU+0Zzrf7y2Ygzd3Z3Y6+yDD31S5UMMoJ4V0m5fi/aJHtnxCBz YOBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684582872; x=1687174872; 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=WS6V+8rWxdpYyJSfmHUdwqs1siiSRdR3bQRv9LKaPtA=; b=SwW+JNMP1JdhWRDL7fjF92CKyqrcpTQ+/hfKar1xOsaDOfL/eK+AEhR+G85FkFUTFr VQCK/PbAlgHrhXNvEgqjxBfAgYpthjVyy1NWsu1ZRPGcrFw2co6NWwXZd0fAF6edlTHk e+/w/BlHwAeTgYBQxZHWOVLH5gWsLveRJFpVB6nLCOUCJMNNUabfkcGKUbmk7q5FwV4m uYx8tM/BYrNtbwb0nLVmaABhIDAQKt8W1GubK0JT3PNA4nXegjMJzmvgHST1aCUKRxL8 1zj+GCdul/OqLAMBWhYr8gY7qzlirOnCpwl+57PLP2UAHksRsAq8dzdGw4QXjsa4B7Sr 04Qg== X-Gm-Message-State: AC+VfDzDpOL14q1wA/gX0bfRGcgFgD/L0FshTx4K+LdNlKriIp96Migz FjSfweQvXI0yOvynXflDf8kcgrGoFPU= X-Google-Smtp-Source: ACHHUZ56XQIlDwU1r2kStlmVwia3kGOQZT4yRtzPV41Kypo2oIwDEMYW3HTvUoo1HWFHfQNkmRFv1A== X-Received: by 2002:a17:90a:6506:b0:24e:4c8:3ae5 with SMTP id i6-20020a17090a650600b0024e04c83ae5mr5650172pjj.28.1684582872268; Sat, 20 May 2023 04:41:12 -0700 (PDT) Received: from squeak.grove.modra.org ([2406:3400:51d:8cc0:ffe9:fc64:f919:42db]) by smtp.gmail.com with ESMTPSA id c14-20020a17090ad90e00b00247601ce2aesm2881523pjv.20.2023.05.20.04.41.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 20 May 2023 04:41:11 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id C3C501142BF9; Sat, 20 May 2023 21:11:08 +0930 (ACST) Date: Sat, 20 May 2023 21:11:08 +0930 From: Alan Modra To: binutils@sourceware.org Cc: "Maciej W. Rozycki" , Chenghua Xu Subject: Re: Protect mips_hi16_list from fuzzed debug info Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-3034.3 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 is a slightly modified version of the patch posted at https://sourceware.org/pipermail/binutils/2023-February/125916.html with the logic for detecting orphan hi16 relocs in free_mips_hi16_list improved so that the warning can be enabled. OK to apply? === 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 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 49355a42f7d..18b04a1abb5 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. */ @@ -1418,6 +1414,30 @@ free_ecoff_debug (struct ecoff_debug_info *debug) debug->external_ext = NULL; } +/* 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_close_and_cleanup (bfd *abfd) { @@ -1427,15 +1447,13 @@ _bfd_mips_elf_close_and_cleanup (bfd *abfd) if (tdata != 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) free_ecoff_debug (&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_close_and_cleanup (abfd); } @@ -2557,26 +2575,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; @@ -2615,40 +2629,40 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol, 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. */ + 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) + 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 @@ -2664,13 +2678,13 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol, hi->rel.addend += vallo; - 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); } @@ -13344,24 +13358,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; -- Alan Modra Australia Development Lab, IBM