This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils 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] libdw: Prevent infinite recursion when processing DW_TAG_imported_unit.


On Mon, 2015-01-12 at 03:15 +0100, Petr Machata wrote:
> Mark Wielaard <mjw@redhat.com> writes:
> 
> > +  /* Checks the given DIE hasn't been imported yet to prevent cycles.  */
> > +  inline bool imports_contains (Dwarf_Die *die)
> > +  {
> > +    struct Dwarf_Die_Chain *import = imports;
> > +    while (import != NULL)
> > +      {
> > +	if (import->die.addr == die->addr)
> > +	  return true;
> > +	import = import->parent;
> > +      }
> 
> This is equivalent but is IMHO canonical:
> 
>     for (Dwarf_Die_Chain *import = imports; import != NULL;
>          import = import->parent)
>       if (import->die.addr == die->addr)
>         return true;

Yes, nicer, less braces.

Also "fixed up" this:

diff --git a/libdw/libdw_visit_scopes.c b/libdw/libdw_visit_scopes.c
index 8b4ae1f..1b8941f 100644
--- a/libdw/libdw_visit_scopes.c
+++ b/libdw/libdw_visit_scopes.c
@@ -117,13 +117,13 @@ __libdw_visit_scopes (depth, root, imports, previsit, postvisit, arg)
            if (INTUSE(dwarf_formref_die) (attr, &child.die) != NULL
                 && INTUSE(dwarf_child) (&child.die, &child.die) == 0)
              {
-               if (imports_contains (&child.die))
+               if (imports_contains (&orig_child_die))
                  {
                    __libdw_seterrno (DWARF_E_INVALID_DWARF);
                    return -1;
                  }
                struct Dwarf_Die_Chain *orig_imports = imports;
-               struct Dwarf_Die_Chain import = { .die = child.die,
+               struct Dwarf_Die_Chain import = { .die = orig_child_die,
                                                  .parent = orig_imports };
                imports = &import;
                int result = walk_children ();

We were storing and checking against the first child of the imported DIE
instead of the importing DIE. The effect is the same (theoretically
there could be a difference if there were no children, but in that case
we didn't process anything anyway). And I think it is slightly clearer
recording the importing DIEs (and easier when debugging to see how the
import chain looks).

Pushed to master as attached with those two changes.

Thanks,

Mark
From daf278fda6d9bd329b517757f9ba2d74619f3be7 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Sun, 11 Jan 2015 15:00:52 +0100
Subject: [PATCH] libdw: Prevent infinite recursion when processing
 DW_TAG_imported_unit.

Invalid DWARF could create cycles with DW_TAG_imported_unit, which would
lead to infinite recursion and stack overflow in libdw_visit_scopes.
Keep track of imported units and error out when a cycle is detected.

Found by afl-fuzz.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdw/ChangeLog             | 13 +++++++++++++
 libdw/dwarf_func_inline.c   |  4 ++--
 libdw/dwarf_getfuncs.c      |  4 ++--
 libdw/dwarf_getscopes.c     |  8 ++++----
 libdw/dwarf_getscopes_die.c |  4 ++--
 libdw/libdwP.h              |  5 +++--
 libdw/libdw_visit_scopes.c  | 28 +++++++++++++++++++++++++---
 7 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index abc2d71..edafe97 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,16 @@
+2015-01-11  Mark Wielaard  <mjw@redhat.com>
+
+	* dwarf_func_inline.c (dwarf_func_inline_instances): Call
+	__libdw_visit_scopes with NULL imports.
+	* dwarf_getfuncs.c (dwarf_getfuncs): Likewise.
+	* dwarf_getscopes.c (pc_record): Likewise.
+	(dwarf_getscopes): Likewise.
+	* dwarf_getscopes_die.c (dwarf_getscopes_die): Likewise.
+	* libdwP.h (__libdw_visit_scopes): Add imports argument.
+	* libdw_visit_scopes.c (__libdw_visit_scopes): Likewise. Add new
+	function imports_contains. Push and pop imports around walk_children
+	when processing DW_TAG_imported_unit.
+
 2014-12-18  Ulrich Drepper  <drepper@gmail.com>
 
 	* Makefile.am: Suppress output of textrel_check command.
diff --git a/libdw/dwarf_func_inline.c b/libdw/dwarf_func_inline.c
index bc9db1c..1f04adf 100644
--- a/libdw/dwarf_func_inline.c
+++ b/libdw/dwarf_func_inline.c
@@ -1,5 +1,5 @@
 /* Convenience functions for handling DWARF descriptions of inline functions.
-   Copyright (C) 2005,2006 Red Hat, Inc.
+   Copyright (C) 2005,2006,2015 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -97,5 +97,5 @@ dwarf_func_inline_instances (Dwarf_Die *func,
 {
   struct visitor_info v = { func->addr, callback, arg };
   struct Dwarf_Die_Chain cu = { .die = CUDIE (func->cu), .parent = NULL };
-  return __libdw_visit_scopes (0, &cu, &scope_visitor, NULL, &v);
+  return __libdw_visit_scopes (0, &cu, NULL, &scope_visitor, NULL, &v);
 }
diff --git a/libdw/dwarf_getfuncs.c b/libdw/dwarf_getfuncs.c
index f79b0a7..b95f06f 100644
--- a/libdw/dwarf_getfuncs.c
+++ b/libdw/dwarf_getfuncs.c
@@ -1,5 +1,5 @@
 /* Get function information.
-   Copyright (C) 2005, 2013 Red Hat, Inc.
+   Copyright (C) 2005, 2013, 2015 Red Hat, Inc.
    This file is part of elfutils.
    Written by Ulrich Drepper <drepper@redhat.com>, 2005.
 
@@ -109,7 +109,7 @@ dwarf_getfuncs (Dwarf_Die *cudie, int (*callback) (Dwarf_Die *, void *),
   struct visitor_info v = { callback, arg, (void *) offset, NULL, c_cu };
   struct Dwarf_Die_Chain chain = { .die = CUDIE (cudie->cu),
 				   .parent = NULL };
-  int res = __libdw_visit_scopes (0, &chain, &tree_visitor, NULL, &v);
+  int res = __libdw_visit_scopes (0, &chain, NULL, &tree_visitor, NULL, &v);
 
   if (res == DWARF_CB_ABORT)
     return (ptrdiff_t) v.last_addr;
diff --git a/libdw/dwarf_getscopes.c b/libdw/dwarf_getscopes.c
index 0ca6da0..df480d3 100644
--- a/libdw/dwarf_getscopes.c
+++ b/libdw/dwarf_getscopes.c
@@ -1,5 +1,5 @@
 /* Return scope DIEs containing PC address.
-   Copyright (C) 2005, 2007 Red Hat, Inc.
+   Copyright (C) 2005, 2007, 2015 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -176,7 +176,7 @@ pc_record (unsigned int depth, struct Dwarf_Die_Chain *die, void *arg)
      If we don't find it, return to search the containing scope.
      If we do find it, the nonzero return value will bail us out
      of the postorder traversal.  */
-  return __libdw_visit_scopes (depth, die, &origin_match, NULL, a);
+  return __libdw_visit_scopes (depth, die, NULL, &origin_match, NULL, a);
 }
 
 
@@ -189,10 +189,10 @@ dwarf_getscopes (Dwarf_Die *cudie, Dwarf_Addr pc, Dwarf_Die **scopes)
   struct Dwarf_Die_Chain cu = { .parent = NULL, .die = *cudie };
   struct args a = { .pc = pc };
 
-  int result = __libdw_visit_scopes (0, &cu, &pc_match, &pc_record, &a);
+  int result = __libdw_visit_scopes (0, &cu, NULL, &pc_match, &pc_record, &a);
 
   if (result == 0 && a.scopes != NULL)
-    result = __libdw_visit_scopes (0, &cu, &origin_match, NULL, &a);
+    result = __libdw_visit_scopes (0, &cu, NULL, &origin_match, NULL, &a);
 
   if (result > 0)
     *scopes = a.scopes;
diff --git a/libdw/dwarf_getscopes_die.c b/libdw/dwarf_getscopes_die.c
index d361585..8e2e41d 100644
--- a/libdw/dwarf_getscopes_die.c
+++ b/libdw/dwarf_getscopes_die.c
@@ -1,5 +1,5 @@
 /* Return scope DIEs containing given DIE.
-   Copyright (C) 2005 Red Hat, Inc.
+   Copyright (C) 2005, 2015 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -67,7 +67,7 @@ dwarf_getscopes_die (Dwarf_Die *die, Dwarf_Die **scopes)
 
   struct Dwarf_Die_Chain cu = { .die = CUDIE (die->cu), .parent = NULL };
   void *info = die->addr;
-  int result = __libdw_visit_scopes (1, &cu, &scope_visitor, NULL, &info);
+  int result = __libdw_visit_scopes (1, &cu, NULL, &scope_visitor, NULL, &info);
   if (result > 0)
     *scopes = info;
   return result;
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index 5ab7219..e38e0c9 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -1,5 +1,5 @@
 /* Internal definitions for libdwarf.
-   Copyright (C) 2002-2011, 2013, 2014 Red Hat, Inc.
+   Copyright (C) 2002-2011, 2013-2015 Red Hat, Inc.
    This file is part of elfutils.
    Written by Ulrich Drepper <drepper@redhat.com>, 2002.
 
@@ -555,6 +555,7 @@ struct Dwarf_Die_Chain
 };
 extern int __libdw_visit_scopes (unsigned int depth,
 				 struct Dwarf_Die_Chain *root,
+				 struct Dwarf_Die_Chain *imports,
 				 int (*previsit) (unsigned int depth,
 						  struct Dwarf_Die_Chain *,
 						  void *arg),
@@ -562,7 +563,7 @@ extern int __libdw_visit_scopes (unsigned int depth,
 						   struct Dwarf_Die_Chain *,
 						   void *arg),
 				 void *arg)
-  __nonnull_attribute__ (2, 3) internal_function;
+  __nonnull_attribute__ (2, 4) internal_function;
 
 /* Parse a DWARF Dwarf_Block into an array of Dwarf_Op's,
    and cache the result (via tsearch).  */
diff --git a/libdw/libdw_visit_scopes.c b/libdw/libdw_visit_scopes.c
index 487375d..ac7e853 100644
--- a/libdw/libdw_visit_scopes.c
+++ b/libdw/libdw_visit_scopes.c
@@ -1,5 +1,5 @@
 /* Helper functions to descend DWARF scope trees.
-   Copyright (C) 2005,2006,2007 Red Hat, Inc.
+   Copyright (C) 2005,2006,2007,2015 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -65,9 +65,10 @@ may_have_scopes (Dwarf_Die *die)
 }
 
 int
-__libdw_visit_scopes (depth, root, previsit, postvisit, arg)
+__libdw_visit_scopes (depth, root, imports, previsit, postvisit, arg)
      unsigned int depth;
      struct Dwarf_Die_Chain *root;
+     struct Dwarf_Die_Chain *imports;
      int (*previsit) (unsigned int depth, struct Dwarf_Die_Chain *, void *);
      int (*postvisit) (unsigned int depth, struct Dwarf_Die_Chain *, void *);
      void *arg;
@@ -81,10 +82,21 @@ __libdw_visit_scopes (depth, root, previsit, postvisit, arg)
 
   inline int recurse (void)
     {
-      return __libdw_visit_scopes (depth + 1, &child,
+      return __libdw_visit_scopes (depth + 1, &child, imports,
 				   previsit, postvisit, arg);
     }
 
+  /* Checks the given DIE hasn't been imported yet to prevent cycles.  */
+  inline bool imports_contains (Dwarf_Die *die)
+  {
+    for (struct Dwarf_Die_Chain *import = imports; import != NULL;
+	 import = import->parent)
+      if (import->die.addr == die->addr)
+	return true;
+
+    return false;
+  }
+
   inline int walk_children ()
   {
     do
@@ -103,7 +115,17 @@ __libdw_visit_scopes (depth, root, previsit, postvisit, arg)
 	    if (INTUSE(dwarf_formref_die) (attr, &child.die) != NULL
                 && INTUSE(dwarf_child) (&child.die, &child.die) == 0)
 	      {
+		if (imports_contains (&orig_child_die))
+		  {
+		    __libdw_seterrno (DWARF_E_INVALID_DWARF);
+		    return -1;
+		  }
+		struct Dwarf_Die_Chain *orig_imports = imports;
+		struct Dwarf_Die_Chain import = { .die = orig_child_die,
+						  .parent = orig_imports };
+		imports = &import;
 		int result = walk_children ();
+		imports = orig_imports;
 		if (result != DWARF_CB_OK)
 		  return result;
 	      }
-- 
1.8.3.1


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