readelf section reading
Alan Modra
amodra@gmail.com
Fri Jun 11 08:03:12 GMT 2021
This is a followup to git commit 8ff66993e0b5, a patch aimed at
segfaults found invoking readelf multiple times with fuzzed objects.
In that patch I added code to clear more stashed data early in
process_section_headers, along with any stashed section headers. This
patch instead relies on clearing out the stash at the end of
process_object, making sure that process_object doesn't exit early.
The patch also introduces some new wrapper functions.
* readelf.c (GET_ELF_SYMBOLS): Delete. Replace with..
(get_elf_symbols): ..this new function throughout.
(get_32bit_section_headers): Don't free section_headers.
(get_64bit_section_headers): Likewise.
(get_section_headers): New function, use throughout in place of
32bit and 64bit variants.
(get_dynamic_section): Similarly.
(process_section_headers): Don't free filedata memory here.
(get_file_header): Don't get section headers here..
(process_object): ..Read them here instead. Don't exit without
freeing filedata memory.
diff --git a/binutils/readelf.c b/binutils/readelf.c
index ebb93b7dc1a..52d5302d07b 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -357,10 +357,6 @@ static const char * get_symbol_version_string
#define DT_VERSIONTAGIDX(tag) (DT_VERNEEDNUM - (tag)) /* Reverse order! */
-#define GET_ELF_SYMBOLS(file, section, sym_count) \
- (is_32bit_elf ? get_32bit_elf_symbols (file, section, sym_count) \
- : get_64bit_elf_symbols (file, section, sym_count))
-
#define VALID_SYMBOL_NAME(strtab, strtab_size, offset) \
(strtab != NULL && offset < strtab_size)
#define VALID_DYNAMIC_NAME(filedata, offset) \
@@ -5756,7 +5752,6 @@ get_32bit_section_headers (Filedata * filedata, bool probe)
if (shdrs == NULL)
return false;
- free (filedata->section_headers);
filedata->section_headers = (Elf_Internal_Shdr *)
cmalloc (num, sizeof (Elf_Internal_Shdr));
if (filedata->section_headers == NULL)
@@ -5823,7 +5818,6 @@ get_64bit_section_headers (Filedata * filedata, bool probe)
if (shdrs == NULL)
return false;
- free (filedata->section_headers);
filedata->section_headers = (Elf_Internal_Shdr *)
cmalloc (num, sizeof (Elf_Internal_Shdr));
if (filedata->section_headers == NULL)
@@ -5858,6 +5852,21 @@ get_64bit_section_headers (Filedata * filedata, bool probe)
return true;
}
+static bool
+get_section_headers (Filedata *filedata, bool probe)
+{
+ if (filedata->section_headers != NULL)
+ return true;
+
+ if (filedata->file_header.e_shoff == 0)
+ return true;
+
+ if (is_32bit_elf)
+ return get_32bit_section_headers (filedata, probe);
+ else
+ return get_64bit_section_headers (filedata, probe);
+}
+
static Elf_Internal_Sym *
get_32bit_elf_symbols (Filedata * filedata,
Elf_Internal_Shdr * section,
@@ -6094,6 +6103,17 @@ get_64bit_elf_symbols (Filedata * filedata,
return isyms;
}
+static Elf_Internal_Sym *
+get_elf_symbols (Filedata *filedata,
+ Elf_Internal_Shdr *section,
+ unsigned long *num_syms_return)
+{
+ if (is_32bit_elf)
+ return get_32bit_elf_symbols (filedata, section, num_syms_return);
+ else
+ return get_64bit_elf_symbols (filedata, section, num_syms_return);
+}
+
static const char *
get_elf_section_flags (Filedata * filedata, bfd_vma sh_flags)
{
@@ -6451,23 +6471,6 @@ process_section_headers (Filedata * filedata)
Elf_Internal_Shdr * section;
unsigned int i;
- free (filedata->section_headers);
- filedata->section_headers = NULL;
- free (filedata->dynamic_symbols);
- filedata->dynamic_symbols = NULL;
- filedata->num_dynamic_syms = 0;
- free (filedata->dynamic_strings);
- filedata->dynamic_strings = NULL;
- filedata->dynamic_strings_length = 0;
- free (filedata->dynamic_syminfo);
- filedata->dynamic_syminfo = NULL;
- while (filedata->symtab_shndx_list != NULL)
- {
- elf_section_list *next = filedata->symtab_shndx_list->next;
- free (filedata->symtab_shndx_list);
- filedata->symtab_shndx_list = next;
- }
-
if (filedata->file_header.e_shnum == 0)
{
/* PR binutils/12467. */
@@ -6497,16 +6500,8 @@ process_section_headers (Filedata * filedata)
(unsigned long) filedata->file_header.e_shoff);
}
- if (is_32bit_elf)
- {
- if (! get_32bit_section_headers (filedata, false))
- return false;
- }
- else
- {
- if (! get_64bit_section_headers (filedata, false))
- return false;
- }
+ if (!get_section_headers (filedata, false))
+ return false;
/* Read in the string table, so that we have names to display. */
if (filedata->file_header.e_shstrndx != SHN_UNDEF
@@ -6615,7 +6610,7 @@ process_section_headers (Filedata * filedata)
CHECK_ENTSIZE (section, i, Sym);
filedata->dynamic_symbols
- = GET_ELF_SYMBOLS (filedata, section, &filedata->num_dynamic_syms);
+ = get_elf_symbols (filedata, section, &filedata->num_dynamic_syms);
filedata->dynamic_symtab_section = section;
break;
@@ -7170,7 +7165,7 @@ get_symtab (Filedata *filedata, Elf_Internal_Shdr *symsec,
{
*strtab = NULL;
*strtablen = 0;
- *symtab = GET_ELF_SYMBOLS (filedata, symsec, nsyms);
+ *symtab = get_elf_symbols (filedata, symsec, nsyms);
if (*symtab == NULL)
return false;
@@ -7341,7 +7336,7 @@ process_section_groups (Filedata * filedata)
{
symtab_sec = sec;
free (symtab);
- symtab = GET_ELF_SYMBOLS (filedata, symtab_sec, & num_syms);
+ symtab = get_elf_symbols (filedata, symtab_sec, & num_syms);
}
if (symtab == NULL)
@@ -10278,6 +10273,18 @@ get_64bit_dynamic_section (Filedata * filedata)
return true;
}
+static bool
+get_dynamic_section (Filedata *filedata)
+{
+ if (filedata->dynamic_section)
+ return true;
+
+ if (is_32bit_elf)
+ return get_32bit_dynamic_section (filedata);
+ else
+ return get_64bit_dynamic_section (filedata);
+}
+
static void
print_dynamic_flags (bfd_vma flags)
{
@@ -10621,16 +10628,8 @@ process_dynamic_section (Filedata * filedata)
return true;
}
- if (is_32bit_elf)
- {
- if (! get_32bit_dynamic_section (filedata))
- return false;
- }
- else
- {
- if (! get_64bit_dynamic_section (filedata))
- return false;
- }
+ if (!get_dynamic_section (filedata))
+ return false;
/* Find the appropriate symbol table. */
if (filedata->dynamic_symbols == NULL || do_histogram)
@@ -10714,7 +10713,7 @@ the .dynsym section doesn't match the DT_SYMTAB and DT_SYMENT tags\n"));
section.sh_name = filedata->string_table_length;
filedata->dynamic_symbols
- = GET_ELF_SYMBOLS (filedata, §ion,
+ = get_elf_symbols (filedata, §ion,
&filedata->num_dynamic_syms);
if (filedata->dynamic_symbols == NULL
|| filedata->num_dynamic_syms != num_of_syms)
@@ -11744,7 +11743,7 @@ process_version_sections (Filedata * filedata)
found = true;
- symbols = GET_ELF_SYMBOLS (filedata, link_section, & num_syms);
+ symbols = get_elf_symbols (filedata, link_section, & num_syms);
if (symbols == NULL)
break;
@@ -12946,7 +12945,7 @@ process_symbol_table (Filedata * filedata)
else
printf (_(" Num: Value Size Type Bind Vis Ndx Name\n"));
- symtab = GET_ELF_SYMBOLS (filedata, section, & num_syms);
+ symtab = get_elf_symbols (filedata, section, & num_syms);
if (symtab == NULL)
continue;
@@ -14312,7 +14311,7 @@ apply_relocations (Filedata * filedata,
if (filedata->file_header.e_machine == EM_SH)
is_rela = false;
- symtab = GET_ELF_SYMBOLS (filedata, symsec, & num_syms);
+ symtab = get_elf_symbols (filedata, symsec, & num_syms);
for (rp = relocs; rp < relocs + num_relocs; ++rp)
{
@@ -21185,16 +21184,6 @@ get_file_header (Filedata * filedata)
filedata->file_header.e_shstrndx = BYTE_GET (ehdr64.e_shstrndx);
}
- if (filedata->file_header.e_shoff)
- {
- /* There may be some extensions in the first section header. Don't
- bomb if we can't read it. */
- if (is_32bit_elf)
- get_32bit_section_headers (filedata, true);
- else
- get_64bit_section_headers (filedata, true);
- }
-
return true;
}
@@ -21305,19 +21294,8 @@ open_file (const char * pathname, bool is_separate)
if (! get_file_header (filedata))
goto fail;
- if (filedata->file_header.e_shoff)
- {
- bool res;
-
- /* Read the section headers again, this time for real. */
- if (is_32bit_elf)
- res = get_32bit_section_headers (filedata, false);
- else
- res = get_64bit_section_headers (filedata, false);
-
- if (!res)
- goto fail;
- }
+ if (!get_section_headers (filedata, false))
+ goto fail;
return filedata;
@@ -21393,8 +21371,15 @@ process_object (Filedata * filedata)
initialise_dump_sects (filedata);
+ /* There may be some extensions in the first section header. Don't
+ bomb if we can't read it. */
+ get_section_headers (filedata, true);
+
if (! process_file_header (filedata))
- return false;
+ {
+ res = false;
+ goto out;
+ }
if (! process_section_headers (filedata))
{
@@ -21490,6 +21475,7 @@ process_object (Filedata * filedata)
if (! process_arch_specific (filedata))
res = false;
+ out:
free_filedata (filedata);
free_debug_memory ();
--
Alan Modra
Australia Development Lab, IBM
More information about the Binutils
mailing list