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

[Douglas B Rupp <rupp at adacore dot com>]

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;
> >       else
> >         {
> >   	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.

elf-eh-frame.c/
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)