commit b425859021d17adf62f06fb904797cf8642986ad Author: Nick Clifton Date: Wed Feb 5 16:27:38 2025 +0000 Fix another illegal memory access triggered by corrupt ELF input files. PR 32644 commit 931494c9a89558acb36a03a340c01726545eef24 Author: Nick Clifton Date: Wed Feb 5 15:43:04 2025 +0000 Add even more checks for corrupt input when processing relocations for ELF files. PR 32643 commit 18cc11a2771d9e40180485da9a4fb660c03efac3 Author: Nick Clifton Date: Wed Feb 5 14:31:10 2025 +0000 Prevent illegal memory access when checking relocs in a corrupt ELF binary. PR 32641 commit f9978defb6fab0bd8583942d97c112b0932ac814 Author: Nick Clifton Date: Wed Feb 5 11:15:11 2025 +0000 Prevent illegal memory access when indexing into the sym_hashes array of the elf bfd cookie structure. PR 32636 diff -rup binutils.orig/bfd/elf-bfd.h binutils-2.43.1/bfd/elf-bfd.h --- binutils.orig/bfd/elf-bfd.h 2025-02-11 10:58:25.223874250 +0000 +++ binutils-2.43.1/bfd/elf-bfd.h 2025-02-11 11:13:12.177965286 +0000 @@ -3147,6 +3147,9 @@ extern bool _bfd_elf_link_mmap_section_c extern void _bfd_elf_link_munmap_section_contents (asection *); +extern struct elf_link_hash_entry * _bfd_elf_get_link_hash_entry + (struct elf_link_hash_entry **, unsigned int, Elf_Internal_Shdr *); + /* Large common section. */ extern asection _bfd_elf_large_com_section; diff -rup binutils.orig/bfd/elflink.c binutils-2.43.1/bfd/elflink.c --- binutils.orig/bfd/elflink.c 2025-02-11 10:58:25.256874290 +0000 +++ binutils-2.43.1/bfd/elflink.c 2025-02-11 11:11:22.536375589 +0000 @@ -96,22 +96,64 @@ _bfd_elf_link_keep_memory (struct bfd_li return true; } -asection * -_bfd_elf_section_for_symbol (struct elf_reloc_cookie *cookie, - unsigned long r_symndx, - bool discard) +static struct elf_link_hash_entry * +get_link_hash_entry (struct elf_link_hash_entry ** sym_hashes, + unsigned int symndx, + unsigned int ext_sym_start) +{ + if (sym_hashes == NULL + /* Guard against corrupt input. See PR 32636 for an example. */ + || symndx < ext_sym_start) + return NULL; + + struct elf_link_hash_entry *h = sym_hashes[symndx - ext_sym_start]; + + /* The hash might be empty. See PR 32641 for an example of this. */ + if (h == NULL) + return NULL; + + while (h->root.type == bfd_link_hash_indirect + || h->root.type == bfd_link_hash_warning) + h = (struct elf_link_hash_entry *) h->root.u.i.link; + + return h; +} + +struct elf_link_hash_entry * +_bfd_elf_get_link_hash_entry (struct elf_link_hash_entry ** sym_hashes, + unsigned int symndx, + Elf_Internal_Shdr * symtab_hdr) +{ + if (symtab_hdr == NULL) + return NULL; + + return get_link_hash_entry (sym_hashes, symndx, symtab_hdr->sh_info); +} + +static struct elf_link_hash_entry * +get_ext_sym_hash_from_cookie (struct elf_reloc_cookie *cookie, unsigned long r_symndx) { + if (cookie == NULL || cookie->sym_hashes == NULL) + return NULL; + if (r_symndx >= cookie->locsymcount || ELF_ST_BIND (cookie->locsyms[r_symndx].st_info) != STB_LOCAL) - { - struct elf_link_hash_entry *h; + return get_link_hash_entry (cookie->sym_hashes, r_symndx, cookie->extsymoff); - h = cookie->sym_hashes[r_symndx - cookie->extsymoff]; + return NULL; +} - while (h->root.type == bfd_link_hash_indirect - || h->root.type == bfd_link_hash_warning) - h = (struct elf_link_hash_entry *) h->root.u.i.link; +asection * +_bfd_elf_section_for_symbol (struct elf_reloc_cookie *cookie, + unsigned long r_symndx, + bool discard) +{ + struct elf_link_hash_entry *h; + h = get_ext_sym_hash_from_cookie (cookie, r_symndx); + + if (h != NULL) + { if ((h->root.type == bfd_link_hash_defined || h->root.type == bfd_link_hash_defweak) && discarded_section (h->root.u.def.section)) @@ -119,21 +161,20 @@ _bfd_elf_section_for_symbol (struct elf_ else return NULL; } - else - { - /* It's not a relocation against a global symbol, - but it could be a relocation against a local - symbol for a discarded section. */ - asection *isec; - Elf_Internal_Sym *isym; - /* Need to: get the symbol; get the section. */ - isym = &cookie->locsyms[r_symndx]; - isec = bfd_section_from_elf_index (cookie->abfd, isym->st_shndx); - if (isec != NULL - && discard ? discarded_section (isec) : 1) - return isec; - } + /* It's not a relocation against a global symbol, + but it could be a relocation against a local + symbol for a discarded section. */ + asection *isec; + Elf_Internal_Sym *isym; + + /* Need to: get the symbol; get the section. */ + isym = &cookie->locsyms[r_symndx]; + isec = bfd_section_from_elf_index (cookie->abfd, isym->st_shndx); + if (isec != NULL + && discard ? discarded_section (isec) : 1) + return isec; + return NULL; } @@ -9058,7 +9099,6 @@ set_symbol_value (bfd *bfd_with_globals, size_t symidx, bfd_vma val) { - struct elf_link_hash_entry **sym_hashes; struct elf_link_hash_entry *h; size_t extsymoff = locsymcount; @@ -9081,12 +9121,12 @@ set_symbol_value (bfd *bfd_with_globals, /* It is a global symbol: set its link type to "defined" and give it a value. */ - - sym_hashes = elf_sym_hashes (bfd_with_globals); - h = sym_hashes [symidx - extsymoff]; - while (h->root.type == bfd_link_hash_indirect - || h->root.type == bfd_link_hash_warning) - h = (struct elf_link_hash_entry *) h->root.u.i.link; + h = get_link_hash_entry (elf_sym_hashes (bfd_with_globals), symidx, extsymoff); + if (h == NULL) + { + /* FIXMEL What should we do ? */ + return; + } h->root.type = bfd_link_hash_defined; h->root.u.def.value = val; h->root.u.def.section = bfd_abs_section_ptr; @@ -11568,10 +11608,19 @@ elf_link_input_bfd (struct elf_final_lin || (elf_bad_symtab (input_bfd) && flinfo->sections[symndx] == NULL)) { - struct elf_link_hash_entry *h = sym_hashes[symndx - extsymoff]; - while (h->root.type == bfd_link_hash_indirect - || h->root.type == bfd_link_hash_warning) - h = (struct elf_link_hash_entry *) h->root.u.i.link; + struct elf_link_hash_entry *h; + + h = get_link_hash_entry (sym_hashes, symndx, extsymoff); + if (h == NULL) + { + _bfd_error_handler + /* xgettext:c-format */ + (_("error: %pB: unable to create group section symbol"), + input_bfd); + bfd_set_error (bfd_error_bad_value); + return false; + } + /* Arrange for symbol to be output. */ h->indx = -2; elf_section_data (osec)->this_hdr.sh_info = -2; @@ -11706,7 +11755,7 @@ elf_link_input_bfd (struct elf_final_lin || (elf_bad_symtab (input_bfd) && flinfo->sections[r_symndx] == NULL)) { - h = sym_hashes[r_symndx - extsymoff]; + h = get_link_hash_entry (sym_hashes, r_symndx, extsymoff); /* Badly formatted input files can contain relocs that reference non-existant symbols. Check here so that @@ -11715,17 +11764,13 @@ elf_link_input_bfd (struct elf_final_lin { _bfd_error_handler /* xgettext:c-format */ - (_("error: %pB contains a reloc (%#" PRIx64 ") for section %pA " + (_("error: %pB contains a reloc (%#" PRIx64 ") for section '%pA' " "that references a non-existent global symbol"), input_bfd, (uint64_t) rel->r_info, o); bfd_set_error (bfd_error_bad_value); return false; } - while (h->root.type == bfd_link_hash_indirect - || h->root.type == bfd_link_hash_warning) - h = (struct elf_link_hash_entry *) h->root.u.i.link; - s_type = h->type; /* If a plugin symbol is referenced from a non-IR file, @@ -11941,7 +11986,6 @@ elf_link_input_bfd (struct elf_final_lin && flinfo->sections[r_symndx] == NULL)) { struct elf_link_hash_entry *rh; - unsigned long indx; /* This is a reloc against a global symbol. We have not yet output all the local symbols, so @@ -11950,11 +11994,13 @@ elf_link_input_bfd (struct elf_final_lin reloc to point to the global hash table entry for this symbol. The symbol index is then set at the end of bfd_elf_final_link. */ - indx = r_symndx - extsymoff; - rh = elf_sym_hashes (input_bfd)[indx]; - while (rh->root.type == bfd_link_hash_indirect - || rh->root.type == bfd_link_hash_warning) - rh = (struct elf_link_hash_entry *) rh->root.u.i.link; + rh = get_link_hash_entry (elf_sym_hashes (input_bfd), + r_symndx, extsymoff); + if (rh == NULL) + { + /* FIXME: Generate an error ? */ + continue; + } /* Setting the index to -2 tells elf_link_output_extsym that this symbol is @@ -13958,25 +14004,21 @@ _bfd_elf_gc_mark_hook (asection *sec, struct elf_link_hash_entry *h, Elf_Internal_Sym *sym) { - if (h != NULL) + if (h == NULL) + return bfd_section_from_elf_index (sec->owner, sym->st_shndx); + + switch (h->root.type) { - switch (h->root.type) - { - case bfd_link_hash_defined: - case bfd_link_hash_defweak: - return h->root.u.def.section; + case bfd_link_hash_defined: + case bfd_link_hash_defweak: + return h->root.u.def.section; - case bfd_link_hash_common: - return h->root.u.c.p->section; + case bfd_link_hash_common: + return h->root.u.c.p->section; - default: - break; - } + default: + return NULL; } - else - return bfd_section_from_elf_index (sec->owner, sym->st_shndx); - - return NULL; } /* Return the debug definition section. */ @@ -14025,56 +14067,49 @@ _bfd_elf_gc_mark_rsec (struct bfd_link_i if (r_symndx == STN_UNDEF) return NULL; - if (r_symndx >= cookie->locsymcount - || ELF_ST_BIND (cookie->locsyms[r_symndx].st_info) != STB_LOCAL) + h = get_ext_sym_hash_from_cookie (cookie, r_symndx); + if (h == NULL) { - bool was_marked; + /* A corrup tinput file can lead to a situation where the index + does not reference either a local or an external symbol. */ + if (r_symndx >= cookie->locsymcount) + return NULL; - h = cookie->sym_hashes[r_symndx - cookie->extsymoff]; - if (h == NULL) - { - info->callbacks->einfo (_("%F%P: corrupt input: %pB\n"), - sec->owner); - return NULL; - } - while (h->root.type == bfd_link_hash_indirect - || h->root.type == bfd_link_hash_warning) - h = (struct elf_link_hash_entry *) h->root.u.i.link; + return (*gc_mark_hook) (sec, info, cookie->rel, NULL, + &cookie->locsyms[r_symndx]); + } - was_marked = h->mark; - h->mark = 1; - /* Keep all aliases of the symbol too. If an object symbol - needs to be copied into .dynbss then all of its aliases - should be present as dynamic symbols, not just the one used - on the copy relocation. */ - hw = h; - while (hw->is_weakalias) - { - hw = hw->u.alias; - hw->mark = 1; - } + bool was_marked = h->mark; - if (!was_marked && h->start_stop && !h->root.ldscript_def) - { - if (info->start_stop_gc) - return NULL; + h->mark = 1; + /* Keep all aliases of the symbol too. If an object symbol + needs to be copied into .dynbss then all of its aliases + should be present as dynamic symbols, not just the one used + on the copy relocation. */ + hw = h; + while (hw->is_weakalias) + { + hw = hw->u.alias; + hw->mark = 1; + } - /* To work around a glibc bug, mark XXX input sections - when there is a reference to __start_XXX or __stop_XXX - symbols. */ - else if (start_stop != NULL) - { - asection *s = h->u2.start_stop_section; - *start_stop = true; - return s; - } - } + if (!was_marked && h->start_stop && !h->root.ldscript_def) + { + if (info->start_stop_gc) + return NULL; - return (*gc_mark_hook) (sec, info, cookie->rel, h, NULL); + /* To work around a glibc bug, mark XXX input sections + when there is a reference to __start_XXX or __stop_XXX + symbols. */ + else if (start_stop != NULL) + { + asection *s = h->u2.start_stop_section; + *start_stop = true; + return s; + } } - return (*gc_mark_hook) (sec, info, cookie->rel, NULL, - &cookie->locsyms[r_symndx]); + return (*gc_mark_hook) (sec, info, cookie->rel, h, NULL); } /* COOKIE->rel describes a relocation against section SEC, which is @@ -15095,17 +15130,12 @@ bfd_elf_reloc_symbol_deleted_p (bfd_vma if (r_symndx == STN_UNDEF) return true; - if (r_symndx >= rcookie->locsymcount - || ELF_ST_BIND (rcookie->locsyms[r_symndx].st_info) != STB_LOCAL) - { - struct elf_link_hash_entry *h; - - h = rcookie->sym_hashes[r_symndx - rcookie->extsymoff]; - - while (h->root.type == bfd_link_hash_indirect - || h->root.type == bfd_link_hash_warning) - h = (struct elf_link_hash_entry *) h->root.u.i.link; + struct elf_link_hash_entry *h; + h = get_ext_sym_hash_from_cookie (rcookie, r_symndx); + + if (h != NULL) + { if ((h->root.type == bfd_link_hash_defined || h->root.type == bfd_link_hash_defweak) && (h->root.u.def.section->owner != rcookie->abfd @@ -15115,6 +15145,10 @@ bfd_elf_reloc_symbol_deleted_p (bfd_vma } else { + if (r_symndx >= rcookie->locsymcount) + /* This can happen with corrupt input. */ + return false; + /* It's not a relocation against a global symbol, but it could be a relocation against a local symbol for a discarded section. */ diff -rup binutils.orig/bfd/elfxx-x86.c binutils-2.43.1/bfd/elfxx-x86.c --- binutils.orig/bfd/elfxx-x86.c 2025-02-11 10:58:25.307874350 +0000 +++ binutils-2.43.1/bfd/elfxx-x86.c 2025-02-11 11:15:21.937392450 +0000 @@ -972,15 +972,7 @@ _bfd_x86_elf_check_relocs (bfd *abfd, goto error_return; } - if (r_symndx < symtab_hdr->sh_info) - h = NULL; - else - { - h = sym_hashes[r_symndx - symtab_hdr->sh_info]; - while (h->root.type == bfd_link_hash_indirect - || h->root.type == bfd_link_hash_warning) - h = (struct elf_link_hash_entry *) h->root.u.i.link; - } + h = _bfd_elf_get_link_hash_entry (sym_hashes, r_symndx, symtab_hdr); if (X86_NEED_DYNAMIC_RELOC_TYPE_P (is_x86_64, r_type) && NEED_DYNAMIC_RELOCATION_P (is_x86_64, info, true, h, sec, @@ -1205,10 +1197,12 @@ _bfd_x86_elf_link_relax_section (bfd *ab else { /* Get H and SEC for GENERATE_DYNAMIC_RELOCATION_P below. */ - h = sym_hashes[r_symndx - symtab_hdr->sh_info]; - while (h->root.type == bfd_link_hash_indirect - || h->root.type == bfd_link_hash_warning) - h = (struct elf_link_hash_entry *) h->root.u.i.link; + h = _bfd_elf_get_link_hash_entry (sym_hashes, r_symndx, symtab_hdr); + if (h == NULL) + { + /* FIXMEL: Issue an error message ? */ + continue; + } if (h->root.type == bfd_link_hash_defined || h->root.type == bfd_link_hash_defweak)