gdb: Fix use after free bug in compile_object_run
authorAndrew Burgess <andrew.burgess@embecosm.com>
Fri, 18 Sep 2020 17:23:06 +0000 (18:23 +0100)
committerAndrew Burgess <andrew.burgess@embecosm.com>
Fri, 18 Sep 2020 18:18:53 +0000 (19:18 +0100)
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.

gdb/ChangeLog
gdb/compile/compile-object-run.c

index 3614d8ed8ef2491a5cf20b70cd5b3fdce41de973..fc3d6e387ae8c787013521813285d4589de6d046 100644 (file)
@@ -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.
index 533478a0fb46c8b25646a7a06415bee0668f547f..985c6f363a3ae4cf4143503486084a29ae518c28 100644 (file)
@@ -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),