From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by sourceware.org (Postfix) with ESMTPS id 91ED2384D193 for ; Wed, 29 Jun 2022 16:22:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 91ED2384D193 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f44.google.com with SMTP id v193-20020a1cacca000000b003a051f41541so3532318wme.5 for ; Wed, 29 Jun 2022 09:22:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=neG3KnTKWFIgNTfqpmePc6n4kKSKrL9S+mFl8GitwD8=; b=IMRgfOKD5JK+lesFQNmMLq/YkpDaRsvqyMpYmZ0Oaf2kmVQ+ITdkSfG8R+Rk5ePzaq BowYUNpJuGAH9zSHgtWiz/xR1LtcZ/dTD50CWLVx29436axKLrnLGtPTV8ANl9EHU2fH Sg5JvTir47U1X1HjFhYVJ7QlaiEWJLvDJ8G9liEtWEZaVOlC1FyOOl72/QLU0yysl2Yy 9f89cEsUsfJTSVORRhZkuEECaX9pzmK89jzX1LscOKB8puYatA/5jr8zTfFmSabzhR+x 5hltrWjIZWEWPnsjTzyV1UqzroO24mfeuflEZ0IYH1yYTD4I50FXiJ5TzCyghOT3vmbV GZpQ== X-Gm-Message-State: AJIora/UD3LPA143nU7tsZWFDXN8yLg4fgWUKBKiT0DRybwtBZa+CRpl bh/Igal8FTOtzeDCm/hjk4t+B76EdXA= X-Google-Smtp-Source: AGRyM1t87lBObtHPOwXCYjXIRH5SOZPXygSp3HUocmq1SWYdNn/UTAvpTvePBWBHF+tHencLCF2ekg== X-Received: by 2002:a05:600c:19c8:b0:3a1:792e:f913 with SMTP id u8-20020a05600c19c800b003a1792ef913mr739057wmq.182.1656519768256; Wed, 29 Jun 2022 09:22:48 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id y15-20020a5d4acf000000b0021b9c520953sm17126153wrs.64.2022.06.29.09.22.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Jun 2022 09:22:47 -0700 (PDT) Message-ID: Date: Wed, 29 Jun 2022 17:22:46 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: [PATCH] Fix GDBserver regression due to change to avoid reading shell registers Content-Language: en-US To: Simon Marchi , Andrew Burgess , gdb-patches@sourceware.org References: <83ceec06f74f4ce6a2dc8d794031fb233567ba6d.1655136816.git.aburgess@redhat.com> <48d211cc-96f3-8c84-5aec-7023054d45ac@palves.net> <87fsjqdqco.fsf@redhat.com> <6db0b4b8-25fb-8f4c-382e-d45f6edcff17@palves.net> <87a69xdqfo.fsf@redhat.com> <87ea7153-6f53-06f9-3aa7-4ce90031b3e8@simark.ca> From: Pedro Alves In-Reply-To: <87ea7153-6f53-06f9-3aa7-4ce90031b3e8@simark.ca> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, WEIRD_PORT 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: Wed, 29 Jun 2022 16:22:52 -0000 On 2022-06-29 16:17, Simon Marchi wrote: > This introduces the following ASan crash for me: ... > It looks like in that non-existing-program case, the process gets deleted before > the starting_up flag gets restored to false. Thanks. The patch below fixes it. I've also run the whole testsuite with --target_board=native-extended-gdbserver, on x86-64 Ubuntu 20.04, with Asan, and saw no regressions. >From 5ceb44cdf51b764193bea229c80e55ebb0d9c55c Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 29 Jun 2022 16:38:43 +0100 Subject: [PATCH] Fix GDBserver regression due to change to avoid reading shell registers Simon reported that the recent change to make GDB and GDBserver avoid reading shell registers caused a GDBserver regression, caught with ASan while running gdb.server/non-existing-program.exp: $ /home/smarchi/build/binutils-gdb/gdb/testsuite/../../gdb/../gdbserver/gdbserver stdio non-existing-program ================================================================= ==127719==ERROR: AddressSanitizer: heap-use-after-free on address 0x60f0000000e9 at pc 0x55bcbfa301f4 bp 0x7ffd238a7320 sp 0x7ffd238a7310 WRITE of size 1 at 0x60f0000000e9 thread T0 #0 0x55bcbfa301f3 in scoped_restore_tmpl::~scoped_restore_tmpl() /home/smarchi/src/binutils-gdb/gdbserver/../gdbsupport/scoped_restore.h:86 #1 0x55bcbfa2ffe9 in post_fork_inferior(int, char const*) /home/smarchi/src/binutils-gdb/gdbserver/fork-child.cc:120 #2 0x55bcbf9c9199 in linux_process_target::create_inferior(char const*, std::__debug::vector > const&) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:991 #3 0x55bcbf954549 in captured_main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3941 #4 0x55bcbf9552f0 in main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4084 #5 0x7ff9d663b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2) #6 0x55bcbf8ef2bd in _start (/home/smarchi/build/binutils-gdb/gdbserver/gdbserver+0x1352bd) 0x60f0000000e9 is located 169 bytes inside of 176-byte region [0x60f000000040,0x60f0000000f0) freed by thread T0 here: #0 0x7ff9d6c6f0c7 in operator delete(void*) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:160 #1 0x55bcbf910d00 in remove_process(process_info*) /home/smarchi/src/binutils-gdb/gdbserver/inferiors.cc:164 #2 0x55bcbf9c4ac7 in linux_process_target::remove_linux_process(process_info*) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:454 #3 0x55bcbf9cdaa6 in linux_process_target::mourn(process_info*) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:1599 #4 0x55bcbf988dc4 in target_mourn_inferior(ptid_t) /home/smarchi/src/binutils-gdb/gdbserver/target.cc:205 #5 0x55bcbfa32020 in startup_inferior(process_stratum_target*, int, int, target_waitstatus*, ptid_t*) /home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/fork-inferior.c:515 #6 0x55bcbfa2fdeb in post_fork_inferior(int, char const*) /home/smarchi/src/binutils-gdb/gdbserver/fork-child.cc:111 #7 0x55bcbf9c9199 in linux_process_target::create_inferior(char const*, std::__debug::vector > const&) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:991 #8 0x55bcbf954549 in captured_main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3941 #9 0x55bcbf9552f0 in main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4084 #10 0x7ff9d663b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2) previously allocated by thread T0 here: #0 0x7ff9d6c6e5a7 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99 #1 0x55bcbf910ad0 in add_process(int, int) /home/smarchi/src/binutils-gdb/gdbserver/inferiors.cc:144 #2 0x55bcbf9c477d in linux_process_target::add_linux_process_no_mem_file(int, int) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:425 #3 0x55bcbf9c8f4c in linux_process_target::create_inferior(char const*, std::__debug::vector > const&) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:985 #4 0x55bcbf954549 in captured_main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3941 #5 0x55bcbf9552f0 in main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4084 #6 0x7ff9d663b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2) Above we see that in the non-existing-program case, the process gets deleted before the starting_up flag gets restored to false. This happens because startup_inferior call target_mourn_inferior before throwing an error, and in GDBserver, unlike in GDB, mourning deletes the process. Fix this by not using a scoped_restore to manage the starting_up flag, since we should only clear it when startup_inferior doesn't throw. Change-Id: I67325d6f81c64de4e89e20e4ec4556f57eac7f6c --- gdbserver/fork-child.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/gdbserver/fork-child.cc b/gdbserver/fork-child.cc index 7ea66f2a435..078612f7091 100644 --- a/gdbserver/fork-child.cc +++ b/gdbserver/fork-child.cc @@ -105,12 +105,21 @@ post_fork_inferior (int pid, const char *program) #endif process_info *proc = find_process_pid (pid); - scoped_restore save_starting_up - = make_scoped_restore (&proc->starting_up, true); + + /* If the inferior fails to start, startup_inferior mourns the + process (which deletes it), and then throws an error. This means + that on exception return, we don't need or want to clear this + flag back, as PROC won't exist anymore. Thus, we don't use a + scoped_restore. */ + proc->starting_up = true; startup_inferior (the_target, pid, START_INFERIOR_TRAPS_EXPECTED, &cs.last_status, &cs.last_ptid); + + /* If we get here, the process was successfully started. */ + proc->starting_up = false; + current_thread->last_resume_kind = resume_stop; current_thread->last_status = cs.last_status; signal_pid = pid; base-commit: b955c336f93a1b8ab433b713739c145c56f5c027 -- 2.36.0