Re: eh-frame: CIE initial_instructions overflow

[Alan Modra <amodra at gmail dot com>]

On Thu, Dec 19, 2013 at 04:46:32PM +0100, Christophe Lyon wrote:
> On 19 December 2013 12:31, nick clifton <nickc@redhat.com> wrote:
> > Hi Christophe,
> >
> >
> >> Currently, in bfd/elf-eh-frame.c we have a definition of struct cie
> >> which ends with:
> >>    unsigned char initial_instructions[50];
> >
> >
> > This is a bug...
> >
> >
> >> In _bfd_elf_parse_eh_frame(), we have:
> >>        initial_insn_length = end - buf;
> >>        if (initial_insn_length <= sizeof (cie->initial_instructions))
> >>          {
> >>            cie->initial_insn_length = initial_insn_length;
> >>            memcpy (cie->initial_instructions, buf, initial_insn_length);
> >>          }
> >
> >
> > IMHO - there should be no fixed size for the initial_instructions buffer.
> > Instead the code at this point should allocate and copy the buffer that has
> > just been created.
> >
> That's my feeling too, but I asked because this is going to be a
> rather intrusive change: the surrounding code allocates an array of
> CIE after having counted them.
> Having a dynamic size here means we have to parse all the CIEs once to
> get the biggest size, then allocate local_cies accordingly, then
> reparse the section contents.
> 
> Not sure how costly it is to parse CIEs in general? Is there a risk to
> increase link times significantly ?

cie.initial_instructions[] is used when deciding whether CIEs can be
merged.  If CIEs with larger numbers of insns than we can handle are
rare (and we might need to bump the size of cie.initial_instructions[]
to make that true) then we can simply say that such CIEs can't be
merged.  I don't think I'd bother with variable size buffers.
Something like the following ought to work.

	* elf-eh-frame.c (cie_eq): Return false when initial_insn_length
	is too large.
	(cie_compute_hash): Don't exceed bounds of initial_instructions.
	(_bfd_elf_parse_eh_frame): Always set initial_insn_length, and
	save as much of insns to initial_instructions[] as will fit.

diff --git a/bfd/elf-eh-frame.c b/bfd/elf-eh-frame.c
index 832a991..4b6e8ea 100644
--- a/bfd/elf-eh-frame.c
+++ b/bfd/elf-eh-frame.c
@@ -235,6 +235,7 @@ cie_eq (const void *e1, const void *e2)
       && c1->lsda_encoding == c2->lsda_encoding
       && c1->fde_encoding == c2->fde_encoding
       && c1->initial_insn_length == c2->initial_insn_length
+      && c1->initial_insn_length <= sizeof (c1->initial_instructions)
       && memcmp (c1->initial_instructions,
 		 c2->initial_instructions,
 		 c1->initial_insn_length) == 0)
@@ -254,6 +255,7 @@ static hashval_t
 cie_compute_hash (struct cie *c)
 {
   hashval_t h = 0;
+  size_t len;
   h = iterative_hash_object (c->length, h);
   h = iterative_hash_object (c->version, h);
   h = iterative_hash (c->augmentation, strlen (c->augmentation) + 1, h);
@@ -267,7 +269,10 @@ cie_compute_hash (struct cie *c)
   h = iterative_hash_object (c->lsda_encoding, h);
   h = iterative_hash_object (c->fde_encoding, h);
   h = iterative_hash_object (c->initial_insn_length, h);
-  h = iterative_hash (c->initial_instructions, c->initial_insn_length, h);
+  len = c->initial_insn_length;
+  if (len > sizeof (c->initial_instructions))
+    len = sizeof (c->initial_instructions);
+  h = iterative_hash (c->initial_instructions, len, h);
   c->hash = h;
   return h;
 }
@@ -762,11 +767,10 @@ _bfd_elf_parse_eh_frame (bfd *abfd, struct bfd_link_info *info,
 	    cie->fde_encoding = DW_EH_PE_absptr;
 
 	  initial_insn_length = end - buf;
-	  if (initial_insn_length <= sizeof (cie->initial_instructions))
-	    {
-	      cie->initial_insn_length = initial_insn_length;
-	      memcpy (cie->initial_instructions, buf, initial_insn_length);
-	    }
+	  cie->initial_insn_length = initial_insn_length;
+	  memcpy (cie->initial_instructions, buf,
+		  initial_insn_length <= sizeof (cie->initial_instructions)
+		  ? initial_insn_length : sizeof (cie->initial_instructions));
 	  insns = buf;
 	  buf += initial_insn_length;
 	  ENSURE_NO_RELOCS (buf);


-- 
Alan Modra
Australia Development Lab, IBM