From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id CW92Ac0lpGde0CUAWB0awg (envelope-from ) for ; Wed, 05 Feb 2025 22:00:29 -0500 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=dkcI7oaR; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id E45D41E105; Wed, 5 Feb 2025 22:00:28 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED 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 C81A41E05C for ; Wed, 5 Feb 2025 22:00:27 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4E307385842C for ; Thu, 6 Feb 2025 03:00:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4E307385842C Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=dkcI7oaR Received: from mail-pl1-x629.google.com (mail-pl1-x629.google.com [IPv6:2607:f8b0:4864:20::629]) by sourceware.org (Postfix) with ESMTPS id 4AFEE3858C51 for ; Thu, 6 Feb 2025 02:59:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4AFEE3858C51 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4AFEE3858C51 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::629 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1738810794; cv=none; b=XiVhoh36zHbvuc8f+Z5U+9XOey/EoD6Yfj3ZruOBBXVpcJjwh+/8R0G21f38X7zkzBBG2+E5rdMVVw1RmPwqt8jrm4ID3xdFTmd8eGqBZgcKZXZ87ZEpXKeROnlvim8TyalQ4g3t9jhNBXqsPFogYdDoB35FgDt+Ix1+yPK6ut4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1738810794; c=relaxed/simple; bh=r6snTW0zzjCQeoaor97cAV3gbILflxFsyDjii+i9vZ4=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=K1uCutTMSq4UA1SQTX5IiWlfxWlZe3FGeOvd9Xc3LXdRLW54hJRvFrO8rcdqSmUjZd5QOhwBKC8SypY8sxS8xLUitYelo7eeLss4riUhwl7neQNTWEdI+H+SaExmz6M4vuyIisKUzTXldQmJDOtssfxNmYGLWVML8RVnbWA4cKk= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4AFEE3858C51 Received: by mail-pl1-x629.google.com with SMTP id d9443c01a7336-2166360285dso8213435ad.1 for ; Wed, 05 Feb 2025 18:59:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1738810793; x=1739415593; darn=sourceware.org; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=VcqwDftauG/ikmlNAWrCqqj/QVlv0NEGd0TdqHzzTiA=; b=dkcI7oaRYQspnh3EkspK7lcupcLtDKCbbzJJVNsFqYMt4H0OWDi3VrZyB2pTPpnSGh hxyvUx/ZrC6jdxfY0A82MPUT4aImYNitQ2yvEOromRi/sP3oOZ1TN1D8ec0fUIThjCja GbcSI/LwzZMplyEQ6xgkb0f04F3BpEhxIQaVZ6viCjIeqp+fRVQcg+fO0kv916RNhNXZ vHqal905KEoTfWisMDFFsJIcwVApgw3Oq1RPrBNxV4jm6YdeU1m6Zb4D1bMoMoFVlvn2 uKcxE7OY040YZJOvIVRo06Pk/jiLZ0OTtvmFDuwgwDZva7aBD9BstP9kcLWaN6RyfD3v e43w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738810793; x=1739415593; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=VcqwDftauG/ikmlNAWrCqqj/QVlv0NEGd0TdqHzzTiA=; b=GJILXKLVxZxd2stsOuMc3hFiBy9nSmRD+kY0J/C6+6iIKA4dKafIzph7rO6QshcDP0 8wc9384TcmKr+LAdTOtla+Kl1I55YSbRuM/R6LEwy5EqzdEvDKmzrCBOu5YfFRXM1PTj gTwbtO5whh3qosTxJkt6OJnMpVSAX+XEu18uBtEs3FEXgSqBx2pF+BM7oCTP4IM3Ht6k Cqmc9p8eTCH9iZfutiZdhyVeE+QFQzOuCvlBHdtaiKGz2sL7/EzeKr09MzCcsMBmW5E0 Uim/jvbO7Rt+84vQcNGhLaY1tgs3NWdDx0vy6+RtQQshn8xSogbhyDHnsTcfaV7XxCI1 hhPQ== X-Forwarded-Encrypted: i=1; AJvYcCWg9SrcDwcMU7dr7uxtDg76k+5eqpuxPZJOllWn8R9TS99SCE8KMu59klp/w0Gn2yWjmwu7T6x+R4vz8w==@sourceware.org X-Gm-Message-State: AOJu0YyWNuQn1Flwy3sCiHoENTbkOa7Y9He8v305Noao2JXiSKsoaKdz n2Xr8UQLD781T28HhsoisJ5D7mYjxkyr394xGrmmKgZIqCyBPbxNDcpOvoJ4Va8c3MZKDFmAcub / X-Gm-Gg: ASbGncssgqDiBoc90Kk7k0BtsjoqGpRjTTdRy0ROyZz5y2vEIs6fPymWBG8vsuud7yQ ZWWjclv0k+xwmWE3+1EKAJMfxuBykW7mJPZHtbcWihUG/54KnjddfzLwsGl7RbNZVuo33rYgwqd UUd7KweTPo8obWuB75YymMIX43COSXJTQWyd7H4eswR0OofYCFZ0zK21pKGaz98UUay51+JLJ6I 7t8gJnfGRGgdFewCk9AHaXeJVxhvXFPnUJxDV6WCb8zxNurHHLevGZvVas2mAyqDOPCGYPhOtpm oEO4vpyjYxnS5bcFWdnfwYk= X-Google-Smtp-Source: AGHT+IG2atPqCHGndRWgKfoBEAaRNIeOoObQ627fZM8ZOTd6wrEeUSV1QPYQbkFbDYjEtlyrgTX0eg== X-Received: by 2002:a17:902:e74a:b0:215:72a7:f39f with SMTP id d9443c01a7336-21f17edebe9mr78758985ad.36.1738810793214; Wed, 05 Feb 2025 18:59:53 -0800 (PST) Received: from localhost ([2804:14d:7e39:8470:7e5:16be:a0c6:ff7]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-21f3683d8ffsm1302365ad.150.2025.02.05.18.59.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 18:59:52 -0800 (PST) From: Thiago Jung Bauermann To: "Schimpe, Christina" Cc: Guinevere Larsen , "gdb-patches@sourceware.org" Subject: Re: [PATCH 02/12] gdbserver: Add optional runtime register set type. In-Reply-To: (Christina Schimpe's message of "Thu, 30 Jan 2025 10:28:18 +0000") References: <20241220200501.324191-1-christina.schimpe@intel.com> <20241220200501.324191-3-christina.schimpe@intel.com> <17373ce7-75d2-458f-a9a0-dd9db428371a@redhat.com> User-Agent: mu4e 1.12.8; emacs 29.4 Date: Wed, 05 Feb 2025 23:59:48 -0300 Message-ID: <87tt97ixqz.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain 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 "Schimpe, Christina" writes: >> -----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. */ I agree with Guinevere's remarks, and also with the suggestion of improving the commit message. But I would also like to suggest being a bit more direct in the comment above. At least the way I read it, I thought that EINVAL was a normal thing for ptrace to return if shadow stack was disabled for the process. What about something like: 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 which means that it's unsupported by the kernel. */ > 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? -- Thiago