[PATCH] libdwfl: Only intern CU when not EOF marker and cuoff points to a DIE.

[Mark Wielaard <mjw at redhat dot com>]

This replaces the two previously proposed patches for libdwfl/cu.c:
  libdwfl: Sanity check cu offset before trying to intern.
  libdwfl: Arange CU cannot point to the EOF marker.

The issue is kind of the same and still didn't fully solve the root
cause. This patch makes sure we never insert a bogus DIE nor an EOF
marker. It also makes sure that we only tdestroy the lazy_cu_root
when we really don't need it anymore.

The attached patch looks big, but ignoring whitespace/indenting changes
it is simply:

--- a/libdwfl/cu.c
+++ b/libdwfl/cu.c
@@ -171,6 +171,29 @@ compare_cukey (const void *a, const void *b)
 static Dwfl_Error
 intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result)
 {
+  if (unlikely (cuoff + 4 >= mod->dw->sectiondata[IDX_debug_info]->d_size))
+    {
+      if (mod->lazycu == 1)
+       {
+         /* This is the EOF marker.  Now we have interned all the CUs.
+            One increment in MOD->lazycu counts not having hit EOF yet.  */
+         *result = (void *) -1;
+         less_lazy (mod);
+         return DWFL_E_NOERROR;
+       }
+      else
+       {
+         /* Unexpected EOF, most likely a bogus aranges.  */
+         return (DWFL_E (LIBDW, DWARF_E_INVALID_DWARF));
+       }
+    }
+
+  /* Make sure the cuoff points to a real DIE.  */
+  Dwarf_Die cudie;
+  Dwarf_Die *die = INTUSE(dwarf_offdie) (mod->dw, cuoff, &cudie);
+  if (die == NULL)
+    return DWFL_E_LIBDW;
+
   struct Dwarf_CU dwkey;
   struct dwfl_cu key;
   key.die.cu = &dwkey;
@@ -182,16 +205,6 @@ intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result)
 
   if (*found == &key || *found == NULL)
     {
-      if (unlikely (cuoff + 4 >= mod->dw->sectiondata[IDX_debug_info]->d_size))
-       {
-         /* This is the EOF marker.  Now we have interned all the CUs.
-            One increment in MOD->lazycu counts not having hit EOF yet.  */
-         *found = *result = (void *) -1;
-         less_lazy (mod);
-         return DWFL_E_NOERROR;
-       }
-      else
-       {
       /* This is a new entry, meaning we haven't looked at this CU.  */
 
       *found = NULL;
@@ -203,15 +216,7 @@ intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result)
       cu->mod = mod;
       cu->next = NULL;
       cu->lines = NULL;
-
-         /* XXX use non-searching lookup */
-         Dwarf_Die *die = INTUSE(dwarf_offdie) (mod->dw, cuoff, &cu->die);
-         if (die == NULL)
-           {
-             free (cu);
-             return DWFL_E_LIBDW;
-           }
-         assert (die == &cu->die);
+      cu->die = cudie;
 
       struct dwfl_cu **newvec = realloc (mod->cu, ((mod->ncu + 1)
                                                   * sizeof (mod->cu[0])));
@@ -228,7 +233,6 @@ intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result)
 
       *found = cu;
     }
-    }
 
   *result = *found;
   return DWFL_E_NOERROR;

<--->

We need to check the cuoff points to a real Dwarf_Die before trying to
intern the cu with tsearch. Otherwise bogus keys might end up in the
search tree with NULL cus. That will cause crashes in compare_cukey
during next insertion or deletion of cus. We also don't want to insert
the EOF marker and unconditionally tdestroy the lazy_cu_root. The EOF
could be caused by bad DWARF from a bogus agranges entry.

https://bugzilla.redhat.com/show_bug.cgi?id=1170810#c30

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdwfl/ChangeLog |  5 ++++
 libdwfl/cu.c      | 84 +++++++++++++++++++++++++++++--------------------------
 2 files changed, 49 insertions(+), 40 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index de76378..47f3854 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2015-05-07  Mark Wielaard  <mjw@redhat.com>
+
+	* cu.c (intern_cu): Check for EOF and check cuoff points to a real
+	Dwarf_Die before interning. Explicitly check EOF is expected.
+
 2015-05-05  Mark Wielaard  <mjw@redhat.com>
 
 	* dwfl_lineinfo.c (dwfl_lineinfo): Check info->file is valid.
diff --git a/libdwfl/cu.c b/libdwfl/cu.c
index 3ac341e..8f35b6b 100644
--- a/libdwfl/cu.c
+++ b/libdwfl/cu.c
@@ -171,63 +171,67 @@ compare_cukey (const void *a, const void *b)
 static Dwfl_Error
 intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result)
 {
-  struct Dwarf_CU dwkey;
-  struct dwfl_cu key;
-  key.die.cu = &dwkey;
-  dwkey.offset_size = 0;
-  dwkey.start = cuoff - (3 * 0 - 4 + 3);
-  struct dwfl_cu **found = tsearch (&key, &mod->lazy_cu_root, &compare_cukey);
-  if (unlikely (found == NULL))
-    return DWFL_E_NOMEM;
-
-  if (*found == &key || *found == NULL)
+  if (unlikely (cuoff + 4 >= mod->dw->sectiondata[IDX_debug_info]->d_size))
     {
-      if (unlikely (cuoff + 4 >= mod->dw->sectiondata[IDX_debug_info]->d_size))
+      if (mod->lazycu == 1)
 	{
 	  /* This is the EOF marker.  Now we have interned all the CUs.
 	     One increment in MOD->lazycu counts not having hit EOF yet.  */
-	  *found = *result = (void *) -1;
+	  *result = (void *) -1;
 	  less_lazy (mod);
 	  return DWFL_E_NOERROR;
 	}
       else
 	{
-	  /* This is a new entry, meaning we haven't looked at this CU.  */
+	  /* Unexpected EOF, most likely a bogus aranges.  */
+	  return (DWFL_E (LIBDW, DWARF_E_INVALID_DWARF));
+	}
+    }
 
-	  *found = NULL;
+  /* Make sure the cuoff points to a real DIE.  */
+  Dwarf_Die cudie;
+  Dwarf_Die *die = INTUSE(dwarf_offdie) (mod->dw, cuoff, &cudie);
+  if (die == NULL)
+    return DWFL_E_LIBDW;
 
-	  struct dwfl_cu *cu = malloc (sizeof *cu);
-	  if (unlikely (cu == NULL))
-	    return DWFL_E_NOMEM;
+  struct Dwarf_CU dwkey;
+  struct dwfl_cu key;
+  key.die.cu = &dwkey;
+  dwkey.offset_size = 0;
+  dwkey.start = cuoff - (3 * 0 - 4 + 3);
+  struct dwfl_cu **found = tsearch (&key, &mod->lazy_cu_root, &compare_cukey);
+  if (unlikely (found == NULL))
+    return DWFL_E_NOMEM;
 
-	  cu->mod = mod;
-	  cu->next = NULL;
-	  cu->lines = NULL;
+  if (*found == &key || *found == NULL)
+    {
+      /* This is a new entry, meaning we haven't looked at this CU.  */
 
-	  /* XXX use non-searching lookup */
-	  Dwarf_Die *die = INTUSE(dwarf_offdie) (mod->dw, cuoff, &cu->die);
-	  if (die == NULL)
-	    {
-	      free (cu);
-	      return DWFL_E_LIBDW;
-	    }
-	  assert (die == &cu->die);
+      *found = NULL;
 
-	  struct dwfl_cu **newvec = realloc (mod->cu, ((mod->ncu + 1)
-						       * sizeof (mod->cu[0])));
-	  if (newvec == NULL)
-	    {
-	      free (cu);
-	      return DWFL_E_NOMEM;
-	    }
-	  mod->cu = newvec;
+      struct dwfl_cu *cu = malloc (sizeof *cu);
+      if (unlikely (cu == NULL))
+	return DWFL_E_NOMEM;
 
-	  mod->cu[mod->ncu++] = cu;
-	  if (cu->die.cu->start == 0)
-	    mod->first_cu = cu;
+      cu->mod = mod;
+      cu->next = NULL;
+      cu->lines = NULL;
+      cu->die = cudie;
 
-	  *found = cu;
+      struct dwfl_cu **newvec = realloc (mod->cu, ((mod->ncu + 1)
+						   * sizeof (mod->cu[0])));
+      if (newvec == NULL)
+	{
+	  free (cu);
+	  return DWFL_E_NOMEM;
 	}
+      mod->cu = newvec;
+
+      mod->cu[mod->ncu++] = cu;
+      if (cu->die.cu->start == 0)
+	mod->first_cu = cu;
+
+      *found = cu;
     }
 
   *result = *found;
-- 
2.1.0