From patchwork Thu Oct 24 11:06:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Brown X-Patchwork-Id: 13848797 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 658C9CE8E60 for ; Thu, 24 Oct 2024 11:18:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Cc:To:Message-Id: Content-Transfer-Encoding:Content-Type:MIME-Version:Subject:Date:From: Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender :Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Owner; bh=CbvErAglLNwq0XXtCF2MODZK1/Oqp0dLxoHy2paTXK0=; b=oiX07lgvW6nj1AydtqLxQrqxcm uQNCYNX1X0EYFc/S3EkET8I07d6qoxgldKcDERQCr0Hxu6zD+vs82LvAqdohaVSEDxs+7r/5AFTWb p7yJfVN9qzuvfcIt514th+F2qDp7dr//WpQeb7wBv9e8feKeSjvBNAgCvwQ4stn/ZCETdExTdHf4N uCMCukY7VaoKEK5+8UIMHpJQQ0Hd4dEnSn731NIuS/ULsXElqvbjMUBpx3DB3aCHJ4H3JeOZXut2G 9l+fW/WE5qUvMkH9NzVQVtqAn6hQ2/wNIIIwKSHmVwJvjUciESwYhq187t0M+NY4WHTTG3zhNCtrY Ng97aIfQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t3vrD-00000000BsA-3Eq0; Thu, 24 Oct 2024 11:18:23 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t3vh2-00000000ASN-48cL for linux-arm-kernel@lists.infradead.org; Thu, 24 Oct 2024 11:07:54 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id C4E925C5FAD; Thu, 24 Oct 2024 11:07:47 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 98707C4CEC7; Thu, 24 Oct 2024 11:07:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1729768072; bh=3u4MB5fzt4B4URpnmS1cFnecFjFi0U4J8+oHzjRXNFc=; h=From:Date:Subject:To:Cc:From; b=cqNmRswks3sQEtF9sXALrcrZ8xOPReinoA9Jl/lcbNSxGECx5gJ5hmCNQ7HYCHawW gXz0oJ7rmrKm+sP0HGor9TOMT5iigGaVbMsIk5lN5lbdTBCBt/2ORk3vxZFAcg88Se +v4/3oOUdpjT05Hn0vJWxAwEA4yeuQGN1BCAfPXAJDGXvyjM04aRe10/RywsccbDEV qkxCZVlIrnoQQGaL1z8HiBVLrdOKwhQmGFSCYdOzWTUZN8LwNgA7y4lcOvkKaNfu5Q NTXpod3T9SC4A8jT0LygRmI0CbT3IP/a2dhT1Hlhl2IgZ+kbx1IFMGbZ/2Y53l+RA/ G3Eh2/KnZM08g== From: Mark Brown Date: Thu, 24 Oct 2024 12:06:42 +0100 Subject: [PATCH] arm64/signal: Consistently invalidate the in register FP state in restore MIME-Version: 1.0 Message-Id: <20241024-arm64-fp-sig-flush-v1-1-05148eb92a84@kernel.org> X-B4-Tracking: v=1; b=H4sIAEIqGmcC/x3MTQqAIBBA4avErBtIs6CuEi1MRxvoD4ciiO6et PwW7z0glJgE+uKBRBcL71uGKgtws90iIfts0JU2qtI12rS2BsOBwhHDcsqMZJR3nWvsRB3k8Eg U+P6nw/i+H+pSWTBkAAAA X-Change-ID: 20241023-arm64-fp-sig-flush-e41dc9c5abe9 To: Catalin Marinas , Will Deacon Cc: Mark Rutland , Dave Martin , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Mark Brown X-Mailer: b4 0.15-dev-9b746 X-Developer-Signature: v=1; a=openpgp-sha256; l=6416; i=broonie@kernel.org; h=from:subject:message-id; bh=3u4MB5fzt4B4URpnmS1cFnecFjFi0U4J8+oHzjRXNFc=; b=owEBbQGS/pANAwAKASTWi3JdVIfQAcsmYgBnGiqFEOxzTMkfhPBnTWTIFQZyU/t2zA4Qi1/mdz4D kSuonjGJATMEAAEKAB0WIQSt5miqZ1cYtZ/in+ok1otyXVSH0AUCZxoqhQAKCRAk1otyXVSH0JL3B/ 0Z9vAnd4lxDbFQQWjIPda2uhIWErXyDn5gl+tMeU8RnUgU1AqsmGYG8DHcTO/TUNhgkD3d0yYWpm0v j95tQP4R3MtezAEW7LreC2XSh9svhyTBjNHByXAcJ6tmqY9RJckOsXlobvvDpGtmbcPzkCBPRQzdk3 SmLG/Gj7W9k9BanzEg0q71PN/jToajUy4I7hvfxN0o4rbh9nBF6ddOQmIH/5XFrm5EJjMzfpL2BfCI ame4rVqvNvBclRnC2NU8hJcSMhT6q1hoGYGIOxSqI+nzsjsCFkhBKfPJoMepge9xpttwlifNCeFsr3 Qpio13QGlRSSLN1I0pehGGL9OsVB/v X-Developer-Key: i=broonie@kernel.org; a=openpgp; fpr=3F2568AAC26998F9E813A1C5C3F436CA30F5D8EB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241024_040753_153883_53DA578B X-CRM114-Status: GOOD ( 19.42 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org When restoring the SVE and SME specific floating point register states we flush the task floating point state, marking the hardware state as stale so that preemption does not result in us saving register state from the signal handler on top of the restored context and forcing a reload from memory. For the plain FPSIMD state we don't do this, we just copy the state from userspace and then force an immediate reload of the register state. This isn't racy against context switch since we copy the incoming data onto the stack rather than directly into the task struct but it's still messy and inconsistent. Simplify things and avoid a potential source of error by moving the invalidation of the CPU state to the main restore_sigframe() and reworking the restore of the FPSIMD state to update the task struct and rely on loading as part of the general do_notify_resume() handling for return to user like we do for the SVE and SME state. Signed-off-by: Mark Brown --- arch/arm64/kernel/signal.c | 70 +++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 48 deletions(-) --- base-commit: 8e929cb546ee42c9a61d24fae60605e9e3192354 change-id: 20241023-arm64-fp-sig-flush-e41dc9c5abe9 Best regards, diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c index 5619869475304776fc005fe24a385bf86bfdd253..a536d65b1f11957fa0f84094df542175d050e00a 100644 --- a/arch/arm64/kernel/signal.c +++ b/arch/arm64/kernel/signal.c @@ -210,7 +210,7 @@ static int preserve_fpsimd_context(struct fpsimd_context __user *ctx) static int restore_fpsimd_context(struct user_ctxs *user) { - struct user_fpsimd_state fpsimd; + struct user_fpsimd_state *fpsimd = ¤t->thread.uw.fpsimd_state; int err = 0; /* check the size information */ @@ -218,18 +218,14 @@ static int restore_fpsimd_context(struct user_ctxs *user) return -EINVAL; /* copy the FP and status/control registers */ - err = __copy_from_user(fpsimd.vregs, &(user->fpsimd->vregs), - sizeof(fpsimd.vregs)); - __get_user_error(fpsimd.fpsr, &(user->fpsimd->fpsr), err); - __get_user_error(fpsimd.fpcr, &(user->fpsimd->fpcr), err); + err = __copy_from_user(fpsimd->vregs, &(user->fpsimd->vregs), + sizeof(fpsimd->vregs)); + __get_user_error(fpsimd->fpsr, &(user->fpsimd->fpsr), err); + __get_user_error(fpsimd->fpcr, &(user->fpsimd->fpcr), err); clear_thread_flag(TIF_SVE); current->thread.fp_type = FP_STATE_FPSIMD; - /* load the hardware registers from the fpsimd_state structure */ - if (!err) - fpsimd_update_current_state(&fpsimd); - return err ? -EFAULT : 0; } @@ -333,7 +329,7 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user) { int err = 0; unsigned int vl, vq; - struct user_fpsimd_state fpsimd; + struct user_fpsimd_state *fpsimd = ¤t->thread.uw.fpsimd_state; u16 user_vl, flags; if (user->sve_size < sizeof(*user->sve)) @@ -376,16 +372,6 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user) if (user->sve_size < SVE_SIG_CONTEXT_SIZE(vq)) return -EINVAL; - /* - * Careful: we are about __copy_from_user() directly into - * thread.sve_state with preemption enabled, so protection is - * needed to prevent a racing context switch from writing stale - * registers back over the new data. - */ - - fpsimd_flush_task_state(current); - /* From now, fpsimd_thread_switch() won't touch thread.sve_state */ - sve_alloc(current, true); if (!current->thread.sve_state) { clear_thread_flag(TIF_SVE); @@ -408,14 +394,10 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user) fpsimd_only: /* copy the FP and status/control registers */ /* restore_sigframe() already checked that user->fpsimd != NULL. */ - err = __copy_from_user(fpsimd.vregs, user->fpsimd->vregs, - sizeof(fpsimd.vregs)); - __get_user_error(fpsimd.fpsr, &user->fpsimd->fpsr, err); - __get_user_error(fpsimd.fpcr, &user->fpsimd->fpcr, err); - - /* load the hardware registers from the fpsimd_state structure */ - if (!err) - fpsimd_update_current_state(&fpsimd); + err = __copy_from_user(fpsimd->vregs, user->fpsimd->vregs, + sizeof(fpsimd->vregs)); + __get_user_error(fpsimd->fpsr, &user->fpsimd->fpsr, err); + __get_user_error(fpsimd->fpcr, &user->fpsimd->fpcr, err); return err ? -EFAULT : 0; } @@ -524,16 +506,6 @@ static int restore_za_context(struct user_ctxs *user) if (user->za_size < ZA_SIG_CONTEXT_SIZE(vq)) return -EINVAL; - /* - * Careful: we are about __copy_from_user() directly into - * thread.sme_state with preemption enabled, so protection is - * needed to prevent a racing context switch from writing stale - * registers back over the new data. - */ - - fpsimd_flush_task_state(current); - /* From now, fpsimd_thread_switch() won't touch thread.sve_state */ - sme_alloc(current, true); if (!current->thread.sme_state) { current->thread.svcr &= ~SVCR_ZA_MASK; @@ -601,16 +573,6 @@ static int restore_zt_context(struct user_ctxs *user) if (nregs != 1) return -EINVAL; - /* - * Careful: we are about __copy_from_user() directly into - * thread.zt_state with preemption enabled, so protection is - * needed to prevent a racing context switch from writing stale - * registers back over the new data. - */ - - fpsimd_flush_task_state(current); - /* From now, fpsimd_thread_switch() won't touch ZT in thread state */ - err = __copy_from_user(thread_zt_state(¤t->thread), (char __user const *)user->zt + ZT_SIG_REGS_OFFSET, @@ -876,6 +838,18 @@ static int restore_sigframe(struct pt_regs *regs, if (err == 0) err = parse_user_sigframe(&user, sf); + /* + * Careful: we are about __copy_from_user() directly into + * thread floating point state with preemption enabled, so + * protection is needed to prevent a racing context switch + * from writing stale registers back over the new data. Mark + * the register floating point state as invalid and unbind the + * task from the CPU to force a reload before we return to + * userspace. fpsimd_flush_task_state() has a check for FP + * support. + */ + fpsimd_flush_task_state(current); + if (err == 0 && system_supports_fpsimd()) { if (!user.fpsimd) return -EINVAL;