From f84ce13b6708801ca1d6289b7c4003e2f5a6d7f9 Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Mon, 13 Feb 2017 14:03:22 +0000 Subject: [PATCH] Fix read-after-free error in readelf when processing multiple, relocated sections in an MSP430 binary. PR binutils/21139 * readelf.c (target_specific_reloc_handling): Add num_syms parameter. Check for symbol table overflow before accessing symbol value. If reloc pointer is NULL, discard all saved state. (apply_relocations): Pass num_syms to target_specific_reloc_handling. Call target_specific_reloc_handling with a NULL reloc pointer after processing all of the relocs. --- binutils/ChangeLog | 10 +++++ binutils/readelf.c | 109 ++++++++++++++++++++++++++++++++++----------- 2 files changed, 94 insertions(+), 25 deletions(-) diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 6480c8f58f8..254268945c9 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,13 @@ +2017-02-13 Nick Clifton + + PR binutils/21139 + * readelf.c (target_specific_reloc_handling): Add num_syms + parameter. Check for symbol table overflow before accessing + symbol value. If reloc pointer is NULL, discard all saved state. + (apply_relocations): Pass num_syms to target_specific_reloc_handling. + Call target_specific_reloc_handling with a NULL reloc pointer + after processing all of the relocs. + 2017-02-13 Nick Clifton PR binutils/21137 diff --git a/binutils/readelf.c b/binutils/readelf.c index e474f277b3a..de961c48303 100644 --- a/binutils/readelf.c +++ b/binutils/readelf.c @@ -11586,15 +11586,27 @@ process_syminfo (FILE * file ATTRIBUTE_UNUSED) /* Check to see if the given reloc needs to be handled in a target specific manner. If so then process the reloc and return TRUE otherwise return - FALSE. */ + FALSE. + + If called with reloc == NULL, then this is a signal that reloc processing + for the current section has finished, and any saved state should be + discarded. */ static bfd_boolean target_specific_reloc_handling (Elf_Internal_Rela * reloc, unsigned char * start, unsigned char * end, - Elf_Internal_Sym * symtab) + Elf_Internal_Sym * symtab, + unsigned long num_syms) { - unsigned int reloc_type = get_reloc_type (reloc->r_info); + unsigned int reloc_type = 0; + unsigned long sym_index = 0; + + if (reloc) + { + reloc_type = get_reloc_type (reloc->r_info); + sym_index = get_reloc_symindex (reloc->r_info); + } switch (elf_header.e_machine) { @@ -11603,6 +11615,12 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, { static Elf_Internal_Sym * saved_sym = NULL; + if (reloc == NULL) + { + saved_sym = NULL; + return TRUE; + } + switch (reloc_type) { case 10: /* R_MSP430_SYM_DIFF */ @@ -11610,7 +11628,12 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, break; /* Fall through. */ case 21: /* R_MSP430X_SYM_DIFF */ - saved_sym = symtab + get_reloc_symindex (reloc->r_info); + /* PR 21139. */ + if (sym_index >= num_syms) + error (_("MSP430 SYM_DIFF reloc contains invalid symbol index %lu\n"), + sym_index); + else + saved_sym = symtab + sym_index; return TRUE; case 1: /* R_MSP430_32 or R_MSP430_ABS32 */ @@ -11635,16 +11658,21 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, int reloc_size = reloc_type == 1 ? 4 : 2; bfd_vma value; - value = reloc->r_addend - + (symtab[get_reloc_symindex (reloc->r_info)].st_value - - saved_sym->st_value); - - if (start + reloc->r_offset + reloc_size >= end) - /* PR 21137 */ - error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"), - start + reloc->r_offset + reloc_size, end); + if (sym_index >= num_syms) + error (_("MSP430 reloc contains invalid symbol index %lu\n"), + sym_index); else - byte_put (start + reloc->r_offset, value, reloc_size); + { + value = reloc->r_addend + (symtab[sym_index].st_value + - saved_sym->st_value); + + if (start + reloc->r_offset + reloc_size >= end) + /* PR 21137 */ + error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"), + start + reloc->r_offset + reloc_size, end); + else + byte_put (start + reloc->r_offset, value, reloc_size); + } saved_sym = NULL; return TRUE; @@ -11664,13 +11692,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, { static Elf_Internal_Sym * saved_sym = NULL; + if (reloc == NULL) + { + saved_sym = NULL; + return TRUE; + } + switch (reloc_type) { case 34: /* R_MN10300_ALIGN */ return TRUE; case 33: /* R_MN10300_SYM_DIFF */ - saved_sym = symtab + get_reloc_symindex (reloc->r_info); + if (sym_index >= num_syms) + error (_("MN10300_SYM_DIFF reloc contains invalid symbol index %lu\n"), + sym_index); + else + saved_sym = symtab + sym_index; return TRUE; + case 1: /* R_MN10300_32 */ case 2: /* R_MN10300_16 */ if (saved_sym != NULL) @@ -11678,15 +11717,20 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, int reloc_size = reloc_type == 1 ? 4 : 2; bfd_vma value; - value = reloc->r_addend - + (symtab[get_reloc_symindex (reloc->r_info)].st_value - - saved_sym->st_value); - - if (start + reloc->r_offset + reloc_size >= end) - error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"), - start + reloc->r_offset + reloc_size, end); + if (sym_index >= num_syms) + error (_("MN10300 reloc contains invalid symbol index %lu\n"), + sym_index); else - byte_put (start + reloc->r_offset, value, reloc_size); + { + value = reloc->r_addend + (symtab[sym_index].st_value + - saved_sym->st_value); + + if (start + reloc->r_offset + reloc_size >= end) + error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"), + start + reloc->r_offset + reloc_size, end); + else + byte_put (start + reloc->r_offset, value, reloc_size); + } saved_sym = NULL; return TRUE; @@ -11706,12 +11750,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, static bfd_vma saved_sym2 = 0; static bfd_vma value; + if (reloc == NULL) + { + saved_sym1 = saved_sym2 = 0; + return TRUE; + } + switch (reloc_type) { case 0x80: /* R_RL78_SYM. */ saved_sym1 = saved_sym2; - saved_sym2 = symtab[get_reloc_symindex (reloc->r_info)].st_value; - saved_sym2 += reloc->r_addend; + if (sym_index >= num_syms) + error (_("RL78_SYM reloc contains invalid symbol index %lu\n"), + sym_index); + else + { + saved_sym2 = symtab[sym_index].st_value; + saved_sym2 += reloc->r_addend; + } return TRUE; case 0x83: /* R_RL78_OPsub. */ @@ -12360,7 +12416,7 @@ apply_relocations (void * file, reloc_type = get_reloc_type (rp->r_info); - if (target_specific_reloc_handling (rp, start, end, symtab)) + if (target_specific_reloc_handling (rp, start, end, symtab, num_syms)) continue; else if (is_none_reloc (reloc_type)) continue; @@ -12456,6 +12512,9 @@ apply_relocations (void * file, } free (symtab); + /* Let the target specific reloc processing code know that + we have finished with these relocs. */ + target_specific_reloc_handling (NULL, NULL, NULL, NULL, 0); if (relocs_return) { -- 2.30.2