From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D9D79C5320E for ; Sun, 25 Aug 2024 18:16:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6C9158D0025; Sun, 25 Aug 2024 14:16:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 69C4E8D0022; Sun, 25 Aug 2024 14:16:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 566178D0025; Sun, 25 Aug 2024 14:16:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 38C8F8D0022 for ; Sun, 25 Aug 2024 14:16:50 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id E3BF2C0437 for ; Sun, 25 Aug 2024 18:16:49 +0000 (UTC) X-FDA: 82491573738.13.6F536BB Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf03.hostedemail.com (Postfix) with ESMTP id 560C320015 for ; Sun, 25 Aug 2024 18:16:48 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=none; spf=pass (imf03.hostedemail.com: domain of cmarinas@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=cmarinas@kernel.org; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724609789; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HF50t7QhNR/0jEpO5qPHMd1FOGaXm6NkrNxsKc9YqVM=; b=kKlM9wJrtwq8WPe0pI6A1v7ul/g0d8zBYrPwwqGy5xAW6Zt5RgtsGFer2emhHpETwxZy5s m5NH3i4nUcSjbh3sgvPzbInLXfDuicJ8xMaohipbXW+nvvjJqlQrtoaBmTRq/DYRpj8ke3 +0vuj1Gw7evtEeqi+kH7ezXNWcsESFc= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=none; spf=pass (imf03.hostedemail.com: domain of cmarinas@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=cmarinas@kernel.org; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724609789; a=rsa-sha256; cv=none; b=hQc499jEJdASCNd5nClL4eWltwCa2yCdUU95u4SBXN2euVooj9O1hiWWscXerG+IRqoMiB Zx6TMwKnIhbMh9xpcnuO4x/0IsoWbc3PCbiUu6CRorFA6W9INqcwbH2LF/o67zZNpUwxGw uNI7N6rGlHQcOC6cQzRS/gukRFFsIoE= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id BAA8FA426D0; Fri, 23 Aug 2024 15:59:13 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4B7C1C32786; Fri, 23 Aug 2024 15:59:14 +0000 (UTC) Date: Fri, 23 Aug 2024 16:59:11 +0100 From: Catalin Marinas To: Mark Brown Cc: Will Deacon , Jonathan Corbet , Andrew Morton , Marc Zyngier , Oliver Upton , James Morse , Suzuki K Poulose , Arnd Bergmann , Oleg Nesterov , Eric Biederman , Shuah Khan , "Rick P. Edgecombe" , Deepak Gupta , Ard Biesheuvel , Szabolcs Nagy , Kees Cook , "H.J. Lu" , Paul Walmsley , Palmer Dabbelt , Albert Ou , Florian Weimer , Christian Brauner , Thiago Jung Bauermann , Ross Burton , Yury Khrustalev , Wilco Dijkstra , linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org, kvmarm@lists.linux.dev, linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: Re: [PATCH v11 25/39] arm64/signal: Expose GCS state in signal frames Message-ID: References: <20240822-arm64-gcs-v11-0-41b81947ecb5@kernel.org> <20240822-arm64-gcs-v11-25-41b81947ecb5@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Stat-Signature: 3w5ccpkwqmtjgycsefe41zgpsj4bw7ob X-Rspamd-Queue-Id: 560C320015 X-Rspamd-Server: rspam11 X-HE-Tag: 1724609808-745671 X-HE-Meta: U2FsdGVkX1+84eduUA6lqupkj7CuiW8JEFR0Ozr3d9UBQSWwMQgH7c018r831X3TuzyvFdTyg3MeKRYnTOMX1ZMPFeUBDsbbCg12mN+/UgADSCrY5ISjXuFa3GqCjQFQ4gRQctYSfJZYQzw45qGGv1tCE3hZmnOeOqyedoBN+Ta30FiazixzsDh5yvmcHoRzsMcNfPkwkf5Z78a1W/Oc+naSXm+WJ7jBblOBBJ8UJAHvsqj2sMQ1R+8h4hDztlDN3Cut4aEaQ5YmePhBRjqGLx72oWVaEchryuTD0mmGytqiGDg+Mqo8f8LxoJAAEsmXy3H4N35FMa+nlgTbD0xdXqTfsH1EN7l2lWo8jI7ho+RgjjE2jRROkFw4yrhj8c5AhKu2qUOehR7Cby31Wa7gowSvwy2hBfN+m98EO+bm2Ep5+KpmSH9/CfuKIJjdGtZPdsMKO0tjuF36oKgcufhEGLYCKE16qmKwUfwG00n+SL/xzSmmaEohNwqzNMRufjVpxLBoFVQNt2mRwa/xx3QTjRk1luoCDAdAEmkeobkS+f2VicfOcRlBEKC6NDLKNUlIJ43dPp6xY3tmHaCSZKbPdjbkPwJB5KyeC9yOAXc6M/nS19Ifg5aLpG9wSFq2TjwClnzmXMsZYSOIK1DfNvBr94E4iwO0dZnj28v663EclNXWWhZZhNtJDBB29dYZRjEG/esjpERBNluaajPX6WigHOoLRzSHyq3kQpJcQCveziLVrssJzfKsbTb/Ifr0eu+ycXkVf7W7Xr5F9ruxQ71M3WRfr1JJy6aCVTHeqoOgK3GvmpwhYLBWoa70TQOIA013EqxlpDmf2rhixeKwVh226gzPnGlbsyxdQ/HmAcEXwv2oZpwxzHJTz+C+3PB6sFNh/E20eksLXAufbNYMsyIZS2p9PCmqJuWHV7PGIgmUV4BqcwXHIMqwJCG3gh//Rzy7L7EIv4K+FUvRmEjd/OF olu70pAw U2a9LfRAzOIX9c7tcYMYd+Oy6EeJvJh6+DyWC6Agw7WsClrzUfVDHicDE1/2vw6HOhaatA5k5JvrE9ty7GWjtJXgrF5wpnJ9NR6IGZp0n8o3YTc5ZAuP5JOvh2oPoycWel0J2Pj1o2O//e8UFoCp67TfopR7AnFqyJnrEMFTAPK8dVQjTDp1oToOT5qKx0yMKYE0v74ysE9Lao7QOa1NQ/pYjwPHDhyDlu6UH/gnv3DqY62cV7z4MwgSv9l/KG5bNbRu9NVFjUNrHlGZWhD6zc/agXdE515Ck2LnGCC2HqwyO+kzJuuE6mYDP+t58dFw9pIGSm9MHCYOCaLA3qL/qPfWKfgZ2pi6m/uA76BzFwlbVJ9kxMrlq6ssaQ3z9aD7lGU6QIdKUyCavkWjI6mlcfQWaPgrt9qIt1uHZLbDoHu8jO3NlRinU23qzli3qj/rNJkqbt7AVnlN9Jy4= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Aug 23, 2024 at 11:25:30AM +0100, Mark Brown wrote: > On Fri, Aug 23, 2024 at 10:37:19AM +0100, Catalin Marinas wrote: > > On Thu, Aug 22, 2024 at 02:15:28AM +0100, Mark Brown wrote: > > > > + gcs_preserve_current_state(); > > > + gcspr = current->thread.gcspr_el0 - 8; > > > > + __put_user_error(gcspr, &ctx->gcspr, err); > > > Do we actually need to store the gcspr value after the cap token has > > been pushed or just the value of the interrupted context? If we at some > > point get a sigaltshadowstack() syscall, the saved GCS wouldn't point to > > the new stack but rather the original one. Unwinders should be able to > > get the actual GCSPR_EL0 register, no need for the sigcontext to point > > to the new shadow stack. > > We could store either the cap token or the interrupted GCSPR_EL0 (the > address below the cap token). It felt more joined up to go with the cap > token since notionally signal return is consuming the cap token but > either way would work, we could just add an offset when looking at the > pointer. In a hypothetical sigaltshadowstack() scenario, would the cap go on the new signal shadow stack or on the old one? I assume on the new one but in sigcontext we'd save the original GCSPR_EL0. In such hypothetical case, the original GCSPR_EL0 would not need 8 subtracted. I need to think some more about this. The gcs_restore_signal() function makes sense, it starts with the current GCSPR_EL0 on the signal stack and consumes the token, adds 8 to the shadow stack pointer. The restore_gcs_context() one is confusing as it happens before consuming the cap token and assumes that the GCSPR_EL0 value actually points to the signal stack. If we ever implement an alternative shadow stack, the original GCSPR_EL0 of the interrupted context would be lost. I know it's not planned for now but the principles should be the same. The sigframe.uc should store the interrupted state. To me the order for sigreturn should be first to consume the cap token, validate it etc. and then restore GCSPR_EL0 to whatever was saved in the sigframe.uc prior to the signal being delivered. > > Also in gcs_signal_entry() in the previous patch, we seem to subtract 16 > > rather than 8. > > We need to not only place a cap but also a GCS frame for the sigreturn > trampoline, the sigreturn trampoline isn't part of the interrupted > context so isn't included in the signal frame but it needs to have a > record on the GCS so that the signal handler doesn't just generate a GCS > fault if it tries to return to the trampoline. This means that the > GCSPR_EL0 that is set for the signal handler needs to move two entries, > one for the cap token and one for the trampoline. Yes, this makes sense. > > What I find confusing is that both restore_gcs_context() and > > gcs_restore_signal() seem to touch current->thread.gcspr_el0 and the > > sysreg. Which one takes priority? I should probably check the branch out > > to see the end result. > > restore_gcs_context() is loading values from the signal frame in memory > (which will only happen if a GCS context is present) then > gcs_restore_signal() consumes the token at the top of the stack. The > split is because userspace can skip the restore_X_context() functions > for the optional signal frame elements by removing them from the context > but we want to ensure that we always consume a token. I agree we should always consume a token but this should be done from the actual hardware GCSPR_EL0 value on the sigreturn call rather than the one restored from sigframe.uc. The restoring should be the last step. -- Catalin