From: Alan Modra Date: Fri, 11 Jun 2021 04:42:07 +0000 (+0930) Subject: readelf section reading X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=4de91c10cdd9;p=binutils-gdb.git readelf section reading 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/ChangeLog b/binutils/ChangeLog index 76324997e9d..bd91a3911d2 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,17 @@ +2021-06-11 Alan Modra + + * 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 * MAINTAINERS: Remove Daniel Jacobwitz from the maintainers list. 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 ();