From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM04-BN8-obe.outbound.protection.outlook.com (mail-bn8nam04on2068.outbound.protection.outlook.com [40.107.100.68]) by sourceware.org (Postfix) with ESMTPS id A004A3857B8C for ; Fri, 17 Jun 2022 10:10:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A004A3857B8C ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eIAJ/gHwCvlNZ72Sv8Var339yTMzeaLIsfgXrwJDFq/PDoz/lH7uC9lE7bKhfM1Qy/J7pTnEUBfQS3oWmf4dMhVy+pLXGmgR8PkgIhhCEGQ51evboTGWUey0Arrz1TMZJPKbjwjOiveG/Ha0edWc+9ZtWt54xU0zm5AbMNS/Zu0woX1EN/G6QMPqM/aZZntLsjSY5YORTNYVLV7M5r9CpPg6sklgphrcbauN37zdha+BdGeDtlVK/ZiYYUevFvyU5ItSyKnnAAegZ1K2ZBA0LuC35x6tmhuF/YKU6bHbaPg3W7oMsiSkGQCBhvMhYthQohSzhBCsRFayhf/6+Yw6Iw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=AK0wCbttN72h1ELmJMEg53kJpEQD+3eJ2ElJG5ZS3iE=; b=H0TEB2pijmHn2kfCgONTISORXOimLYOt1lvNv5dTNevkugAgOT7Nzp+wCfwHXbt5QDMuycDzotGYcWh5y3+Nk7y9T5w6XHnBTmX4mbnGVKeCuJ6apr8JLBWIOf0UrKxFJ9rzYQqu76wCHm1FgyYHypV4JXbl6thOJ6AdQJSe5IvcVaYlMPukmS5+PDjcRF2WS+40pGduqWQVAkmAJZI30+Vp9xpvmQ0YuZYHtvG/WY4iEqbhO87FE3SBpeF9h7zR5kE6DO+hi188H+eO3gxrzcljG7OFVRNGr26jR3ZYl+V7lHSM2eZEDFGDYQRXYEEvgIuS/sIcHm/63SKbjc18Fg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=sourceware.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none Received: from MWHPR15CA0070.namprd15.prod.outlook.com (2603:10b6:301:4c::32) by DM6PR12MB3883.namprd12.prod.outlook.com (2603:10b6:5:1c9::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5353.13; Fri, 17 Jun 2022 10:10:53 +0000 Received: from CO1NAM11FT005.eop-nam11.prod.protection.outlook.com (2603:10b6:301:4c:cafe::c3) by MWHPR15CA0070.outlook.office365.com (2603:10b6:301:4c::32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5353.17 via Frontend Transport; Fri, 17 Jun 2022 10:10:52 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by CO1NAM11FT005.mail.protection.outlook.com (10.13.174.147) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.5353.14 via Frontend Transport; Fri, 17 Jun 2022 10:10:52 +0000 Received: from khazad-dum.amd.com (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.28; Fri, 17 Jun 2022 05:10:47 -0500 From: Lancelot SIX To: CC: , Lancelot SIX Subject: [PATCH 2/3] gdb/varobj: Fix use after free in varobj Date: Fri, 17 Jun 2022 11:10:23 +0100 Message-ID: <20220617101024.2830260-3-lancelot.six@amd.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220617101024.2830260-1-lancelot.six@amd.com> References: <20220617101024.2830260-1-lancelot.six@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB03.amd.com (10.181.40.144) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 39c68bb9-8f8a-4e53-0ba5-08da5049a901 X-MS-TrafficTypeDiagnostic: DM6PR12MB3883:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: fWYMdxnflGnQ3D8CBE4J3VQZhPcH0Aodm410e2Jxb5WoZQH1KUKMiOwu5U4+P0wMsHVBvc3lJKDOyYRoRTHS74sNRGTSusW7Nk5MMm9OLpZfv9lSHMgEFmXW3Xop5yXVEEmjSKUCLWLNHA8wArRfJxB93x0hWnuzIhH8GLlWaXa8D2g3nkOKQ+r1JfGCwKX+Y1dMPmSYAJ/jwsBUxwnQ9CZmlghb5EDWZDa6iP5pY5Wz2XO7oD6wEDMZ3PwYCzxyxapWhI3vvH9TJXSDCfmeGOjTvotXUpKV4dFwIHE6UxzU4PXdRXaLRxA3EvVlAw2jbIaMyDCmKNSbTU/qJH+LvNDJTTTUpzDdwtnwAxCb3hMLOyFl61sNHpM9FrWHzcsKPZgsSfRybtmBsk86iCIySJpQrAItmLVKBff/ncKv/HsexKvmo6bWKFnPRWVv5TjZK4ya/Efm+szRjxQKHudEyfVoP6vS6DAGrmS6Td80/TsJrfPuwiCoOURbigeTAoUfBrpN9/DyPqA2kKqfUCXE4mpTjDMpEEM9wFOm1QGaVmKKj+hiM2JcC2UFfABcw+c5OaeKwlZPhYCIynGBeXJnS6HcB1KDr6fsTNnE9TYNvHaob0LiXHWcdO5Lmp+nvmEuCw6MpUhKoB/jJX9A07iuIzoDkpYpjdLuhmF/cCrCA6klz8kHQ7napcSHIia5KpSRWvxXfHJedwswraK3wzdcZTnmjq7+DRyBMfu80jGhbUjraG4cgTTuJAQQMfsAbR0geo1YIieU9y3Z7VmE/sCoL5L4UlRvYxrKp9azlt8g3uMXwf4uSZDMWwboZf6LQMVCEZcUe9YuQs/BRJn4OePuIA== X-Forefront-Antispam-Report: CIP:165.204.84.17; CTRY:US; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:SATLEXMB04.amd.com; PTR:InfoDomainNonexistent; CAT:NONE; SFS:(13230016)(4636009)(46966006)(40470700004)(36840700001)(47076005)(4326008)(8936002)(86362001)(7696005)(8676002)(426003)(336012)(70206006)(498600001)(5660300002)(70586007)(30864003)(83380400001)(54906003)(81166007)(36860700001)(16526019)(36756003)(316002)(6916009)(6666004)(186003)(356005)(2616005)(1076003)(40460700003)(82310400005)(2906002)(26005)(403724002)(2004002)(36900700001); DIR:OUT; SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jun 2022 10:10:52.2880 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 39c68bb9-8f8a-4e53-0ba5-08da5049a901 X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d; Ip=[165.204.84.17]; Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: CO1NAM11FT005.eop-nam11.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB3883 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Jun 2022 10:10:58 -0000 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. A consequence of those changes is that the varobj->root->exp field of a varobj can now be NULL. The rest of the changes ensure that the varobj machinery is able to support such situation when re-evaluating the var (under varobj_update). When the varobj->root->exp is initialized, this patch also makes sure to keep a reference to its gdbarch and language_defn members. 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. --- .../gdb.mi/mi-var-invalidate-shlib-lib.c | 30 +++++++ .../gdb.mi/mi-var-invalidate-shlib.c | 27 +++++++ .../gdb.mi/mi-var-invalidate-shlib.exp | 80 +++++++++++++++++++ gdb/testsuite/lib/mi-support.exp | 2 +- gdb/value.c | 21 +++++ gdb/varobj.c | 66 ++++++++++++++- 6 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-var-invalidate-shlib-lib.c create mode 100644 gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.c create mode 100644 gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib-lib.c b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib-lib.c new file mode 100644 index 00000000000..0abbdbdf9c6 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib-lib.c @@ -0,0 +1,30 @@ +/* Copyright 2022 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +struct bar +{ + int a; + int b; +}; + +static struct bar global_var = { 2, 2 }; + +void +foo () +{ + struct bar local_var = { 1, 1 }; +} diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.c b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.c new file mode 100644 index 00000000000..10101bb8d22 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.c @@ -0,0 +1,27 @@ +/* Copyright 2022 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +int +main (int argc, char *argv []) +{ + void *shlib = dlopen (SHLIB_PATH, RTLD_LAZY); + void (*foo)() = dlsym (shlib, "foo"); + foo (); + dlclose (shlib); +} diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp new file mode 100644 index 00000000000..5219946d845 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp @@ -0,0 +1,80 @@ +# Copyright 2007-2022 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# Test that varobj are invalidated after the shlib they point to goes +# away. + + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +if { [skip_shlib_tests] } { + return 0 +} + +standard_testfile .c -lib.c +set shlib_path [standard_output_file ${testfile}-lib.so] + +if { [gdb_compile_shlib $srcdir/$subdir/$srcfile2 $shlib_path {debug}] != "" } { + untested "failed to compile" + return -1 +} + +set opts [list shlib_load debug additional_flags=-DSHLIB_PATH="${shlib_path}"] +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $opts] != "" } { + untested "failed to compile" + return -1 +} + +gdb_exit +if { [mi_gdb_start] } { + return 0 +} + +# Start the process once and create varobjs referencing the loaded objfiles. +with_test_prefix "setup" { + mi_load_shlibs $shlib_path + mi_delete_breakpoints + mi_gdb_reinitialize_dir $srcdir/$subdir + mi_gdb_load $binfile + + mi_runto foo -pending + + mi_create_varobj global_var global_var "create global gloal_var" + mi_create_floating_varobj floating_local local_var "create floating local_var" + + # Get ourself in a frame where the floating var is invalid. + mi_gdb_test "-exec-finish" ".*" + mi_expect_stop "function-finished" main ".*" ".*" "\[0-9\]+" { ".*" } "out of foo" +} + +# Reload the entire process +with_test_prefix "restart process" { + mi_delete_breakpoints + mi_gdb_load ${binfile} + mi_runto_main +} + +with_test_prefix "in new process" { + # The global var was invalidated when the objfile got unloaded. + mi_gdb_test "-var-update global_var" \ + "\\^done,changelist=\\\[\{name=\"global_var\",in_scope=\"invalid\",has_more=\"0\"\}\]" \ + "global invalidated" + + # Floating varobj should still be valid, but out of scope at the moment. + mi_gdb_test "-var-update floating_local" \ + "\\^done,changelist=\\\[{name=\"floating_local\",in_scope=\"false\",type_changed=\"false\",has_more=\"0\"}\\\]" \ + "floating_local still valid but not in scope" +} diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index e724b2eeb51..ca56e12b06b 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -1121,7 +1121,7 @@ proc mi_runto_helper {func run_or_continue args} { global mi_gdb_prompt expect_out global hex decimal fullname_syntax - parse_args {{qualified}} + parse_args {{qualified} {pending}} set test "mi runto $func" if {$pending} { diff --git a/gdb/value.c b/gdb/value.c index 022fca91a42..b9d2937c608 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -46,6 +46,7 @@ #include "cli/cli-style.h" #include "expop.h" #include "inferior.h" +#include "varobj.h" /* Definition of a user function. */ struct internal_function @@ -2596,6 +2597,21 @@ preserve_one_internalvar (struct internalvar *var, struct objfile *objfile, } } +static void +preserve_one_varobj (struct varobj *varobj, struct objfile *objfile, + htab_t copied_types) +{ + if (varobj->type->is_objfile_owned () + && varobj->type->objfile_owner () == objfile) + { + varobj->type + = copy_type_recursive (objfile, varobj->type, copied_types); + } + + if (varobj->value != nullptr) + preserve_one_value (varobj->value.get (), objfile, copied_types); +} + /* Update the internal variables and value history when OBJFILE is discarded; we must copy the types out of the objfile. New global types will be created for every convenience variable which currently points to @@ -2617,6 +2633,11 @@ preserve_values (struct objfile *objfile) for (var = internalvars; var; var = var->next) preserve_one_internalvar (var, objfile, copied_types.get ()); + /* For the remaining varobj, check that none has type owned by OBJFILE. */ + all_root_varobjs ([&copied_types, objfile](struct varobj *varobj) + { preserve_one_varobj (varobj, objfile, + copied_types.get ()); }); + preserve_ext_lang_values (objfile, copied_types.get ()); } diff --git a/gdb/varobj.c b/gdb/varobj.c index 1aca015a21a..caaffe4ea70 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) @@ -2071,6 +2083,12 @@ value_of_root (struct varobj **var_handle, bool *type_changed) { struct value *value; + /* The expression have been invalidated (because it did reference an + objfile which is not available anymore) and we did not manage to + recerate it for a floating varobj. */ + if ((*var_handle)->root->exp == nullptr) + return nullptr; + value = value_of_root_1 (var_handle); if (var->value == NULL || value == NULL) { @@ -2373,6 +2391,49 @@ 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) +{ + auto varobj_invalidate_if_uses_objfile_worker + = [objfile](struct varobj *var) + { + if (var->root->valid_block != nullptr + && block_objfile (var->root->valid_block) == 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 this 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 changed with gdbarch-owned + equivalents. */ + }; + + all_root_varobjs (varobj_invalidate_if_uses_objfile_worker); +} + /* A hash function for a varobj. */ static hashval_t @@ -2407,4 +2468,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"); } -- 2.25.1