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 86404C531DC for ; Fri, 23 Aug 2024 10:25:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DAF4180088; Fri, 23 Aug 2024 06:25:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D5F2580084; Fri, 23 Aug 2024 06:25:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BFFD780088; Fri, 23 Aug 2024 06:25:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id A0CA380084 for ; Fri, 23 Aug 2024 06:25:37 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 43CEF81B58 for ; Fri, 23 Aug 2024 10:25:37 +0000 (UTC) X-FDA: 82483128714.08.55FFE8F Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf24.hostedemail.com (Postfix) with ESMTP id 2A501180004 for ; Fri, 23 Aug 2024 10:25:34 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=p85uLFPz; spf=pass (imf24.hostedemail.com: domain of broonie@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=broonie@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724408718; 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:dkim-signature; bh=vgmYubq5HUXowh65b00woM9rn/yYQJJRM4nTVJol038=; b=2wPhZyAjksUbcHJ6gSQIKUSTv9JtX7SP0DFfovZT0nzRj7tB2+NpT4bLqqwssXVQfNXWmb YiIQ6GhQz0jq842RLdtbjPtduZIDwCzXsEUahBuwQ1w+Wykm5o5/yBmjpMhMUbYPSBxLpC NQFzxiAKdibSlBn+mwRSxzxtTfAqvqU= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=p85uLFPz; spf=pass (imf24.hostedemail.com: domain of broonie@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=broonie@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724408718; a=rsa-sha256; cv=none; b=2sEtjmMzcO2nB4+QChFDs/svY5fGnqWE1KN/aVOJRtnAjNcrmwqcUxe78fqgyQRpIzsn+V 5u4dMNRNXevr+ByS6Bxj2hMdlNYC2YbfY8lVAB9XcpM/UXqZ8EzBPTLs5MX51aisaHTdws eU6S0NfNxnK5q1rx4q37irHeDbIU+1c= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 3D95660C01; Fri, 23 Aug 2024 10:25:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 73EFEC32786; Fri, 23 Aug 2024 10:25:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1724408733; bh=sQqhNYedR1BK3builnVoyg4SxBnBW3yPxbGa623Z0Lg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=p85uLFPzRzTjF8TlOpeiA4rumcvxDEt5Fylq/9D05p3sIyY+055i0xe3zxSZLAoqN QZHNkumhB+xPzgZ/sxTPyBYYrNrG8f5anr2pue94kpefrN+CCKb4iLhzMhRTBpSA0K sVYCVfW5FfB2qLD+0FFI6UGF0ZbAxjh5/DQ4oH00TXYoDQs14NRelVjlKHMrBh/wqb WIAQ2tuAChr2+JPT3qMr5aq++L4A6q4aPfN6kAHB7/S6XhE8E13elQ+Sv0HMTl8mQd AV1ckfw3bNkrK34vkLNFaaA8WrSpMbnM/HArzMkeG+5njHE00+M1umnvsYvWZyPQyj cpi/uhsdQGqjA== Date: Fri, 23 Aug 2024 11:25:30 +0100 From: Mark Brown To: Catalin Marinas 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: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="/CrZc0/PCTYKFxlL" Content-Disposition: inline In-Reply-To: X-Cookie: Your love life will be... interesting. X-Rspam-User: X-Stat-Signature: 4owdeqkymyaycyj7qm9awtimuibzkf7k X-Rspamd-Queue-Id: 2A501180004 X-Rspamd-Server: rspam11 X-HE-Tag: 1724408734-369105 X-HE-Meta: U2FsdGVkX1+XWjGZJQDAnj5ybJnxoTqzH1MBfAC/T3VHUWG6Tf5hCECTG8ujtx79PuLJH3dLBlr3IJRknebtMvnK4DXXzpZbLJOHeZF0sel9XVggDWpC68FexKPyTS0p7JRKMoGTYCc+nuPP/SdCDtGsZx8mniT3xKu32ob94fFjp2yBFpncA2P7XLxN1QATHe1h15XvrRhs3ztAk1GhbF3m2WDW4g/K64b3qq8n7HS0IRIvwGzdhXASfpU0AG5EEobz3XfyNOkBMLcJHi76+ElLYsiidPIPY3Yn3fFNOPq7TKkXt09iCP/YaHS3tUsyc5oyIEnRjQMJqZo9PzP1qptNwtW2hjjTIFJb+jmBDCKxQ0J7y+/FBVvpVViKdutQf7ZmlBchDp79rvCquG0EYYbDdUm1CFjO6dqTQFjQsP2JP93VachmY5CMox+/vzjC2eTezdlKIDsp/RhVMtga3BXUhtMV3ieMIe7wd1WD6LkormXv5qjX4C2+H7fnExPKG2AXDBTLMvgAoXAmc+97t+BzjR/ZrXsslfTDhqu72d7+h6HCjjAnXn5BMxZeLMyo8DBkv+vh8x1JcnVzsFctbkI+3yKwzrkYls0KkvMFKJFpS9EumX//oxZJWAL76gE9pqgW86fYmuuJDv+R3mGe3ZCHVcclWz/EQMgt/XVQ6U7cIjYo6QkQozJSJYyTg5ZFsVhXya3U0mmaU1UmYwtSBuhswrUFN0E+Cm9OeyEuhJILICHqkbKtffsVUupa0ltVXThYOv2hd/iQmiI5cl35zg7sM7IbYD/zKitFc/3hweeWXAMzRCD2qDXK2vCpy/8uybMF80E81iDRn+KcbCh1WlmGQoQWhvafIyUw+g6JHh/8Bfn70WLnTaSZJd+cPcjlIoyZKeREcywYdgO4W5O6XlKxM6bhiVrFwFwGF5o789hLroZZK8WNC/N08622CCMS7IYCxZE2XUIdkY630Ky M/Av1Wff cfGtQHFmfB3A3M/O5n2xW2J5i+7zgp8hKs8Tb8gHkL0EQUxIXroiSktJRPg3YwgaOjhQX9DpzBkQQvUEXi14agqcAdDUXA+aw/zwV3ru78lp1sQLMDpuwI7t9gJUs91u/cxrsll2l4iMZ3kbHRvWrFQXGWGXGHKaCr9BvUKu5Kd6Eb/USONfbhfXlb8bIxs74MWlKGPCk9loHn6KFSpxnkv3w9R1cizC0izEzRsUxXEda5t4dnHw6UWZCvFBREbJV1vDy4oLzzUhCrrRkAVmbebyv6OZDdj6Ngz3z5PS96/DnHiGIuJV03rvHon10bMrNmUFVQBI0/sHhY3uneqa2sspmuRqCkHfzrA80V7yy5EViKRI= 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: --/CrZc0/PCTYKFxlL Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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. > 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. > 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. > > + /* > > + * We let userspace set GCSPR_EL0 to anything here, we will > > + * validate later in gcs_restore_signal(). > > + */ > > + current->thread.gcspr_el0 = gcspr; > > + write_sysreg_s(current->thread.gcspr_el0, SYS_GCSPR_EL0); > So in preserve_gcs_context(), we subtract 8 from the gcspr_el0 value. > Where is it added back? When we consumed the GCS cap token. > > + if (add_all || task_gcs_el0_enabled(current)) { > > + err = sigframe_alloc(user, &user->gcs_offset, > > + sizeof(struct gcs_context)); > > + if (err) > > + return err; > > + } > I'm still not entirely convinced of this conditional saving and the > interaction with unwinders. In a previous thread you mentioned that we > need to keep the GCSPR_EL0 sysreg value up to date even after disabling > GCS for a thread as not to confuse the unwinders. We could get a signal > delivered together with a sigreturn without any context switch. Do we > lose any state? > It might help if you describe the scenario, maybe even adding a comment > in the code, otherwise I'm sure we'll forget in a few months time. We should probably just change that back to saving unconditionally - it looks like the decision on worrying about overflowing the default signal frame is that we just shouldn't. --/CrZc0/PCTYKFxlL Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmbIY5MACgkQJNaLcl1U h9Bi+gf8CXq7dITB6mfDzRqVPcrdnIF0zDFjmHC8iIhqzkArOPeUrHuV8v1JvQKo xxY+T75lyZYVhNmi/4AnCvx2NQ+73+wmg5ZJyg3MMYv67UP5A4Gk8cOO3DP4gNZp GkinUu6g/33nKttu0Z8bNrylbmSVqqtFnh3Bzbv74LhYyKDe/dQ9CuCtZBEgjcQN U3x2p8TYyhcqu6/7OkT7MNJtI825nOVxVQh2B4Rlfp0ea+z2UNkTocgU12qsyZRt ew3ERI/4vdbAgPIIvqX5hmkLCEn6GNpyohGHm6LJ1BEZx3DGM9Ev6Q/ImtCMF+ty 5N4sxm/JaJbA6ExZyirRb8m7CyRISQ== =OryW -----END PGP SIGNATURE----- --/CrZc0/PCTYKFxlL--