Re: [PATCH 8/9] Use htab_up in type copying

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom Tromey <tom@tromey.com> [2020-07-18 11:29:14 -0600]:

> This changes create_copied_types_hash to return an htab_up, then
> modifies the callers to avoid explicit use of htab_delete.
> 
> gdb/ChangeLog
> 2020-07-18  Tom Tromey  <tom@tromey.com>
> 
> 	* value.c (preserve_values): Update.
> 	* python/py-type.c (save_objfile_types): Update.
> 	* guile/scm-type.c (save_objfile_types): Update.
> 	* gdbtypes.h (create_copied_types_hash): Return htab_up.
> 	* gdbtypes.c (create_copied_types_hash): Return htab_up.
> 	* compile/compile-object-run.c (compile_object_run): Update.

This introduced a use after free error, which is addressed by the
patch below.

Thanks,
Andrew

---


From 1c878706d8752b1f1ff61a8a5270675f2b4d828a Mon Sep 17 00:00:00 2001
From: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Fri, 18 Sep 2020 18:23:06 +0100
Subject: [PATCH] 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.

gdb/ChangeLog:

	* compile/compile-object-run.c (create_copied_type_recursive): New
	function.
	(compile_object_run): Use new function.
---
 gdb/ChangeLog                    |  6 ++++++
 gdb/compile/compile-object-run.c | 17 ++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

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),
-- 
2.25.4