From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-114.mimecast.com (us-smtp-delivery-114.mimecast.com [170.10.129.114]) by sourceware.org (Postfix) with ESMTPS id 0CA8F3858D1E for ; Wed, 17 May 2023 18:14:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0CA8F3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=labware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=labware.com Received: from NAM12-BN8-obe.outbound.protection.outlook.com (mail-bn8nam12lp2171.outbound.protection.outlook.com [104.47.55.171]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-563-AtAwuZUGMDGcTozp6I8NZw-1; Wed, 17 May 2023 14:14:36 -0400 X-MC-Unique: AtAwuZUGMDGcTozp6I8NZw-1 Received: from SJ2PR17MB6568.namprd17.prod.outlook.com (2603:10b6:a03:4f7::10) by SJ0PR17MB4432.namprd17.prod.outlook.com (2603:10b6:a03:296::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6340.27; Wed, 17 May 2023 18:14:33 +0000 Received: from SJ2PR17MB6568.namprd17.prod.outlook.com ([fe80::fa1f:c830:6d52:12ab]) by SJ2PR17MB6568.namprd17.prod.outlook.com ([fe80::fa1f:c830:6d52:12ab%6]) with mapi id 15.20.6387.030; Wed, 17 May 2023 18:14:33 +0000 From: Jan Vrany To: gdb-patches@sourceware.org CC: tom@tromey.com, Wenyan.Xin@windriver.com, Jan Vrany Subject: [PATCH v2] gdb: fix post-hook execution for remote targets Date: Wed, 17 May 2023 19:14:24 +0100 Message-ID: <20230517181424.263749-1-jan.vrany@labware.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <87pm785iob.fsf@tromey.com> References: <87pm785iob.fsf@tromey.com> X-ClientProxiedBy: LNXP265CA0030.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:5c::18) To SJ2PR17MB6568.namprd17.prod.outlook.com (2603:10b6:a03:4f7::10) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: SJ2PR17MB6568:EE_|SJ0PR17MB4432:EE_ X-MS-Office365-Filtering-Correlation-Id: ff832970-616c-4218-1b2b-08db57029068 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0 X-Microsoft-Antispam-Message-Info: ZWXtPJaHkQFb1QEiDtqmflLJstrerYNNqujaxcGeOQ8IUz6d86LFFPVIWCrmqb/f1eD0dKmT6l7yFfvzMMXuq2fFLxllhsMc2PWtqos+yZ99auZ5DlGWgxiFvoaGVcKKPbGuptEx4CP1HnxN26JOp2uq/u7aeunvWYGTPgaT8B5mkUDdDrdaljCKkc7yhIOq+FEz20vY90iUshfwZ6cPDpBaedxB6f9mLe1/JCFmckeECc59cMXaGE+DtrHVph9nAFVJbwFxCNWNpfOWLSLVBkccwfYLG/kMSGLK7yqCMDF53rxyE66jn6p4/XohpD/bXrzY7cBPYJsUVT/2ZFgcs+bJlLk9L0LdMQcgju9R0Y2zd4OMEjlHKIqH9s2jSmkH1GPHf+ozdv7EakEh6hH64GpmF/OC89aotwU9Udvs1rn9FJkzUDbiJHMQGva+TEve+ZPsH81swom/kqpMOqyfmWYEaTqUWM2Mai0ljcwUdxcAI9xfQDjd5rcXVQOHj0PnIHlbEv/3fo3Z3AkolFJL70F2asKByKWHnoLSusu+770= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SJ2PR17MB6568.namprd17.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(39850400004)(396003)(346002)(376002)(136003)(366004)(451199021)(44832011)(5660300002)(2906002)(316002)(8676002)(8936002)(41300700001)(4326008)(6916009)(66556008)(66476007)(66946007)(966005)(84970400001)(478600001)(6666004)(6486002)(6512007)(1076003)(26005)(107886003)(6506007)(186003)(2616005)(83380400001)(38100700002)(36756003)(86362001);DIR:OUT;SFP:1101 X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?2xr+mufcmTzL45iTV8L00DA31kyNEAWLblyWIunnx+QF1KXKndNEPC+aU6LH?= =?us-ascii?Q?4khv7Xa2BpV2WaR2u7m9LRBGUAllLgqAxPmWOJ183WQ86tes6OhArRzJ+JWU?= =?us-ascii?Q?eDaHiSWIUaSoDclQZyL5gx4PmbpVFxOlTOrRqLpvvvn/F/E9Zyt9NnZ/FeIl?= =?us-ascii?Q?P8BmBCVYB5YVFdZ+aPfxeWp1SY8zOi8/Sqgoacgzu+efQBcnWtOJfT3LDPn9?= =?us-ascii?Q?nK1350rDSaWSgYt+c5dWx0zMNgrwASTk9yk/cgjjJhGndW3Nptt4J5hRc4BD?= =?us-ascii?Q?Gth+qTyzXsl+aUXPizD00K12Cf9HT5L4G8bNDg15uxVK/2ky7Z79L2d7o/rH?= =?us-ascii?Q?7XR2RNIQuguCQE0nurKO+6v1Z6iCOV2v33lX4/URokVOk3p3LR4DjyfX3yMV?= =?us-ascii?Q?P2j6uWK4DeFg2DTvoHrNnR+6UwFy9W8NSAG1JBpIuBi8FxmJfna/kJj6uQAq?= =?us-ascii?Q?jalqXVWbutE501VCgWFCCCsp3v588la2eGs3+ww3SxvS3DhMA+5CXvEdAEec?= =?us-ascii?Q?cdQ4OIeaS9HCMZ239FqcM/FJnUVeQ1nDotdglOwG0mH8P+O8Gdhf5XsYF8GV?= =?us-ascii?Q?hRBf/WpKuabtL6TupCKgh2q/l6RRNBFA8HpZZV1dELDRl01PS2v24uWn41QX?= =?us-ascii?Q?FotCu8q6L+S9sFq2+TRrExsO9PoTgFvQlft4lK1X22AQulObzLVAOjYmZL+h?= =?us-ascii?Q?OEXouit8AZnIlrPQjrUa1ikAe8ak67XG9016EmjRS/iZ1zghAlKwLF9nt3K2?= =?us-ascii?Q?E0Do9JaIMaf/fwekpj47htpzENWBuFfMjtlaB3uo/ymj3mdngHoiQaRPbQSw?= =?us-ascii?Q?n6aMwyelGoHAh1KjQVjhirG4o9B72zde6o0YMtq3T3HCYYdM9rVCtJrwPgO9?= =?us-ascii?Q?XiN3EJGcPeH0qu/URF1sD4VfgL0/zaqJgdNx83El/fMMKyS68BlFogU97YwW?= =?us-ascii?Q?ix6McQfI6w+RfyG4zAMqB6E1kFUX2bXf6b6+nI3r8KG/c/1Ft+3+5/Q//QL8?= =?us-ascii?Q?ISPuAqjApcAp6xWRbUPeIUOBUm8aPixnEKXtOpwqgipbct1GFFEQuhRsIhD+?= =?us-ascii?Q?yWZHSOy9MdJiltIReXkm5PmUZMNXjffgj1PegSM9WgByb5SDhiMN0JhqkX0c?= =?us-ascii?Q?cA5hGYetnw62l6RwcElRExXLG3jW+issOzgLXRHm8NfR3LRTPxb5M7tHjlUF?= =?us-ascii?Q?M4kRb5Xi1omLggSOxTgW1UiX4aBHkQKR4gcaa0unU5+xjWF55KEeQ/h0b0vt?= =?us-ascii?Q?Xn5sbt16xE1QrppXPP+HtD02/TO9S9ZNnw+XR+X+BW+l8oYGChDseSac0Mm7?= =?us-ascii?Q?pLczCsf6E3pBHviacbJZsI94Hn8dqd73bgzbYoJAvPKlwIhFhHgGuy3V4V0t?= =?us-ascii?Q?5LdpgaGT5AJiEIPAeZp/xfuBk8OzolFtGsJmvjAgdwDfbmlcZe7+9TuBv1G6?= =?us-ascii?Q?20lNTcZU2S5Ajic+5vc9mhgtXYJ4sAPjRCUozSotupKAvDiOON7zqcA4/9yn?= =?us-ascii?Q?9JV9IF8hOBh2uzp/PrT0ZI3hufH4Clc40iOgbnia2n7Xaz8/dqAZM8YHJ7ZY?= =?us-ascii?Q?gkLPlM+1PxaeiOl5K9zAIW4QNpqwR6ql+KlZV0AJ?= X-OriginatorOrg: labware.com X-MS-Exchange-CrossTenant-Network-Message-Id: ff832970-616c-4218-1b2b-08db57029068 X-MS-Exchange-CrossTenant-AuthSource: SJ2PR17MB6568.namprd17.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 May 2023 18:14:33.0237 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: b5db0322-1aa0-4c0a-859c-ad0f96966f4c X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 2jNakwahwqMkKYnoh3KS6lFK9XNJBDPt4/sndb/YFFKvARCbJekc56Yhs6TwDk/cwbPICUvg39U9Inbf/EWshw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR17MB4432 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: labware.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=WINDOWS-1252 X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,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 List-Id: > I wonder if it would be possible to reimplement dont_repeat so that it > does not overwrite the command. Like, could it just set a flag? Yes, that'd make things clear. I tried couple things but each time something broke. The issue seem to be code like (top.c, execute_command()):= id if (repeat_arguments !=3D NULL && cmd_start =3D=3D saved_command_line= ) =09{ =09 gdb_assert (strlen (args_pointer) >=3D strlen (repeat_arguments)); =09 strcpy (saved_command_line + (args_pointer - cmd_start), =09=09 repeat_arguments); =09} Here we compare the value of saved_command_line so we cannot easily set the flag and then when reading input and flag is set, return empty stri= ng instead of saved_command_line. Truth to be told, I don't understand how it = works and what exactly (for example) the code above does or why it is needed.=20 >=20 > Jan> @@ -575,6 +575,7 @@ execute_command (const char *p, int from_tty) > Jan> struct cmd_list_element *c; > Jan> const char *line; > Jan> const char *cmd_start =3D p; > Jan> + std::string cmd_copy =3D p; >=20 > A bit after this, there's a null check for p: >=20 > /* This can happen when command_line_input hits end of file. */ > if (p =3D=3D NULL) > { > cleanup_if_error.release (); > return; > } >=20 > If that can really happen, then cmd_copy has to be created afterward the > check. Done that, see below.=20 >=20 > Is it possible to write a test case for this? Even one that only fails > with ASAN? >=20 I checked that with my version of GCC, std::string s =3D nullptr; throws an error and abort()s but I could not find a situation when p =3D=3D NULL. The testsuite passes on my machine (even with ASAN). I also tried to open two command line interpreters (using new-io console ...) and then close the PTY but still, I did not hit the case of p being NULL. Jan -- >8 -- Commit b5661ff2 ("gdb: fix possible use-after-free when executing commands") attempted to fix possible use-after-free in case command redefines itself. Commit 37e5833d ("gdb: fix command lookup in execute_command ()") updated the previous fix to handle subcommands as well by using the original command string to lookup the command again after its execution. This fixed the test in gdb.base/define.exp but it turned out that it does not work (at least) for "target remote" and "target extended-remote". The problem is that the command buffer P passed to execute_command () gets overwritten in dont_repeat () while executing "target remote" command itself: =09#0 dont_repeat () at top.c:822 =09#1 0x000055555730982a in target_preopen (from_tty=3D1) at target.c:2483 =09#2 0x000055555711e911 in remote_target::open_1 (name=3D0x55555881c7fe "= :1234", from_tty=3D1, extended_p=3D0) =09 at remote.c:5946 =09#3 0x000055555711d577 in remote_target::open (name=3D0x55555881c7fe ":1= 234", from_tty=3D1) at remote.c:5272 =09#4 0x00005555573062f2 in open_target (args=3D0x55555881c7fe ":1234", fr= om_tty=3D1, command=3D0x5555589d0490) =09 at target.c:853 =09#5 0x0000555556ad22fa in cmd_func (cmd=3D0x5555589d0490, args=3D0x55555= 881c7fe ":1234", from_tty=3D1) =09 at cli/cli-decode.c:2737 =09#6 0x00005555573487fd in execute_command (p=3D0x55555881c802 "4", from_= tty=3D1) at top.c:688 Therefore the second call to lookup_cmd () at line 697 fails to find command because the original command string is gone. This commit addresses this particular problem by creating a *copy* of original command string for the sole purpose of using it after command execution to lookup the command again. It may not be the most efficient way but it's safer given that command buffer is shared and overwritten in hard-to-foresee situations. Tested on x86_64-linux. PR 30249 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D30249 --- gdb/top.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gdb/top.c b/gdb/top.c index 0b819091d11..92de30a1472 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -471,6 +471,8 @@ execute_command (const char *p, int from_tty) return; } =20 + std::string cmd_copy =3D p; + target_log_command (p); =20 while (*p =3D=3D ' ' || *p =3D=3D '\t') @@ -577,7 +579,7 @@ execute_command (const char *p, int from_tty) =09 We need to lookup the command again since during its execution, =09 a command may redefine itself. In this case, C pointer =09 becomes invalid so we need to look it up again. */ - const char *cmd2 =3D cmd_start; + const char *cmd2 =3D cmd_copy.c_str (); c =3D lookup_cmd (&cmd2, cmdlist, "", nullptr, 1, 1); if (c !=3D nullptr) =09execute_cmd_post_hook (c); --=20 2.39.2