Put bfd_section_from_shdr loop detection array in elf_tdata
Alan Modra
amodra@gmail.com
Mon Dec 14 13:10:00 GMT 2020
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.
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index e9c890f6f1..53b5ffc33d 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 419c5f4420..7f2237655d 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 c7da8f6c07..072220e220 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++)
--
Alan Modra
Australia Development Lab, IBM
More information about the Binutils
mailing list