Fix read-after-free error in readelf when processing multiple, relocated sections...

	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.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 6480c8f58f80322dab674f2cd7b51328ca9c030f..254268945c97d4c64e58216af1f30c4c54255252 100644 (file)
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,13 @@
+2017-02-13  Nick Clifton  <nickc@redhat.com>
+
+       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  <nickc@redhat.com>
 
        PR binutils/21137

diff --git a/binutils/readelf.c b/binutils/readelf.c
index e474f277b3ad8c233f7de6242e26adeddc8d50c2..de961c4830316c6666e7c80b6bbdb8168498a920 100644 (file)
--- 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)
        {