From: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Fri, 18 Sep 2020 17:23:06 +0000 (+0100)
Subject: gdb: Fix use after free bug in compile_object_run
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=0295dde6551b898295bd6a2ef7dab425643c4c1e;p=binutils-gdb.git

gdb: Fix use after free bug in compile_object_run

In this commit:

  commit 6108fd1823f9cf036bbbe528ffcdf2fee489b40a
  Date:   Thu Sep 17 11:47:50 2020 -0600

      Use htab_up in type copying

A use after free bug was introduced.  In compile-object-run.c, in the
function compile_object_run, the code used to look like this:

    htab_t copied_types;

    /* .... snip .... */

    /* OBJFILE may disappear while FUNC_TYPE still will be in use.  */
    copied_types = create_copied_types_hash (objfile);
    func_type = copy_type_recursive (objfile, func_type, copied_types);
    htab_delete (copied_types);

    /* .... snip .... */

    call_function_by_hand_dummy (func_val, NULL, args,
                                 do_module_cleanup, data);

The copied_types table exists on the obstack of objfile, but is
deleted once the call to copy_type_recursive has been completed.

After the change the code now looks like this:

    /* OBJFILE may disappear while FUNC_TYPE still will be in use.  */
    htab_up copied_types = create_copied_types_hash (objfile);
    func_type = copy_type_recursive (objfile, func_type, copied_types.get ());

    /* .... snip .... */

    call_function_by_hand_dummy (func_val, NULL, args,
                                 do_module_cleanup, data);

The copied_types is now a unique_ptr and deleted automatically when it
goes out of scope.

The problem however is that objfile, and its included obstack, may be
deleted by the call to do_module_cleanup, which is called by
call_function_by_hand_dummy.

This means that in the new code the objfile, and its obstack, are
deleted before copied_types is deleted, and as copied_types is on the
objfiles obstack, we are now reading undefined memory.

The solution in this commit is to wrap the call to
create_copied_types_hash and copy_type_recursive into a new static
helper function.  The htab_up will then be deleted within the new
function's scope, before objfile is deleted.

This resolves some non-deterministic test failures I was seeing in
gdb.compile/*.exp tests.

gdb/ChangeLog:

	* compile/compile-object-run.c (create_copied_type_recursive): New
	function.
	(compile_object_run): Use new function.
---

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3614d8ed8ef..fc3d6e387ae 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2020-09-18  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* compile/compile-object-run.c (create_copied_type_recursive): New
+	function.
+	(compile_object_run): Use new function.
+
 2020-08-21  Jon Turney  <jon.turney@dronecode.org.uk>
 
 	* NEWS: Mention x86_64 Cygwin core file support.
diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c
index 533478a0fb4..985c6f363a3 100644
--- a/gdb/compile/compile-object-run.c
+++ b/gdb/compile/compile-object-run.c
@@ -105,6 +105,16 @@ do_module_cleanup (void *arg, int registers_valid)
   xfree (data);
 }
 
+/* Create a copy of FUNC_TYPE that is independent of OBJFILE.  */
+
+static type *
+create_copied_type_recursive (objfile *objfile, type *func_type)
+{
+  htab_up copied_types = create_copied_types_hash (objfile);
+  func_type = copy_type_recursive (objfile, func_type, copied_types.get ());
+  return func_type;
+}
+
 /* Perform inferior call of MODULE.  This function may throw an error.
    This function may leave files referenced by MODULE on disk until
    the inferior call dummy frame is discarded.  This function may throw errors.
@@ -143,9 +153,10 @@ compile_object_run (struct compile_module *module)
       int current_arg = 0;
       struct value **vargs;
 
-      /* OBJFILE may disappear while FUNC_TYPE still will be in use.  */
-      htab_up copied_types = create_copied_types_hash (objfile);
-      func_type = copy_type_recursive (objfile, func_type, copied_types.get ());
+      /* OBJFILE may disappear while FUNC_TYPE is still in use as a
+	 result of the call to DO_MODULE_CLEANUP below, so we need a copy
+	 that does not depend on the objfile in anyway.  */
+      func_type = create_copied_type_recursive (objfile, func_type);
 
       gdb_assert (func_type->code () == TYPE_CODE_FUNC);
       func_val = value_from_pointer (lookup_pointer_type (func_type),