From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id wRxSN5uEm2cqah8AWB0awg (envelope-from ) for ; Thu, 30 Jan 2025 08:54:35 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ip7VJjLM; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id D231C1E105; Thu, 30 Jan 2025 08:54:35 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-7.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 20E661E08E for ; Thu, 30 Jan 2025 08:54:35 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7B82E3858C78 for ; Thu, 30 Jan 2025 13:54:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7B82E3858C78 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ip7VJjLM Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id D05783858D34 for ; Thu, 30 Jan 2025 13:53:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D05783858D34 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D05783858D34 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1738245233; cv=none; b=MnCQQUrE0zoNWrb1mH7fZuKpIAsM8kpFTZUBKwLdSN0zYhzCNYSLt3cRIbghILnrQ+lB6VCH6OKElf0myLRqW7Rp3AU/Tdw6QzVY2UzULYcADuPJkElkBS+PwYJVzS7xyqhhAsPOATL2vqyHbYD+rFm3tx8jbCYPQASnfwPTfxg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1738245233; c=relaxed/simple; bh=+APhnUzNP5LbGltCCTVpLLDwy7uAlh8WWDqWCgyigAo=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=UNl73spA05NA/tDYAXDkSpdihLcjrxjZFH0kEXHxJg4UmmxDELSu/JktICG3Zh3/45AgdnLZGO0k2hi/sO3nUA7rGZRoXYoKnx9jiXwlwUfAxtEaliCv9QqS6V6ZS3keVkbr+YfSSYFpkiN2/DTZE22uayAhafMG/gPSupEBLw8= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D05783858D34 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1738245233; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bpY6GADS79UIy5pyGVJg2UBgvOpP4j7UMvnOcdK/rXg=; b=Ip7VJjLM/PZqBy9ITrl5ifa8s/5VeV+bG1SOGteLQhT+Hr6IHSc25C/D0sPTL9mG6l4mc9 Q80GYLq7NSfVpn/mjPWY7vLBnaEFzZbL5MEwgkkVd2p/qA4mhhVypL0zh+s5m3HFEr/4au mYyn2Nx4cdBvm+xuIpqWmtsfjGhkAZU= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-426-Es1Ed0umOdCgQNKwTisSXg-1; Thu, 30 Jan 2025 08:53:52 -0500 X-MC-Unique: Es1Ed0umOdCgQNKwTisSXg-1 X-Mimecast-MFC-AGG-ID: Es1Ed0umOdCgQNKwTisSXg Received: by mail-qv1-f71.google.com with SMTP id 6a1803df08f44-6dcd1e4a051so15279366d6.2 for ; Thu, 30 Jan 2025 05:53:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738245232; x=1738850032; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bpY6GADS79UIy5pyGVJg2UBgvOpP4j7UMvnOcdK/rXg=; b=YWMTOeBy78grP8fWdUFoA0oHVvKjr6tbeVwnXrWZ+w4hAhEeEWH3yDBit6xUWUZvSK pqvoyChxfmkB/HWntoGx0GvAPGZSsgA9scNZjn077Srqy0GOuCcoEVV7rSpoLDaOE9uf w4ix46Hmpqp8fKDzpQ4M46k9Qvg0tRkXALOAAP2b83Q6gNCZqXRudSTiO/Z8b6C2sHyD e9StP2l3WMxvFiCOtBrNNGjZTrbVBOTSHTbKq7AJyFe7FJvNEPN1Ny/txkDLRbQcmBu+ c421XM87UDr/kmSG2elkWhkpjpO49YcweV0e6Ms7i8wY4xm8hTFsV59NKnKJW2SZkdHC 5Jxw== X-Forwarded-Encrypted: i=1; AJvYcCXtOfYgoWPoBUdHrjXhO8h31TnAjZ7Ofn+L9UwoIY+0Q1/oEFm1wIlz+rYTAJD0FqThUwxSiPDhVDOaWA==@sourceware.org X-Gm-Message-State: AOJu0Ywt31KtvLrMufu9UeJIJ3ubhmZXGfButW7MQT6+gygVUzDjUgdS FHi1BzI38Djt5ZKkc1LEpovzZUu4Jssl9+EmPSp52il7S8iuLBvdcEUxLcTWc8NPNDBd3CHBhKI UaznHR9S/WZHJYZOrnA/PK/Udg5SoPYOplfU7K9zx7JDhGBqeinU5uXvTenA= X-Gm-Gg: ASbGncsjiysDDa9YoelDphb9/arGsyi50oV+aJjdZpYJf6KxLd6tRhMSRD7pW7RFHwe p3J+HUl79TRVVy+76aMsdKheOOhSx8j7KiZAi0fKvXgdEGpozNUhNsBleBw4KzTBd5Lkyi05798 4yeKZVgEkiIlThNHuk7ZnbU6u0KHvV+9dBQ1cEuezVvwx2p0W8E/5k1SQ9cjxyOuKqvw6/Zq1Kf 0gfqVcxYG08RvqLE+eXcqB8j/7ZxzEH+SCoU4tghXoByy+S/PznMundjIyiiHN2Yw5YaCIt+mNo /AShjMy/372CYOdY X-Received: by 2002:ad4:4eeb:0:b0:6d8:af66:6344 with SMTP id 6a1803df08f44-6e243befc16mr120699806d6.2.1738245232057; Thu, 30 Jan 2025 05:53:52 -0800 (PST) X-Google-Smtp-Source: AGHT+IHYGCpimrB5sMszOhdsMo3VY9eqXTKo21yeukQytjJGL1EDD6xwTpP8bATLb1TGvwmCYStPMw== X-Received: by 2002:ad4:4eeb:0:b0:6d8:af66:6344 with SMTP id 6a1803df08f44-6e243befc16mr120699546d6.2.1738245231741; Thu, 30 Jan 2025 05:53:51 -0800 (PST) Received: from ?IPV6:2804:14d:8084:9a69::1002? ([2804:14d:8084:9a69::1002]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6e2547f0e36sm6545666d6.18.2025.01.30.05.53.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 30 Jan 2025 05:53:51 -0800 (PST) Message-ID: <46ab3983-6fed-476a-b01b-a20867e84c33@redhat.com> Date: Thu, 30 Jan 2025 10:53:49 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 02/12] gdbserver: Add optional runtime register set type. To: "Schimpe, Christina" , "gdb-patches@sourceware.org" References: <20241220200501.324191-1-christina.schimpe@intel.com> <20241220200501.324191-3-christina.schimpe@intel.com> <17373ce7-75d2-458f-a9a0-dd9db428371a@redhat.com> From: Guinevere Larsen In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: DfPes9EHwmkvjXonIpuPXmLUK3lYrmt5kmPmqimPyEk_1738245232 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 1/30/25 7:28 AM, Schimpe, Christina wrote: >> -----Original Message----- >> From: Guinevere Larsen >> Sent: Tuesday, January 28, 2025 2:35 PM >> To: Schimpe, Christina ; gdb- >> patches@sourceware.org >> Subject: Re: [PATCH 02/12] gdbserver: Add optional runtime register set type. >> >> On 12/20/24 5:04 PM, Schimpe, Christina wrote: >>> Some register sets can be activated and deactivated by the OS during >>> the runtime of a process. One example register is the Intel CET >>> shadow stack pointer. This adds a new type of register set to handle >>> such cases. We shouldn't deactivate these regsets and should not show >>> a warning if we fail to read them. >>> --- >>> gdbserver/linux-low.cc | 40 ++++++++++++++++++++++++++++------------ >>> gdbserver/linux-low.h | 7 ++++++- >>> 2 files changed, 34 insertions(+), 13 deletions(-) >>> >>> diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc index >>> 50ce2b44927..355b28d9fe4 100644 >>> --- a/gdbserver/linux-low.cc >>> +++ b/gdbserver/linux-low.cc >>> @@ -5007,23 +5007,31 @@ regsets_fetch_inferior_registers (struct >> regsets_info *regsets_info, >>> if (res < 0) >>> { >>> if (errno == EIO >>> - || (errno == EINVAL && regset->type == OPTIONAL_REGS)) >>> + || (errno == EINVAL >>> + && (regset->type == OPTIONAL_REGS >>> + || regset->type == OPTIONAL_RUNTIME_REGS))) >>> { >>> /* If we get EIO on a regset, or an EINVAL and the regset is >>> - optional, do not try it again for this process mode. */ >>> + optional, do not try it again for this process mode. >>> + Even if the regset can be enabled at runtime it is safe >>> + to deactivate the regset in case of EINVAL, as we know >>> + the regset itself was the invalid argument of the ptrace >>> + call. */ >>> disable_regset (regsets_info, regset); >> I'm somewhat confused by this patch. >> >> The commit message and other comments here say that optional_runtime_regs >> shouldn't be disabled. However, in here, if we get EINVAL we *will* disable the >> regset. Did you mean to use != instead of == ? >> >> I'll be honest, I don't know enough about the regset subsystem to know which is >> the correct option, I just think it has to be consistent. >> > Hi Guinevere, > > Thank you for the review. > > For the errno EINVAL we want to disable the regset, as we don't want to call ptrace > using NT_X86_SHSTK again. This specific errno can happen if the kernel does not > support ptrace using NT_X86_SHSTK (older linux kernels). I tried to explain that here: > >>> + Even if the regset can be enabled at runtime it is safe >>> + to deactivate the regset in case of EINVAL, as we know >>> + the regset itself was the invalid argument of the ptrace >>> + call. */ > In case shadow stack is just not active but supported by the kernel we see > ENODEV and we don't disable the regset, which I explained in another > comment for the corresponding code area: > > /* ENODATA or ENODEV may be returned if the regset is > currently not "active". For ENODEV we additionally check > if the register set is of type OPTIONAL_RUNTIME_REGS. > This can happen in normal operation, so suppress the > warning in this case. > > I didn't want to be too specific here to make the commit generic. > > Is there something I could add to make it more understandable or maybe > there is just some information missing in the commit message? Yeah, ok, so the commit message needs an update. Maybe something like: Some register sets can be activated and deactivated by the OS during the runtime of a process. One example register is the Intel CET shadow stack pointer. This adds a new type of register set to handle such cases. When reading them, we shouldn't deactivate these regsets and should not show a warning if they are deactivated, but should deactivate them if they are unsupported by the kernel. That can be deciphered based on the error returned by the ptrace call, if we fail to read the registered. Or something similar. I think it is important to explain in the commit message that one error means "inactive" while other means "unsupported", so that in 5-10 years we can look back at this commit and be sure the disable wasn't added incorrectly. -- Cheers, Guinevere Larsen She/Her/Hers