From a86c6c19643e9ac795b17846e5b0db8b3e4c54de Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Mon, 14 Dec 2020 19:36:47 +1030 Subject: [PATCH] Put bfd_section_from_shdr loop detection array in elf_tdata The static variables used by bfd_section_from_shdr to detect loops in ELF sections have a problem: Comparing a BFD pointer doesn't guarantee that the current bfd is the same as the one previously used to allocate the sections_being_created array. For example, doing size bad_elf_1 bad_elf_2 with two corrupted ELF files containing section loops will leave the section_being_created array allocated for the first file and since bfd_close is called for bad_elf_1 before bfd_elf_2 is opened, it is possible that the BFD for the second file is allocated in the same memory as the first file. If bad_elf_2 has more sections than bad_elf_1 then we might write beyond the end of the array. So this patch implements the FIXME Nick put in a comment about attaching the array to the BFD. * elf-bfd.h (struct elf_obj_tdata): Add being_created. * elf.c (bfd_section_from_shdr): Delete static vars for loop detection. Use new tdata variable instead. * elfcode.h (elf_object_p): Allocate being_created. --- bfd/ChangeLog | 7 +++++++ bfd/elf-bfd.h | 1 + bfd/elf.c | 54 +++++++++------------------------------------------ bfd/elfcode.h | 3 +++ 4 files changed, 20 insertions(+), 45 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 8be40185826..e3a5c8a6eeb 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,10 @@ +2020-12-14 Alan Modra + + * elf-bfd.h (struct elf_obj_tdata): Add being_created. + * elf.c (bfd_section_from_shdr): Delete static vars for loop + detection. Use new tdata variable instead. + * elfcode.h (elf_object_p): Allocate being_created. + 2020-12-10 Nelson Chu * elfxx-riscv.c (riscv_ext_dont_care_version): New function. Return diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h index e9c890f6f16..53b5ffc33d5 100644 --- a/bfd/elf-bfd.h +++ b/bfd/elf-bfd.h @@ -1942,6 +1942,7 @@ struct elf_obj_tdata bfd_vma gp; /* The gp value */ unsigned int gp_size; /* The gp size */ unsigned int num_elf_sections; /* elf_sect_ptr size */ + unsigned char *being_created; /* A mapping from external symbols to entries in the linker hash table, used when linking. This is indexed by the symbol index diff --git a/bfd/elf.c b/bfd/elf.c index 419c5f4420c..7f2237655d3 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -2045,49 +2045,20 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) const struct elf_backend_data *bed; const char *name; bfd_boolean ret = TRUE; - static bfd_boolean * sections_being_created = NULL; - static bfd * sections_being_created_abfd = NULL; - static unsigned int nesting = 0; if (shindex >= elf_numsections (abfd)) return FALSE; - if (++ nesting > 3) + /* PR17512: A corrupt ELF binary might contain a loop of sections via + sh_link or sh_info. Detect this here, by refusing to load a + section that we are already in the process of loading. */ + if (elf_tdata (abfd)->being_created[shindex]) { - /* PR17512: A corrupt ELF binary might contain a recursive group of - sections, with each the string indices pointing to the next in the - loop. Detect this here, by refusing to load a section that we are - already in the process of loading. We only trigger this test if - we have nested at least three sections deep as normal ELF binaries - can expect to recurse at least once. - - FIXME: It would be better if this array was attached to the bfd, - rather than being held in a static pointer. */ - - if (sections_being_created_abfd != abfd) - { - free (sections_being_created); - sections_being_created = NULL; - } - if (sections_being_created == NULL) - { - size_t amt = elf_numsections (abfd) * sizeof (bfd_boolean); - - /* PR 26005: Do not use bfd_zalloc here as the memory might - be released before the bfd has been fully scanned. */ - sections_being_created = (bfd_boolean *) bfd_zmalloc (amt); - if (sections_being_created == NULL) - return FALSE; - sections_being_created_abfd = abfd; - } - if (sections_being_created [shindex]) - { - _bfd_error_handler - (_("%pB: warning: loop in section dependencies detected"), abfd); - return FALSE; - } - sections_being_created [shindex] = TRUE; + _bfd_error_handler + (_("%pB: warning: loop in section dependencies detected"), abfd); + return FALSE; } + elf_tdata (abfd)->being_created[shindex] = TRUE; hdr = elf_elfsections (abfd)[shindex]; ehdr = elf_elfheader (abfd); @@ -2611,14 +2582,7 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) fail: ret = FALSE; success: - if (sections_being_created && sections_being_created_abfd == abfd) - sections_being_created [shindex] = FALSE; - if (-- nesting == 0) - { - free (sections_being_created); - sections_being_created = NULL; - sections_being_created_abfd = NULL; - } + elf_tdata (abfd)->being_created[shindex] = FALSE; return ret; } diff --git a/bfd/elfcode.h b/bfd/elfcode.h index c7da8f6c074..072220e220a 100644 --- a/bfd/elfcode.h +++ b/bfd/elfcode.h @@ -704,6 +704,9 @@ elf_object_p (bfd *abfd) elf_elfsections (abfd) = (Elf_Internal_Shdr **) bfd_alloc (abfd, amt); if (!elf_elfsections (abfd)) goto got_no_match; + elf_tdata (abfd)->being_created = bfd_zalloc (abfd, num_sec); + if (!elf_tdata (abfd)->being_created) + goto got_no_match; memcpy (i_shdrp, &i_shdr, sizeof (*i_shdrp)); for (shdrp = i_shdrp, shindex = 0; shindex < num_sec; shindex++) -- 2.30.2