From: Nick Clifton Date: Wed, 5 Feb 2025 15:43:04 +0000 Add even more checks for corrupt input when processing relocations for ELF files. PR 32643 Upstream-Status: Backport [https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=931494c9a89558acb36a03a340c01726545eef24] CVE: CVE-2025-1181 Signed-off-by: Deepesh Varatharajan diff --git a/bfd/elflink.c b/bfd/elflink.c index fd423d61..91cd7c28 100644 --- a/bfd/elflink.c +++ b/bfd/elflink.c @@ -96,15 +96,17 @@ return true; } -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) +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 (symndx < symtab_hdr->sh_info) + 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 - symtab_hdr->sh_info]; + 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) @@ -117,27 +119,28 @@ return h; } -static struct elf_link_hash_entry * -get_ext_sym_hash (struct elf_reloc_cookie *cookie, unsigned long r_symndx) +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) { - struct elf_link_hash_entry *h = NULL; - - if ((r_symndx >= cookie->locsymcount - || ELF_ST_BIND (cookie->locsyms[r_symndx].st_info) != STB_LOCAL) - /* Guard against corrupt input. See PR 32636 for an example. */ - && r_symndx >= cookie->extsymoff) - { - h = cookie->sym_hashes[r_symndx - cookie->extsymoff]; + if (symtab_hdr == NULL) + return NULL; - if (h == NULL) - return NULL; + return get_link_hash_entry (sym_hashes, 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; - } +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) + return get_link_hash_entry (cookie->sym_hashes, r_symndx, cookie->extsymoff); - return h; + return NULL; } asection * @@ -147,7 +150,7 @@ { struct elf_link_hash_entry *h; - h = get_ext_sym_hash (cookie, r_symndx); + h = get_ext_sym_hash_from_cookie (cookie, r_symndx); if (h != NULL) { @@ -9105,7 +9108,6 @@ size_t symidx, bfd_vma val) { - struct elf_link_hash_entry **sym_hashes; struct elf_link_hash_entry *h; size_t extsymoff = locsymcount; @@ -9128,12 +9130,12 @@ /* 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; @@ -11611,10 +11613,19 @@ || (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; @@ -11749,7 +11760,7 @@ || (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 @@ -11758,17 +11769,13 @@ { _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, @@ -11984,7 +11991,6 @@ && 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 @@ -11993,15 +11999,16 @@ 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; - - /* Setting the index to -2 tells - elf_link_output_extsym that this symbol is - used by a reloc. */ + 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 used by a reloc. */ BFD_ASSERT (rh->indx < 0); rh->indx = -2; *rel_hash = rh; @@ -13965,25 +13972,21 @@ 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. */ @@ -14032,46 +14035,49 @@ if (r_symndx == STN_UNDEF) return NULL; - h = get_ext_sym_hash (cookie, r_symndx); + h = get_ext_sym_hash_from_cookie (cookie, r_symndx); + if (h == NULL) + { + /* 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; - if (h != NULL) + return (*gc_mark_hook) (sec, info, cookie->rel, NULL, + &cookie->locsyms[r_symndx]); + } + + bool 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) { - bool was_marked; + hw = hw->u.alias; + hw->mark = 1; + } - 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; - } + if (!was_marked && h->start_stop && !h->root.ldscript_def) + { + if (info->start_stop_gc) + return NULL; - if (!was_marked && h->start_stop && !h->root.ldscript_def) + /* 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) { - if (info->start_stop_gc) - return 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; - } + asection *s = h->u2.start_stop_section; + *start_stop = true; + return s; } - - return (*gc_mark_hook) (sec, info, cookie->rel, h, NULL); } - 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 @@ -15094,7 +15100,7 @@ struct elf_link_hash_entry *h; - h = get_ext_sym_hash (rcookie, r_symndx); + h = get_ext_sym_hash_from_cookie (rcookie, r_symndx); if (h != NULL) {