From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id qNEtB6El919nDAAAWB0awg (envelope-from ) for ; Thu, 07 Jan 2021 10:15:45 -0500 Received: by simark.ca (Postfix, from userid 112) id 0F0901E965; Thu, 7 Jan 2021 10:15:45 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.4 required=5.0 tests=DKIM_SIGNED,MAILING_LIST_MULTI, RDNS_NONE,T_DKIM_INVALID,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 48B201E590 for ; Thu, 7 Jan 2021 10:15:44 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D8167396EC9C; Thu, 7 Jan 2021 15:15:43 +0000 (GMT) Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by sourceware.org (Postfix) with ESMTPS id EC183396EC79 for ; Thu, 7 Jan 2021 15:15:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org EC183396EC79 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x335.google.com with SMTP id a6so5468288wmc.2 for ; Thu, 07 Jan 2021 07:15:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=pvnbSkxHQ6HpSS/CZk7kMTcw7jRL5XLGfCsHDrf6lxo=; b=WxgAZgcZSiMmebPLHMLdA8H5AK+Sz6tS3hHtbojkWfl4wwfjM/wwH6UfIRvfJsMFF3 5cxVM/fVVJNnVFHLkOi2YVEFkraenASTN47B1TP8EY38a9GNuQl4xgWJZmqAGWbTXZUM sJgJCtKARl5yeCJRxoC+sBQdmK6L3fnMrJKCnQigp10QfMv0WrEJLfhdlTArQnl3uCeM xN9UjJs2qnsHbBIExyC87Z90Wjo4I1y2pvAq+qlLcw3wdSwiHqT+SM7o8pRXb/qnaUpb rnA8mDC3hFwM+c18fhHYNxSaK5fT5Yxvx9/mFRNtqL/HQTpspLmaYbbh555a12673Q2G mYlg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=pvnbSkxHQ6HpSS/CZk7kMTcw7jRL5XLGfCsHDrf6lxo=; b=Wf+ow7QZv89Os9VKOIsQkGSz3EsRntMWq2EwU8Ri+6dgo4MUAYuCyusYbEXwC7TnBG phXsa7S5El7zt/7p2wXuDOr5OkUTzWjoLDI575wW6lWVaeBmuckoVriMBwn/zQilToYm H+fhcxd5GS4+CSqjcayYXrbNnB85Zc1y1Zyo382XbYHh9MYw2XUps2SijfkQMlHkHFJo HW5K1osEm/MdWSSZ6YvsXO18Nvsyjqd8OjJKqbInDIh1NwgM/YOwLnApVHXpv1mG2w9I SxjP4b1iQtumvno5drAC/iwdvWhxFnUdF5JeeTGuj9NaIJ0vwe+r+bJvWYjWcVwt3l7a Ypyw== X-Gm-Message-State: AOAM533FmE48WGNi/8Meaa871nDBzzzmxI8i1uLnATdnmJjTJ6ExQzVu wHHVz4kKE+oh2S74YxlMVlKPtA== X-Google-Smtp-Source: ABdhPJwm3D2pCKlFwC5P2QIzACLGvcoG/p+w35PA9n+dP24DVcaqhkvtw9YDnA2jn8YaMIG9/nNapg== X-Received: by 2002:a05:600c:2f17:: with SMTP id r23mr8425948wmn.157.1610032539961; Thu, 07 Jan 2021 07:15:39 -0800 (PST) Received: from localhost (host86-166-129-230.range86-166.btcentralplus.com. [86.166.129.230]) by smtp.gmail.com with ESMTPSA id t10sm8293091wrp.39.2021.01.07.07.15.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Jan 2021 07:15:39 -0800 (PST) Date: Thu, 7 Jan 2021 15:15:38 +0000 From: Andrew Burgess To: Tom de Vries Subject: Re: [PATCH][gdb/remote] Fix invalid pointer in remote_async_serial_handler Message-ID: <20210107151538.GQ2945@embecosm.com> References: <20210107133926.GA6319@delia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210107133926.GA6319@delia> X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 15:10:01 up 29 days, 19:54, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] 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: , Cc: Pedro Alves , gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" * Tom de Vries [2021-01-07 14:39:27 +0100]: > Hi, > > On rare occasions, we run into this ERROR/UNRESOLVED on gdb-10-branch: > ... > (gdb) PASS: gdb.multi/multi-target.exp: continue: non-stop=on: inferior 2 > Remote debugging from host ::1, port 34088^M > Process outputs/gdb.multi/multi-target/multi-target created; pid = 8649^M > monitor exit^M > (gdb) Killing process(es): 8649^M > ERROR: GDB process no longer exists > GDB process exited with wait status 8627 exp14 0 0 CHILDKILLED SIGABRT SIGABRT > UNRESOLVED: gdb.multi/multi-target.exp: continue: non-stop=on: inferior 5 > ... > > A trigger patch makes the crash happen all the time: > ... > diff --git a/gdb/remote.c b/gdb/remote.c > index 71f814efb365..53ff8b63a1dc 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -14161,14 +14161,12 @@ remote_target::is_async_p () > will be able to delay notifying the client of an event until the > point where an entire packet has been received. */ > > -static serial_event_ftype remote_async_serial_handler; > - > static void > remote_async_serial_handler (struct serial *scb, void *context) > { > - /* Don't propogate error information up to the client. Instead let > - the client find out about the error by querying the target. */ > - inferior_event_handler (INF_REG_EVENT); > + remote_state *rs = (remote_state *) context; > + > + mark_async_event_handler (rs->remote_async_inferior_event_token); > } > > static void > ... > > And using -fsanitizer=address we can get a more elaborate error message: > ... > ==7196==ERROR: AddressSanitizer: heap-use-after-free on address \ > 0x6170000bf258 at pc 0x000001481755 bp 0x7fff05b20840 sp 0x7fff05b20838 > READ of size 8 at 0x6170000bf258 thread T0 > #0 0x1481754 in std::_Hashtable remote_arch_state>, std::allocator remote_arch_state> >, std::__detail::_Select1st, std::equal_to, > std::hash, std::__detail::_Mod_range_hashing, > std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, > std::__detail::_Hashtable_traits > >::_M_bucket_index(unsigned long) const > /usr/include/c++/11/bits/hashtable.h:719 > #1 0x147c8ab in std::_Hashtable remote_arch_state>, std::allocator remote_arch_state> >, std::__detail::_Select1st, std::equal_to, > std::hash, std::__detail::_Mod_range_hashing, > std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, > std::__detail::_Hashtable_traits >::find(gdbarch* > const&) /usr/include/c++/11/bits/hashtable.h:1500 > #2 0x147852c in std::unordered_map std::hash, std::equal_to, > std::allocator > > >::find(gdbarch* const&) /usr/include/c++/11/bits/unordered_map.h:869 > #3 0x14306db in remote_state::get_remote_arch_state(gdbarch*) > src/gdb/remote.c:1203 > #4 0x14309dc in remote_target::get_remote_state() src/gdb/remote.c:1232 > #5 0x1470c08 in remote_async_inferior_event_handler src/gdb/remote.c:14169 > #6 0xaa9f6b in check_async_event_handlers() src/gdb/async-event.c:295 > #7 0x1e93ab4 in gdb_do_one_event() src/gdbsupport/event-loop.cc:194 > #8 0x118f5f9 in start_event_loop src/gdb/main.c:356 > #9 0x118f8ed in captured_command_loop src/gdb/main.c:416 > #10 0x1192d6a in captured_main src/gdb/main.c:1253 > #11 0x1192dfa in gdb_main(captured_main_args*) src/gdb/main.c:1268 > #12 0x97b380 in main src/gdb/gdb.c:32 > #13 0x7f550c211349 in __libc_start_main ../csu/libc-start.c:308 > #14 0x97b199 in _start (build/gdb/gdb+0x97b199) > > 0x6170000bf258 is located 600 bytes inside of 648-byte region \ > [0x6170000bf000,0x6170000bf288) > freed by thread T0 here: > #0 0x7f550f516a57 in operator delete(void*, unsigned long) > (/usr/lib64/libasan.so.6+0xaea57) > #1 0x148b1fe in extended_remote_target::~extended_remote_target() > src/gdb/remote.c:958 > #2 0x143b483 in remote_target::close() src/gdb/remote.c:4074 > #3 0x16cb90f in target_close(target_ops*) src/gdb/target.c:3230 > #4 0x16a2635 in decref_target(target_ops*) src/gdb/target.c:557 > #5 0x16a2abb in target_stack::unpush(target_ops*) src/gdb/target.c:645 > #6 0x16d01ef in inferior::unpush_target(target_ops*) > src/gdb/inferior.h:356 > #7 0x16a2877 in unpush_target(target_ops*) src/gdb/target.c:607 > #8 0x16a2adf in unpush_target_and_assert src/gdb/target.c:655 > #9 0x16a2c57 in pop_all_targets_at_and_above(strata) src/gdb/target.c:678 > #10 0x1442749 in remote_unpush_target src/gdb/remote.c:5522 > #11 0x1458c16 in remote_target::readchar(int) src/gdb/remote.c:9137 > #12 0x145b25b in remote_target::getpkt_or_notif_sane_1(std::vector gdb::default_init_allocator > >*, int, int, > int*) src/gdb/remote.c:9683 > #13 0x145bc9a in remote_target::getpkt_sane(std::vector gdb::default_init_allocator > >*, int) > src/gdb/remote.c:9790 > #14 0x145b040 in remote_target::getpkt(std::vector gdb::default_init_allocator > >*, int) > src/gdb/remote.c:9623 > #15 0x145780b in remote_target::remote_read_bytes_1(unsigned long, > unsigned char*, unsigned long, int, unsigned long*) src/gdb/remote.c:8860 > #16 0x145805e in remote_target::remote_read_bytes(unsigned long, > unsigned char*, unsigned long, int, unsigned long*) src/gdb/remote.c:8987 > #17 0x146113a in remote_target::xfer_partial(target_object, char const*, > unsigned char*, unsigned char const*, unsigned long, unsigned long, > unsigned long*) src/gdb/remote.c:10987 > #18 0x16a4004 in raw_memory_xfer_partial(target_ops*, unsigned char*, > unsigned char const*, unsigned long, long, unsigned long*) > src/gdb/target.c:918 > #19 0x16a4fcf in target_xfer_partial(target_ops*, target_object, char > const*, unsigned char*, unsigned char const*, unsigned long, unsigned > long, unsigned long*) src/gdb/target.c:1156 > #20 0x16a5d65 in target_read_partial src/gdb/target.c:1387 > #21 0x16a5f19 in target_read(target_ops*, target_object, char const*, > unsigned char*, unsigned long, long) src/gdb/target.c:1427 > #22 0x16a5666 in target_read_raw_memory(unsigned long, unsigned char*, > long) src/gdb/target.c:1260 > #23 0xd22f2a in dcache_read_line src/gdb/dcache.c:336 > #24 0xd232b7 in dcache_peek_byte src/gdb/dcache.c:403 > #25 0xd23845 in dcache_read_memory_partial(target_ops*, dcache_struct*, > unsigned long, unsigned char*, unsigned long, unsigned long*) src/gdb/dcache.c:484 > #26 0x16a47da in memory_xfer_partial_1 src/gdb/target.c:1041 > #27 0x16a4a1e in memory_xfer_partial src/gdb/target.c:1084 > #28 0x16a4f44 in target_xfer_partial(target_ops*, target_object, > char const*, unsigned char*, unsigned char const*, unsigned long, > unsigned long, unsigned long*) src/gdb/target.c:1141 > #29 0x18203d4 in read_value_memory(value*, long, int, unsigned long, > unsigned char*, unsigned long) src/gdb/valops.c:956 > > previously allocated by thread T0 here: > #0 0x7f550f515c37 in operator new(unsigned long) > (/usr/lib64/libasan.so.6+0xadc37) > #1 0x14429f0 in remote_target::open_1(char const*, int, int) > src/gdb/remote.c:5562 > #2 0x14405e6 in extended_remote_target::open(char const*, int) > src/gdb/remote.c:4907 > #3 0x16a0f3c in open_target src/gdb/target.c:242 > #4 0xc19ff5 in do_sfunc src/gdb/cli/cli-decode.c:111 > #5 0xc221db in cmd_func(cmd_list_element*, char const*, int) > src/gdb/cli/cli-decode.c:2181 > #6 0x16feda6 in execute_command(char const*, int) src/gdb/top.c:668 > #7 0xee9dc9 in command_handler(char const*) src/gdb/event-top.c:588 > #8 0xeea6a8 in command_line_handler(std::unique_ptr gdb::xfree_deleter >&&) src/gdb/event-top.c:773 > #9 0xee8a12 in gdb_rl_callback_handler src/gdb/event-top.c:219 > #10 0x7f550f24aead in rl_callback_read_char > (/lib64/libreadline.so.7+0x31ead) > ... > > The problem is here in remote_async_inferior_event_handler: > ... > static void > remote_async_inferior_event_handler (gdb_client_data data) > { > inferior_event_handler (INF_REG_EVENT); > > remote_target *remote = (remote_target *) data; > remote_state *rs = remote->get_remote_state (); > ... > > The remote target (passed in the data argument) can be destroyed during the > call to inferior_event_handler. If so, the call to remote->get_remote_state > () is done using a dangling pointer. > > Fix this by increasing the reference count on the remote target before calling > inferior_event_handler, such that it won't get destroyed until right before > returning from remote_async_inferior_event_handler. > > Tested on x86_64-linux. > > Intended for gdb-10-branch. > > The problem has stopped reproducing with the trigger patch since master commit > 79952e69634 "Make scoped_restore_current_thread's cdtors exception free > (RFC)". We could still apply this to master though. > > Any comments? Simon already posted this patch series: https://sourceware.org/pipermail/gdb-patches/2020-November/173633.html Which should address this issue, I don't know if you'd seen this or not? You said above that your patch is proposed for gdb-10-branch, but also that it could be applied to master. I think that this would be fine applied to the gdb-10-branch, but I think Simon's fix would be better for master. Though that said, I notice your credit Simon & Pedro in the ChangeLog, so maybe they've expressed a preference for this solution somewhere and I've just missed it? Thanks, Andrew > > Thanks, > - Tom > > [gdb/remote] Fix invalid pointer in remote_async_serial_handler > > gdb/ChangeLog: > > 2021-01-07 Pedro Alves > Simon Marchi > Tom de Vries > > PR remote/26614 > * remote.c (remote_async_inferior_event_handler): Hold a strong > reference to the remote target while handling an event. > > --- > gdb/remote.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index 1407c23c3ce..4896c2af4d9 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -14163,9 +14163,13 @@ remote_async_serial_handler (struct serial *scb, void *context) > static void > remote_async_inferior_event_handler (gdb_client_data data) > { > + remote_target *remote = (remote_target *) data; > + /* Hold a strong reference to the remote target while handling an > + event, since that could result in closing the connection. */ > + auto remote_ref = target_ops_ref::new_reference (remote); > + > inferior_event_handler (INF_REG_EVENT); > > - remote_target *remote = (remote_target *) data; > remote_state *rs = remote->get_remote_state (); > > /* inferior_event_handler may have consumed an event pending on the