readelf section reading

This is a followup to git commit 8ff66993e0, 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.
This commit is contained in:
Alan Modra 2021-06-11 14:12:07 +09:30
parent f64b9b13ce
commit 4de91c10cd
2 changed files with 74 additions and 74 deletions

View file

@ -1,3 +1,17 @@
2021-06-11 Alan Modra <amodra@gmail.com>
* 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.
2021-06-09 Nick Clifton <nickc@redhat.com>
* MAINTAINERS: Remove Daniel Jacobwitz from the maintainers list.

View file

@ -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, &section,
= get_elf_symbols (filedata, &section,
&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 ();