From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 5BB203858D37 for ; Wed, 1 Feb 2023 10:54:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5BB203858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675248868; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=caUW5tFZKXvqDbqTWcMr1WtKAXVGlR+yoddZReBEUdg=; b=Kfug+MnbdQz2TqWanHdelRyzzXbAX3kTKKHBscZ/zPxHr0gpfESQ/lWKFDE06xQO8N8XI9 SSMt3oKUKM2ESTBNNOUCCLTXrHvShjrlc4XOvc7EH5Y9n7kFnkiElGclRPKLSp5K9Z7xXQ YNtFVIJZQqNHbgVogJ8jnFyOVH00Vs0= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-441-iHXiwD6MPTmI3GWNYaiPIA-1; Wed, 01 Feb 2023 05:54:27 -0500 X-MC-Unique: iHXiwD6MPTmI3GWNYaiPIA-1 Received: by mail-qk1-f200.google.com with SMTP id w17-20020a05620a425100b00706bf3b459eso10996333qko.11 for ; Wed, 01 Feb 2023 02:54:27 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=caUW5tFZKXvqDbqTWcMr1WtKAXVGlR+yoddZReBEUdg=; b=Q7tCv0MDisyM4Hd2tUsLsoCsAmqIkMtSbtTxZseyww1FTfv1Kof3gcE0nxL7sESmde pCCAzmCxCLGESjg0mVm1COGGlxwDavPEvPm9zKksBDPCAGuTHjnGi+8XCiPq05+9yeaY jSSuHffzUunD95bB0JGNqxJRjx3fpLZ37lf+WvNl70eD4/LZIzlZam7Tw3HzYPa4jy16 HTszg+Ne4huxplz1etIL9+GilYJKgwblf6KLBSHs5XdPzTGBW6jSP0Heztnri5gF/+8Q wY/oHoUbt3v+2GejK1mhNPwslGa+CkRY6oXg4N1TQuIljBYSuKnPSXF6mrOqN7anltg8 M8eA== X-Gm-Message-State: AO0yUKVAQEmbRL2ILUiKJBVajMF7Vak3Z8P/0yZMgoBJkCE/3Ow4q123 LtH/M9AHvhMggsSNetMoEQ2/jxuRyCMvu/YAchsmiLZmk/ffN+do5jnwma5RGTodp6tLdW8e4WN Adwu1PMSrz52GZED07rG/It89Frn57XRG4NLQ5yngVrjrMJ+g9QjNt4/Lgp1dIN2JtqZvGB0V4Q == X-Received: by 2002:a05:6214:14ae:b0:535:53eb:d1cc with SMTP id bo14-20020a05621414ae00b0053553ebd1ccmr2708286qvb.6.1675248866426; Wed, 01 Feb 2023 02:54:26 -0800 (PST) X-Google-Smtp-Source: AK7set8DxrqWqxZS+ZhpDceFW4uA18ZK6bHzox3FRYYns18qtvC9piF8/qZo3h2g/btGEUHxM31p8g== X-Received: by 2002:a05:6214:14ae:b0:535:53eb:d1cc with SMTP id bo14-20020a05621414ae00b0053553ebd1ccmr2708269qvb.6.1675248866045; Wed, 01 Feb 2023 02:54:26 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id q1-20020a05620a0c8100b006e99290e83fsm11767567qki.107.2023.02.01.02.54.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Feb 2023 02:54:25 -0800 (PST) From: Andrew Burgess To: Thiago Jung Bauermann via Gdb-patches , gdb-patches@sourceware.org Cc: Thiago Jung Bauermann , Simon Marchi Subject: Re: [PATCH v3 2/8] gdbserver: Add PID parameter to linux_get_auxv and linux_get_hwcap In-Reply-To: <20230130044518.3322695-3-thiago.bauermann@linaro.org> References: <20230130044518.3322695-1-thiago.bauermann@linaro.org> <20230130044518.3322695-3-thiago.bauermann@linaro.org> Date: Wed, 01 Feb 2023 10:54:23 +0000 Message-ID: <87sffpu04g.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP 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: Thiago Jung Bauermann via Gdb-patches writes: > This patch doesn't change gdbserver behaviour, but after later changes are > made it avoids a null pointer dereference when HWCAP needs to be obtained > for a specific process while current_thread is nullptr. > > Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID > parameter seems more correct than setting current_thread in one particular > code path. > > Changes are propagated to allow passing the new parameter through the call > chain. > > Approved-By: Simon Marchi > --- > gdbserver/linux-aarch64-low.cc | 7 ++++--- > gdbserver/linux-arm-low.cc | 2 +- > gdbserver/linux-low.cc | 18 +++++++++--------- > gdbserver/linux-low.h | 9 ++++----- > gdbserver/linux-ppc-low.cc | 6 +++--- > gdbserver/linux-s390-low.cc | 2 +- > gdbserver/netbsd-low.cc | 4 +--- > gdbserver/netbsd-low.h | 2 +- > gdbserver/server.cc | 3 ++- > gdbserver/target.cc | 4 ++-- > gdbserver/target.h | 4 ++-- > 11 files changed, 30 insertions(+), 31 deletions(-) > > diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc > index 3c09e086afee..2ed6e95562c5 100644 > --- a/gdbserver/linux-aarch64-low.cc > +++ b/gdbserver/linux-aarch64-low.cc > @@ -846,12 +846,13 @@ aarch64_target::low_arch_setup () > if (is_elf64) > { > struct aarch64_features features; > + int pid = current_thread->id.pid (); > > features.vq = aarch64_sve_get_vq (tid); > /* A-profile PAC is 64-bit only. */ > - features.pauth = linux_get_hwcap (8) & AARCH64_HWCAP_PACA; > + features.pauth = linux_get_hwcap (pid, 8) & AARCH64_HWCAP_PACA; > /* A-profile MTE is 64-bit only. */ > - features.mte = linux_get_hwcap2 (8) & HWCAP2_MTE; > + features.mte = linux_get_hwcap2 (pid, 8) & HWCAP2_MTE; > features.tls = aarch64_tls_register_count (tid); > > current_process ()->tdesc = aarch64_linux_read_description (features); > @@ -3322,7 +3323,7 @@ aarch64_target::supports_memory_tagging () > #endif > } > > - return (linux_get_hwcap2 (8) & HWCAP2_MTE) != 0; > + return (linux_get_hwcap2 (current_thread->id.pid (), 8) & HWCAP2_MTE) != 0; > } > > bool > diff --git a/gdbserver/linux-arm-low.cc b/gdbserver/linux-arm-low.cc > index 98ba0e02524b..5975b44af0ae 100644 > --- a/gdbserver/linux-arm-low.cc > +++ b/gdbserver/linux-arm-low.cc > @@ -958,7 +958,7 @@ get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self) > static const struct target_desc * > arm_read_description (void) > { > - unsigned long arm_hwcap = linux_get_hwcap (4); > + unsigned long arm_hwcap = linux_get_hwcap (current_thread->id.pid (), 4); > > if (arm_hwcap & HWCAP_IWMMXT) > return arm_linux_read_description (ARM_FP_TYPE_IWMMXT); > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc > index 7e1de3978933..5cd22824e470 100644 > --- a/gdbserver/linux-low.cc > +++ b/gdbserver/linux-low.cc > @@ -5483,12 +5483,11 @@ linux_process_target::supports_read_auxv () > to debugger memory starting at MYADDR. */ > > int > -linux_process_target::read_auxv (CORE_ADDR offset, unsigned char *myaddr, > - unsigned int len) > +linux_process_target::read_auxv (int pid, CORE_ADDR offset, > + unsigned char *myaddr, unsigned int len) > { > char filename[PATH_MAX]; > int fd, n; > - int pid = lwpid_of (current_thread); > > xsnprintf (filename, sizeof filename, "/proc/%d/auxv", pid); > > @@ -6982,14 +6981,15 @@ linux_get_pc_64bit (struct regcache *regcache) > /* See linux-low.h. */ > > int > -linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp) > +linux_get_auxv (int pid, int wordsize, CORE_ADDR match, CORE_ADDR *valp) > { > gdb_byte *data = (gdb_byte *) alloca (2 * wordsize); > int offset = 0; > > gdb_assert (wordsize == 4 || wordsize == 8); > > - while (the_target->read_auxv (offset, data, 2 * wordsize) == 2 * wordsize) > + while (the_target->read_auxv (pid, offset, data, 2 * wordsize) > + == 2 * wordsize) > { > if (wordsize == 4) > { > @@ -7019,20 +7019,20 @@ linux_get_auxv (int wordsize, CORE_ADDR match, CORE_ADDR *valp) > /* See linux-low.h. */ > > CORE_ADDR > -linux_get_hwcap (int wordsize) > +linux_get_hwcap (int pid, int wordsize) > { > CORE_ADDR hwcap = 0; > - linux_get_auxv (wordsize, AT_HWCAP, &hwcap); > + linux_get_auxv (pid, wordsize, AT_HWCAP, &hwcap); > return hwcap; > } > > /* See linux-low.h. */ > > CORE_ADDR > -linux_get_hwcap2 (int wordsize) > +linux_get_hwcap2 (int pid, int wordsize) > { > CORE_ADDR hwcap2 = 0; > - linux_get_auxv (wordsize, AT_HWCAP2, &hwcap2); > + linux_get_auxv (pid, wordsize, AT_HWCAP2, &hwcap2); > return hwcap2; > } > > diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h > index aebfe05707e5..221de85aa2ee 100644 > --- a/gdbserver/linux-low.h > +++ b/gdbserver/linux-low.h > @@ -178,7 +178,7 @@ class linux_process_target : public process_stratum_target > > bool supports_read_auxv () override; > > - int read_auxv (CORE_ADDR offset, unsigned char *myaddr, > + int read_auxv (int pid, CORE_ADDR offset, unsigned char *myaddr, > unsigned int len) override; > > int insert_point (enum raw_bkpt_type type, CORE_ADDR addr, > @@ -946,17 +946,16 @@ extern int have_ptrace_getregset; > *VALP and return 1. If not found or if there is an error, return > 0. */ > > -int linux_get_auxv (int wordsize, CORE_ADDR match, > - CORE_ADDR *valp); > +int linux_get_auxv (int pid, int wordsize, CORE_ADDR match, CORE_ADDR *valp); > > /* Fetch the AT_HWCAP entry from the auxv vector, where entries are length > WORDSIZE. If no entry was found, return zero. */ > > -CORE_ADDR linux_get_hwcap (int wordsize); > +CORE_ADDR linux_get_hwcap (int pid, int wordsize); > > /* Fetch the AT_HWCAP2 entry from the auxv vector, where entries are length > WORDSIZE. If no entry was found, return zero. */ > > -CORE_ADDR linux_get_hwcap2 (int wordsize); > +CORE_ADDR linux_get_hwcap2 (int pid, int wordsize); Ideally the comment for these three functions would be updated to mention the PID argument. Thanks, Andrew