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 05CEDC52D7C for ; Fri, 23 Aug 2024 09:37:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 57E6680085; Fri, 23 Aug 2024 05:37:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 52E3080084; Fri, 23 Aug 2024 05:37:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 35CE780085; Fri, 23 Aug 2024 05:37:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 12A1080084 for ; Fri, 23 Aug 2024 05:37:31 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 91BA4A81FB for ; Fri, 23 Aug 2024 09:37:30 +0000 (UTC) X-FDA: 82483007460.24.37B4AE9 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf12.hostedemail.com (Postfix) with ESMTP id D38D140005 for ; Fri, 23 Aug 2024 09:37:28 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf12.hostedemail.com: domain of cmarinas@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=cmarinas@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724405757; 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=y9Fu4lARUpFeVhjfPVviJyJzQZXAZYTBVXGJEHVJJ50=; b=QYmJTG1svXDg7ubhzPr/U/gFK73arWqtbdKoQADHNU/4462w6fEKEs6i98BUGvHjeO2AL8 DP7Y7M9u5gzv4dX338Cb4rs8qZmBXwZFpS6R1eEzofILkiisYzfIDRsUHANVTKNW0jcb9F sDeRgnSjt0zjE0FNlFutJAeKAor653Y= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724405757; a=rsa-sha256; cv=none; b=3LL84X6803MrK7aFznTWLSPIwhOcwHhzWbQoXib0II/EOzbajG8d2uIhENnooaLbOZHnaU n9l8JMZi2BTEtkZUqrfmyPSumP+xeSYWRBM764iZAgC9h+Ga0xwdD4hNznBbbMtJP3MF1w cpXNPkc5vd1unznXxEhxl1qbaAdM+as= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf12.hostedemail.com: domain of cmarinas@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=cmarinas@kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id AED7C60F59; Fri, 23 Aug 2024 09:37:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C2810C32786; Fri, 23 Aug 2024 09:37:21 +0000 (UTC) Date: Fri, 23 Aug 2024 10:37:19 +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: <20240822-arm64-gcs-v11-25-41b81947ecb5@kernel.org> X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: D38D140005 X-Stat-Signature: 3ooyhbez3fk77zkfopixr9e39rsbwga8 X-Rspam-User: X-HE-Tag: 1724405848-542311 X-HE-Meta: U2FsdGVkX19o6+/Fdv0RuDpe4h50rhYb70nVhSGW4FSbaRhx01KiNWbn2Aw3FBBP3FN4oQsRNXvzNdHkx3dmey38zShWO3m+4xKMpb0bBejZAGK/mXjpwrCpWxuETwtY9JzRFaVgnFNz5UpBVXBSNZWWRVMO2UtXnetAnRPC2G6j3K+9tRZKCFEGO6ID6+eQeLUo72leprLUHGJte5PZiG6k+QTBvCx7HoAM0ghtb+tGPygu/hJHIhK3XBJrHxwRPHhYrq4A78UIPhZGHCYSxroq7EuJnbsvamqYIVXcYxXOuVLo4smwIkGRXWC4mgHhqgR1HU4+8MCnwjziLdEXstj09C0RLpXeMKN/qZ0WymbJ53wFR7ixtreg8LftBTR4pw97oT1XpS3nACbkXuHrPlXms8mbMJ3ncME/F5CWkVFgW5yhv0B+EiRwUYZW0yibe8BPBmiC7SJ2+ABsx7Im8NwvOKsDhwiHU+JOWfYgr2BNnExtF9XNws2ZNoZvG2O8Pf+aOiiEljwy3gzpzWQii6xBWu5JvzaREJL1lLcMKxJYq2RwiD6TFZ46jXGNCjp5c16sUS8X6306mqfTbGQFfFYKMTipbhIs8nkuUX3ZHRJKOSaVtGiNTqDYT8VrZtIY1+M69CXqOg4TyPRN0BnFbyECD0PAiCV3bmIfaamxDhcZjiyV+U596bS1ubTC1cPEcaHXp8c2BBSuEzG/UouGnh/hqV8NdQ4hW/wLuTmM7CNZ9WrFk6cGPlqb7ovnWm3M3+bvqC0Mnh237BJrcx0fmTxLPV2yyRHpA43mOruUqAodX+pzGy4PTvtjE8o23GQxTKD2TvCNq9E6UFQAo8FaF/CZ+ZHLzcKzvY0LM/PCHGPqe2O+UXeyCVNWZs9u5rJLOFB5YDVrUleiDwNkGVjs7V59u8Dxmb8IuQUYC+0MEdfrNn4ZpBNGu2K7gZYhGXsoOoVMrrLZclTDUYbgN1I S2q6md84 Eg38ugbPgkI/oN792BYOp9V5ljLpXtZgjH+f9cYj3tIKwIntlajtDIx3bnBC+i0BzJDcGHHzyux5wgFmc6Lr2KZcxECQrqZSollRHksElVoGzXSc1pkFtSHaFPX86CzP5QfwBztkFQipD+zTZzZkjUZGp9CiJ2+CsvgvrNmQPD9u8jqqqnrXcXllw0PA3OBeNLq5l3RsQss6DaocXeGlmjnMURT5sNcDQbNBUC9pIeDfN0FIzg8tAko9rKHYRjs+qX8nFiyNDCrDMXsUYEaIT/EKdGnkCu+JdQw3MAgi7ecMvQChGm6mNSaIBAt88cmkD83AoNveG2G4M/RFp/aA1mCclffpZhL0mLSdByqrkpEaRi7cWDVGcm4tBKGrEv+utfVr627n5+zj66RUOoShp5Fpc2pYYiqMFRK4uK6XqrNeQnxA= 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 Thu, Aug 22, 2024 at 02:15:28AM +0100, Mark Brown wrote: > +static int preserve_gcs_context(struct gcs_context __user *ctx) > +{ > + int err = 0; > + u64 gcspr; > + > + /* > + * We will add a cap token to the frame, include it in the > + * GCSPR_EL0 we report to support stack switching via > + * sigreturn. > + */ > + gcs_preserve_current_state(); > + gcspr = current->thread.gcspr_el0 - 8; > + > + __put_user_error(GCS_MAGIC, &ctx->head.magic, err); > + __put_user_error(sizeof(*ctx), &ctx->head.size, err); > + __put_user_error(gcspr, &ctx->gcspr, err); > + __put_user_error(0, &ctx->reserved, err); > + __put_user_error(current->thread.gcs_el0_mode, > + &ctx->features_enabled, err); > + > + return 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. Also in gcs_signal_entry() in the previous patch, we seem to subtract 16 rather than 8. I admit I haven't checked the past discussions in this area, so maybe I'm missing something. > +static int restore_gcs_context(struct user_ctxs *user) > +{ > + u64 gcspr, enabled; > + int err = 0; > + > + if (user->gcs_size != sizeof(*user->gcs)) > + return -EINVAL; > + > + __get_user_error(gcspr, &user->gcs->gcspr, err); > + __get_user_error(enabled, &user->gcs->features_enabled, err); > + if (err) > + return err; > + > + /* Don't allow unknown modes */ > + if (enabled & ~PR_SHADOW_STACK_SUPPORTED_STATUS_MASK) > + return -EINVAL; > + > + err = gcs_check_locked(current, enabled); > + if (err != 0) > + return err; > + > + /* Don't allow enabling */ > + if (!task_gcs_el0_enabled(current) && > + (enabled & PR_SHADOW_STACK_ENABLE)) > + return -EINVAL; > + > + /* If we are disabling disable everything */ > + if (!(enabled & PR_SHADOW_STACK_ENABLE)) > + enabled = 0; > + > + current->thread.gcs_el0_mode = enabled; > + > + /* > + * 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? 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. > @@ -977,6 +1079,13 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user, > return err; > } > > + 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. -- Catalin