From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1062) id 9DFAE385782B; Thu, 1 Jun 2023 00:33:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9DFAE385782B Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Alan Modra To: bfd-cvs@sourceware.org Subject: [binutils-gdb] Harden PowerPC64 OPD handling against fuzzers X-Act-Checkin: binutils-gdb X-Git-Author: Alan Modra X-Git-Refname: refs/heads/master X-Git-Oldrev: 8261abd51344cb4c454514fba1389a9292f9652b X-Git-Newrev: 6313825cbf834b1852007707cfff2ccd3b0dbd6b Message-Id: <20230601003334.9DFAE385782B@sourceware.org> Date: Thu, 1 Jun 2023 00:33:34 +0000 (GMT) X-BeenThere: binutils-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Jun 2023 00:33:34 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D6313825cbf83= 4b1852007707cfff2ccd3b0dbd6b commit 6313825cbf834b1852007707cfff2ccd3b0dbd6b Author: Alan Modra Date: Wed May 31 15:11:34 2023 +0930 Harden PowerPC64 OPD handling against fuzzers =20 PowerPC64 ELFv1 object files should have at most one .opd section, and OPD handling in elf64-ppc.c makes use of this fact by caching some .opd section info in the per-object bfd.tdata. This was done to avoid another word in the target specific section data. Of course, fuzzers don't respect the ABI, and even non-malicious users can accidentally create multiple .opd sections. So it is better to avoid possible buffer overflows and other confusion when OPD handling for a second .opd section references data for the first .opd section, by keeping the data per-section. =20 The patch also fixes a memory leak, and a corner case where I think we could hit an assertion in opd_entry_value or read out of bounds in ppc64_elf_branch_reloc doing a final link producing non-ppc64 output. (It's a really rare corner case because not only would you need to be linking ppc64 objects to non-ppc64 output, you'd also need a branch reloc symbol to be defined in a .opd section of a non-ppc64 input.) =20 * elf64-ppc.c (is_ppc64_elf): Move earlier in file. (ppc64_elf_branch_reloc): Check symbol bfd before accessing ppc64 elf specific data structures. (struct ppc64_elf_obj_tdata): Move opd union.. (struct _ppc64_elf_section_data): ..to here. (ppc64_elf_before_check_relocs): Allow for opd sec_type already set to sec_opd. (ppc64_elf_check_relocs): Only set sec_type to sec_toc when unset. Error for unexpected toc relocs. (opd_entry_value): Return -1 when non-ppc64 rather than asserting. Check and set sec_type too. Adjust for changed location of contents and relocs. (ppc64_elf_relocate_section): Adjust for changed location of cached .opd relocs. (ppc64_elf_free_cached_info): New function. (bfd_elf64_bfd_free_cached_info): Define. Diff: --- bfd/elf64-ppc.c | 87 +++++++++++++++++++++++++++++++++++++++--------------= ---- 1 file changed, 60 insertions(+), 27 deletions(-) diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c index 33f4275261d..a977bec2f50 100644 --- a/bfd/elf64-ppc.c +++ b/bfd/elf64-ppc.c @@ -90,6 +90,7 @@ static bfd_vma opd_entry_value #define elf_backend_default_execstack 0 =20 #define bfd_elf64_mkobject ppc64_elf_mkobject +#define bfd_elf64_bfd_free_cached_info ppc64_elf_free_cached_info #define bfd_elf64_bfd_reloc_type_lookup ppc64_elf_reloc_type_lookup #define bfd_elf64_bfd_reloc_name_lookup ppc64_elf_reloc_name_lookup #define bfd_elf64_bfd_merge_private_bfd_data ppc64_elf_merge_private_bfd_= data @@ -296,6 +297,10 @@ set_abiversion (bfd *abfd, int ver) elf_elfheader (abfd)->e_flags &=3D ~EF_PPC64_ABI; elf_elfheader (abfd)->e_flags |=3D ver & EF_PPC64_ABI; } + +#define is_ppc64_elf(bfd) \ + (bfd_get_flavour (bfd) =3D=3D bfd_target_elf_flavour \ + && elf_object_id (bfd) =3D=3D PPC64_ELF_DATA) =0C /* Relocation HOWTO's. */ /* Like other ELF RELA targets that don't apply multiple @@ -1462,6 +1467,10 @@ ppc64_elf_branch_reloc (bfd *abfd, arelent *reloc_en= try, asymbol *symbol, return bfd_elf_generic_reloc (abfd, reloc_entry, symbol, data, input_section, output_bfd, error_message); =20 + if (symbol->section->owner =3D=3D NULL + || !is_ppc64_elf (symbol->section->owner)) + return bfd_reloc_continue; + if (strcmp (symbol->section->name, ".opd") =3D=3D 0 && (symbol->section->owner->flags & DYNAMIC) =3D=3D 0) { @@ -1478,7 +1487,6 @@ ppc64_elf_branch_reloc (bfd *abfd, arelent *reloc_ent= ry, asymbol *symbol, elf_symbol_type *elfsym =3D (elf_symbol_type *) symbol; =20 if (symbol->section->owner !=3D abfd - && symbol->section->owner !=3D NULL && abiversion (symbol->section->owner) >=3D 2) { unsigned int i; @@ -1817,15 +1825,6 @@ struct ppc64_elf_obj_tdata sections means we potentially need one of these for each input bfd. = */ struct got_entry tlsld_got; =20 - union - { - /* A copy of relocs before they are modified for --emit-relocs. */ - Elf_Internal_Rela *relocs; - - /* Section contents. */ - bfd_byte *contents; - } opd; - /* Nonzero if this bfd has small toc/got relocs, ie. that expect the reloc to be in the range -32768 to 32767. */ unsigned int has_small_toc_reloc : 1; @@ -1845,10 +1844,6 @@ struct ppc64_elf_obj_tdata #define ppc64_tlsld_got(bfd) \ (&ppc64_elf_tdata (bfd)->tlsld_got) =20 -#define is_ppc64_elf(bfd) \ - (bfd_get_flavour (bfd) =3D=3D bfd_target_elf_flavour \ - && elf_object_id (bfd) =3D=3D PPC64_ELF_DATA) - /* Override the generic function because we store some extras. */ =20 static bool @@ -2016,6 +2011,15 @@ struct _ppc64_elf_section_data =20 /* After editing .opd, adjust references to opd local syms. */ long *adjust; + + union + { + /* A copy of relocs before they are modified for --emit-relocs. */ + Elf_Internal_Rela *relocs; + + /* Section contents. */ + bfd_byte *contents; + } u; } opd; =20 /* An array for toc sections, indexed by offset/8. */ @@ -4479,8 +4483,10 @@ ppc64_elf_before_check_relocs (bfd *ibfd, struct bfd= _link_info *info) =20 if (opd !=3D NULL && opd->size !=3D 0) { - BFD_ASSERT (ppc64_elf_section_data (opd)->sec_type =3D=3D sec_normal= ); - ppc64_elf_section_data (opd)->sec_type =3D sec_opd; + if (ppc64_elf_section_data (opd)->sec_type =3D=3D sec_normal) + ppc64_elf_section_data (opd)->sec_type =3D sec_opd; + else if (ppc64_elf_section_data (opd)->sec_type !=3D sec_opd) + BFD_FAIL (); =20 if (abiversion (ibfd) =3D=3D 0) set_abiversion (ibfd, 1); @@ -5234,7 +5240,7 @@ ppc64_elf_check_relocs (bfd *abfd, struct bfd_link_in= fo *info, return false; =20 ppc64_sec =3D ppc64_elf_section_data (sec); - if (ppc64_sec->sec_type !=3D sec_toc) + if (ppc64_sec->sec_type =3D=3D sec_normal) { bfd_size_type amt; =20 @@ -5247,10 +5253,17 @@ ppc64_elf_check_relocs (bfd *abfd, struct bfd_link_= info *info, ppc64_sec->u.toc.add =3D bfd_zalloc (abfd, amt); if (ppc64_sec->u.toc.add =3D=3D NULL) return false; - BFD_ASSERT (ppc64_sec->sec_type =3D=3D sec_normal); ppc64_sec->sec_type =3D sec_toc; } - BFD_ASSERT (rel->r_offset % 8 =3D=3D 0); + if (ppc64_sec->sec_type !=3D sec_toc + || rel->r_offset % 8 !=3D 0) + { + info->callbacks->einfo (_("%H: %s reloc unsupported here\n"), + abfd, sec, rel->r_offset, + ppc64_elf_howto_table[r_type]->name); + bfd_set_error (bfd_error_bad_value); + return false; + } ppc64_sec->u.toc.symndx[rel->r_offset / 8] =3D r_symndx; ppc64_sec->u.toc.add[rel->r_offset / 8] =3D rel->r_addend; =20 @@ -5531,18 +5544,26 @@ opd_entry_value (asection *opd_sec, Elf_Internal_Rela *lo, *hi, *look; bfd_vma val; =20 + if (!is_ppc64_elf (opd_bfd)) + return (bfd_vma) -1; + + if (ppc64_elf_section_data (opd_sec)->sec_type =3D=3D sec_normal) + ppc64_elf_section_data (opd_sec)->sec_type =3D sec_opd; + else if (ppc64_elf_section_data (opd_sec)->sec_type !=3D sec_opd) + return (bfd_vma) -1; + /* No relocs implies we are linking a --just-symbols object, or looking at a final linked executable with addr2line or somesuch. */ if (opd_sec->reloc_count =3D=3D 0) { - bfd_byte *contents =3D ppc64_elf_tdata (opd_bfd)->opd.contents; + bfd_byte *contents =3D ppc64_elf_section_data (opd_sec)->u.opd.u.con= tents; =20 if (contents =3D=3D NULL) { if ((opd_sec->flags & SEC_HAS_CONTENTS) =3D=3D 0 || !bfd_malloc_and_get_section (opd_bfd, opd_sec, &contents)) return (bfd_vma) -1; - ppc64_elf_tdata (opd_bfd)->opd.contents =3D contents; + ppc64_elf_section_data (opd_sec)->u.opd.u.contents =3D contents; } =20 /* PR 17512: file: 64b9dfbb. */ @@ -5579,9 +5600,7 @@ opd_entry_value (asection *opd_sec, return val; } =20 - BFD_ASSERT (is_ppc64_elf (opd_bfd)); - - relocs =3D ppc64_elf_tdata (opd_bfd)->opd.relocs; + relocs =3D ppc64_elf_section_data (opd_sec)->u.opd.u.relocs; if (relocs =3D=3D NULL) relocs =3D _bfd_elf_link_read_relocs (opd_bfd, opd_sec, NULL, NULL, tr= ue); /* PR 17512: file: df8e1fd6. */ @@ -18055,13 +18074,14 @@ ppc64_elf_relocate_section (bfd *output_bfd, adjusted. Worse, reloc symbol indices will be for the output file rather than the input. Save a copy of the relocs for opd_entry_value. */ - if (is_opd && (info->emitrelocations || bfd_link_relocatable (info))) + if (is_opd + && (info->emitrelocations || bfd_link_relocatable (info)) + && input_section->reloc_count !=3D 0) { bfd_size_type amt; amt =3D input_section->reloc_count * sizeof (Elf_Internal_Rela); rel =3D bfd_alloc (input_bfd, amt); - BFD_ASSERT (ppc64_elf_tdata (input_bfd)->opd.relocs =3D=3D NULL); - ppc64_elf_tdata (input_bfd)->opd.relocs =3D rel; + ppc64_elf_section_data (input_section)->u.opd.u.relocs =3D rel; if (rel =3D=3D NULL) return false; memcpy (rel, relocs, amt); @@ -18376,6 +18396,19 @@ ppc64_elf_finish_dynamic_sections (bfd *output_bfd, return true; } =20 +static bool +ppc64_elf_free_cached_info (bfd *abfd) +{ + if (abfd->sections) + for (asection *opd =3D bfd_get_section_by_name (abfd, ".opd"); + opd !=3D NULL; + opd =3D bfd_get_next_section_by_name (NULL, opd)) + if (opd->reloc_count =3D=3D 0) + free (ppc64_elf_section_data (opd)->u.opd.u.contents); + + return _bfd_free_cached_info (abfd); +} + #include "elf64-target.h" =20 /* FreeBSD support */