msan: use of uninitialised data in get_cie_info

This completely bogus oss-fuzz x86 testcase results in a read from an
uninitialised (at the time check_eh_frame is called) part of an insn
frag:
 .section .debug_frame
 orl $1,x
 .long x
 .uleb128 0,x,0
x:

Fix the problem by verifying the assumption in get_cie_info that a CIE
starts at the beginning of .eh_frame or .debug_frame.  Or at least
exclude silliness involving instructions placed there.  That seems a
useful sanity check.  Also sanity check sizes of initial FDE fields.

Yes, this doesn't completely stop the problem since you could place an
insn with a relocated field later in the CIE.  If fuzzers find such a
testcase I'll ignore it.

	* ehopt.c (struct cie_info): Add "f" field.
	(get_cie_info): Return a bool.  Verify frag at start of chain
	is one with the CIE size found by check_eh_frame.
	(check_eh_frame): Save CIE start frag.  Only accept 4 or 8
	byte fields in state_saw_size, state_saw_cie_offset and
	state_saw_pc_begin.  Formatting.  Localise "fix" variable.

diff --git a/gas/ehopt.c b/gas/ehopt.c
index 3e15fc91ec4963bdcd5adc87aa299d6dbd1a3965..ab976ba5e6d5628335472183bda4729190c6d41b 100644 (file)
--- a/gas/ehopt.c
+++ b/gas/ehopt.c
@@ -90,17 +90,17 @@ __FRAME_BEGIN__:
 
 struct cie_info
 {
+  fragS *f;
   unsigned code_alignment;
   int z_augmentation;
 };
 
 /* Extract information from the CIE.  */
 
-static int
+static bool
 get_cie_info (struct cie_info *info)
 {
   fragS *f;
-  fixS *fix;
   unsigned int offset;
   char CIE_id;
   char augmentation[10];
@@ -110,9 +110,10 @@ get_cie_info (struct cie_info *info)
   /* We should find the CIE at the start of the section.  */
 
   f = seg_info (now_seg)->frchainP->frch_root;
-  fix = seg_info (now_seg)->frchainP->fix_root;
-
-  /* Look through the frags of the section to find the code alignment.  */
+  while (f != NULL && f->fr_fix == 0)
+    f = f->fr_next;
+  if (f != info->f)
+    return false;
 
   /* First make sure that the CIE Identifier Tag is 0/-1.  */
 
@@ -133,7 +134,7 @@ get_cie_info (struct cie_info *info)
       || f->fr_literal[offset + 1] != CIE_id
       || f->fr_literal[offset + 2] != CIE_id
       || f->fr_literal[offset + 3] != CIE_id)
-    return 0;
+    return false;
 
   /* Next make sure the CIE version number is 1.  */
 
@@ -146,7 +147,7 @@ get_cie_info (struct cie_info *info)
   if (f == NULL
       || f->fr_fix - offset < 1
       || f->fr_literal[offset] != 1)
-    return 0;
+    return false;
 
   /* Skip the augmentation (a null terminated string).  */
 
@@ -160,7 +161,7 @@ get_cie_info (struct cie_info *info)
          f = f->fr_next;
        }
       if (f == NULL)
-       return 0;
+       return false;
 
       while (offset < f->fr_fix && f->fr_literal[offset] != '\0')
        {
@@ -181,7 +182,7 @@ get_cie_info (struct cie_info *info)
       f = f->fr_next;
     }
   if (f == NULL)
-    return 0;
+    return false;
 
   augmentation[iaug] = '\0';
   if (augmentation[0] == '\0')
@@ -192,6 +193,7 @@ get_cie_info (struct cie_info *info)
     {
       /* We have to skip a pointer.  Unfortunately, we don't know how
         large it is.  We find out by looking for a matching fixup.  */
+      fixS *fix = seg_info (now_seg)->frchainP->fix_root;
       while (fix != NULL
             && (fix->fx_frag != f || fix->fx_where != offset))
        fix = fix->fx_next;
@@ -205,10 +207,10 @@ get_cie_info (struct cie_info *info)
          f = f->fr_next;
        }
       if (f == NULL)
-       return 0;
+       return false;
     }
   else if (augmentation[0] != 'z')
-    return 0;
+    return false;
 
   /* We're now at the code alignment factor, which is a ULEB128.  If
      it isn't a single byte, forget it.  */
@@ -220,7 +222,7 @@ get_cie_info (struct cie_info *info)
   info->code_alignment = code_alignment;
   info->z_augmentation = (augmentation[0] == 'z');
 
-  return 1;
+  return true;
 }
 
 enum frame_state
@@ -240,7 +242,7 @@ struct frame_data
 {
   enum frame_state state;
 
-  int cie_info_ok;
+  bool cie_info_ok;
   struct cie_info cie_info;
 
   symbolS *size_end_sym;
@@ -320,20 +322,27 @@ check_eh_frame (expressionS *exp, unsigned int *pnbytes)
            {
              d->state = state_saw_size;
              d->size_end_sym = exp->X_add_symbol;
+             if (!d->cie_info.f)
+               d->cie_info.f = frag_now;
            }
        }
       break;
 
     case state_saw_size:
     case state_saw_cie_offset:
-      /* Assume whatever form it appears in, it appears atomically.  */
-      d->state = (enum frame_state) (d->state + 1);
+      if (!(*pnbytes == 4 || *pnbytes == 8))
+       /* Stop scanning if we don't see the expected FDE fields.  */
+       d->state = state_error;
+      else
+       d->state = (enum frame_state) (d->state + 1);
       break;
 
     case state_saw_pc_begin:
       /* Decide whether we should see an augmentation.  */
-      if (! d->cie_info_ok
-         && ! (d->cie_info_ok = get_cie_info (&d->cie_info)))
+      if (!(*pnbytes == 4 || *pnbytes == 8))
+       d->state = state_error;
+      else if (!d->cie_info_ok
+              && !(d->cie_info_ok = get_cie_info (&d->cie_info)))
        d->state = state_error;
       else if (d->cie_info.z_augmentation)
        {
@@ -347,7 +356,7 @@ check_eh_frame (expressionS *exp, unsigned int *pnbytes)
 
     case state_seeing_aug_size:
       /* Bytes == -1 means this comes from an leb128 directive.  */
-      if ((int)*pnbytes == -1 && exp->X_op == O_constant)
+      if ((int) *pnbytes == -1 && exp->X_op == O_constant)
        {
          d->aug_size = exp->X_add_number;
          d->state = state_skipping_aug;
@@ -367,7 +376,7 @@ check_eh_frame (expressionS *exp, unsigned int *pnbytes)
       break;
 
     case state_skipping_aug:
-      if ((int)*pnbytes < 0)
+      if ((int) *pnbytes < 0)
        d->state = state_error;
       else
        {