From bc20e562ec0436b6117b989c0e3d8f66c9d4d979 Mon Sep 17 00:00:00 2001 From: Lancelot SIX Date: Fri, 8 Jul 2022 11:37:18 +0100 Subject: gdb/varobj: Fix use after free in varobj Varobj object contains references to types, variables (i.e. struct variable) and expression. All of those can reference data on an objfile's obstack. It is possible for this objfile to be deleted (and the obstack to be feed), while the varobj remains valid. Later, if the user uses the varobj, this will result in a use-after-free error. With address sanitizer build, this leads to a plain error. For non address sanitizer build we might see undefined behaviour, which manifest themself as assertion failures when accessing data backed by feed memory. This can be observed if we create a varobj that refers to ta symbol in a shared library, after either the objfile gets reloaded (using the `file` command) or after the shared library is unloaded (with a call to dlclose for example). This patch fixes those issues by: - Adding cleanup procedure to the free_objfile observable. When activated this observer clears expressions referencing the objfile being freed, and removes references to blocks belonging to this objfile. - Adding varobj support in the `preserve_values` (gdb.value.c). This ensures that before the objfile is unloaded, any type owned by the objfile referenced by the varobj is replaced by an equivalent type not owned by the objfile. This process is done here instead of in the free_objfile observer in order to reuse the type hash table already used for similar purpose when replacing types of values kept in the value history. This patch also makes sure to keep a reference to the expression's gdbarch and language_defn members when the varobj->root->exp is initialized. Those structures outlive the objfile, so this is safe. This is done because those references might be used initialize a python context even after exp is invalidated. Another approach could have been to initialize the python context with default gdbarch and language_defn (i.e. nullptr) if expr is NULL, but since we might still try to display the value which was obtained by evaluating exp when it was still valid, keeping track of the context which was used at this time seems reasonable. Tested on x86_64-Linux. Co-Authored-By: Pedro Alves --- gdb/varobj.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) (limited to 'gdb/varobj.c') diff --git a/gdb/varobj.c b/gdb/varobj.c index 9a6d579..2b6dfff 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -32,6 +32,7 @@ #include "parser-defs.h" #include "gdbarch.h" #include +#include "observable.h" #if HAVE_PYTHON #include "python/python.h" @@ -72,6 +73,12 @@ struct varobj_root /* The expression for this parent. */ expression_up exp; + /* Cached arch from exp, for use in case exp gets invalidated. */ + struct gdbarch *gdbarch = nullptr; + + /* Cached language from exp, for use in case exp gets invalidated. */ + const struct language_defn *language_defn = nullptr; + /* Block for which this expression is valid. */ const struct block *valid_block = NULL; @@ -206,7 +213,7 @@ is_root_p (const struct varobj *var) /* See python-internal.h. */ gdbpy_enter_varobj::gdbpy_enter_varobj (const struct varobj *var) -: gdbpy_enter (var->root->exp->gdbarch, var->root->exp->language_defn) +: gdbpy_enter (var->root->gdbarch, var->root->language_defn) { } @@ -303,6 +310,11 @@ varobj_create (const char *objname, try { var->root->exp = parse_exp_1 (&p, pc, block, 0, &tracker); + + /* Cache gdbarch and language_defn as they might be used even + after var is invalidated and var->root->exp cleared. */ + var->root->gdbarch = var->root->exp->gdbarch; + var->root->language_defn = var->root->exp->language_defn; } catch (const gdb_exception_error &except) @@ -2378,6 +2390,55 @@ varobj_invalidate (void) all_root_varobjs (varobj_invalidate_iter); } +/* Ensure that no varobj keep references to OBJFILE. */ + +static void +varobj_invalidate_if_uses_objfile (struct objfile *objfile) +{ + if (objfile->separate_debug_objfile_backlink != nullptr) + objfile = objfile->separate_debug_objfile_backlink; + + all_root_varobjs ([objfile] (struct varobj *var) + { + if (var->root->valid_block != nullptr) + { + struct objfile *bl_objfile = block_objfile (var->root->valid_block); + if (bl_objfile->separate_debug_objfile_backlink != nullptr) + bl_objfile = bl_objfile->separate_debug_objfile_backlink; + + if (bl_objfile == objfile) + { + /* The varobj is tied to a block which is going away. There is + no way to reconstruct something later, so invalidate the + varobj completly and drop the reference to the block which is + being freed. */ + var->root->is_valid = false; + var->root->valid_block = nullptr; + } + } + + if (var->root->exp != nullptr + && exp_uses_objfile (var->root->exp.get (), objfile)) + { + /* The varobj's current expression references the objfile. For + globals and floating, it is possible that when we try to + re-evaluate the expression later it is still valid with + whatever is in scope at that moment. Just invalidate the + expression for now. */ + var->root->exp.reset (); + + /* It only makes sense to keep a floating varobj around. */ + if (!var->root->floating) + var->root->is_valid = false; + } + + /* var->value->type and var->type might also reference the objfile. + This is taken care of in value.c:preserve_values which deals with + making sure that objfile-owend types are replaced with + gdbarch-owned equivalents. */ + }); +} + /* A hash function for a varobj. */ static hashval_t @@ -2412,4 +2473,7 @@ _initialize_varobj () _("When non-zero, varobj debugging is enabled."), NULL, show_varobjdebug, &setdebuglist, &showdebuglist); + + gdb::observers::free_objfile.attach (varobj_invalidate_if_uses_objfile, + "varobj"); } -- cgit v1.1