This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix for reads of unallocated memory in ld

On 05/15/2014 04:11 AM, Alan Modra wrote:
On Mon, May 12, 2014 at 08:18:34AM -0700, Douglas B Rupp wrote:
diff -u -p -r1.92 elf-eh-frame.c
--- elf-eh-frame.c	21 Feb 2013 02:29:08 -0000	1.92
+++ elf-eh-frame.c	12 May 2014 15:01:57 -0000
@@ -411,7 +411,7 @@ skip_non_nops (bfd_byte *buf, bfd_byte *
    last = buf;
    while (buf < end)
      if (*buf == DW_CFA_nop)
-      buf++;
+      last = ++buf;
  	if (*buf == DW_CFA_set_loc)

This can't be right.  The idea of skip_non_nops is to remove any
trailing nop padding from the CIE, so that two CIEs that are identical
except for padding, compare equal.

OK, I understand why it's not right. Can you help me understand where this value of 50 comes from (below)? There's no comment so I assume it was chosen perhaps because it was thought to be an ample size, but it is not.

struct cie {
  unsigned char initial_instructions[50];

Later around line 765 we have:
          initial_insn_length = end - buf;
          if (initial_insn_length <= sizeof (cie->initial_instructions))
              cie->initial_insn_length = initial_insn_length;

The comparison fails because initial_insn_length is 54, so
cie->initial_insn_length remains zeroed.  Later skip_non_nops
returns a skip value that is subtracted from zero and causes this field to underflow leading to the reading of unallocated memory.

Shouldn't the "50" be at least "255", which is the maximum value that can be held in cie->initial_insn_length? (This still seems a little iffy since I don't understand how "50" was chosen in the first place)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]