linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yu-cheng Yu <yu-cheng.yu@intel.com>
To: linux-kernel@vger.kernel.org, x86@kernel.org,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Tony Luck <tony.luck@intel.com>,
	Andy Lutomirski <luto@kernel.org>, Borislav Petkov <bp@alien8.de>,
	Rik van Riel <riel@surriel.com>,
	"Ravi V. Shankar" <ravi.v.shankar@intel.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Peter Zijlstra <peterz@infradead.org>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Subject: [PATCH v3 09/10] x86/fpu/xstate: Preserve supervisor states for slow path of __fpu__restore_sig()
Date: Mon, 11 May 2020 13:16:59 -0700	[thread overview]
Message-ID: <20200511201659.10192-1-yu-cheng.yu@intel.com> (raw)
In-Reply-To: <20200328164307.17497-10-yu-cheng.yu@intel.com>

The signal return code is responsible for taking an XSAVE buffer present
in user memory and loading it into the hardware registers.  This
operation only affects user XSAVE state and never affects supervisor state.

The fast path through this code simply points XRSTOR directly at the
user buffer.  However, since user memory is not guaranteed to be always
mapped, this XRSTOR can fail.  If it fails, the signal return code falls
back to a slow path which can tolerate page faults.

That slow path copies the xfeatures one by one out of the user buffer
into the task's fpu state area.  However, by being in a context where it
can handle page faults, the code can also schedule.  The lazy-fpu-load code
would think it has an up-to-date fpstate and would fail to save the
supervisor state when scheduling the task out.  When scheduling back in, it
would likely restore stale supervisor state.

To fix that, preserve supervisor state before the slow path.  Modify
copy_user_to_fpregs_zeroing() so that if it fails, fpregs are not zeroed,
and there is no need for fpregs_deactivate() and supervisor states are
preserved.

Move set_thread_flag(TIF_NEED_FPU_LOAD) to the slow path.  Without doing
this, the fast path also needs supervisor states to be saved first.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
 arch/x86/kernel/fpu/signal.c | 53 +++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index d09d72334a12..545ca4314096 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -262,19 +262,23 @@ sanitize_restored_user_xstate(union fpregs_state *state,
 static int copy_user_to_fpregs_zeroing(void __user *buf, u64 xbv, int fx_only)
 {
 	u64 init_bv;
+	int r;
 
 	if (use_xsave()) {
 		if (fx_only) {
 			init_bv = xfeatures_mask_user() & ~XFEATURE_MASK_FPSSE;
 
-			copy_kernel_to_xregs(&init_fpstate.xsave, init_bv);
-			return copy_user_to_fxregs(buf);
+			r = copy_user_to_fxregs(buf);
+			if (!r)
+				copy_kernel_to_xregs(&init_fpstate.xsave, init_bv);
+			return r;
 		} else {
 			init_bv = xfeatures_mask_user() & ~xbv;
 
-			if (unlikely(init_bv))
+			r = copy_user_to_xregs(buf, xbv);
+			if (!r && unlikely(init_bv))
 				copy_kernel_to_xregs(&init_fpstate.xsave, init_bv);
-			return copy_user_to_xregs(buf, xbv);
+			return r;
 		}
 	} else if (use_fxsr()) {
 		return copy_user_to_fxregs(buf);
@@ -327,28 +331,10 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 		}
 	}
 
-	/*
-	 * The current state of the FPU registers does not matter. By setting
-	 * TIF_NEED_FPU_LOAD unconditionally it is ensured that the our xstate
-	 * is not modified on context switch and that the xstate is considered
-	 * to be loaded again on return to userland (overriding last_cpu avoids
-	 * the optimisation).
-	 */
-	set_thread_flag(TIF_NEED_FPU_LOAD);
-	__fpu_invalidate_fpregs_state(fpu);
-
 	if ((unsigned long)buf_fx % 64)
 		fx_only = 1;
-	/*
-	 * For 32-bit frames with fxstate, copy the fxstate so it can be
-	 * reconstructed later.
-	 */
-	if (ia32_fxstate) {
-		ret = __copy_from_user(&env, buf, sizeof(env));
-		if (ret)
-			goto err_out;
-		envp = &env;
-	} else {
+
+	if (!ia32_fxstate) {
 		/*
 		 * Attempt to restore the FPU registers directly from user
 		 * memory. For that to succeed, the user access cannot cause
@@ -365,10 +351,27 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 			fpregs_unlock();
 			return 0;
 		}
-		fpregs_deactivate(fpu);
 		fpregs_unlock();
+	} else {
+		/*
+		 * For 32-bit frames with fxstate, copy the fxstate so it can
+		 * be reconstructed later.
+		 */
+		ret = __copy_from_user(&env, buf, sizeof(env));
+		if (ret)
+			goto err_out;
+		envp = &env;
 	}
 
+	/*
+	 * The current state of the FPU registers does not matter. By setting
+	 * TIF_NEED_FPU_LOAD unconditionally it is ensured that the our xstate
+	 * is not modified on context switch and that the xstate is considered
+	 * to be loaded again on return to userland (overriding last_cpu avoids
+	 * the optimisation).
+	 */
+	set_thread_flag(TIF_NEED_FPU_LOAD);
+	__fpu_invalidate_fpregs_state(fpu);
 
 	if (use_xsave() && !fx_only) {
 		u64 init_bv = xfeatures_mask_user() & ~user_xfeatures;
-- 
2.21.0


  parent reply	other threads:[~2020-05-11 20:17 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-28 16:42 [PATCH v3 00/10] Support XSAVES supervisor states Yu-cheng Yu
2020-03-28 16:42 ` [PATCH v3 01/10] x86/fpu/xstate: Rename validate_xstate_header() to validate_user_xstate_header() Yu-cheng Yu
2020-04-28 17:11   ` Borislav Petkov
2020-04-28 17:15     ` Yu-cheng Yu
2020-03-28 16:42 ` [PATCH v3 02/10] x86/fpu/xstate: Define new macros for supervisor and user xstates Yu-cheng Yu
2020-03-28 16:43 ` [PATCH v3 03/10] x86/fpu/xstate: Separate user and supervisor xfeatures mask Yu-cheng Yu
2020-03-28 16:43 ` [PATCH v3 04/10] x86/fpu/xstate: Introduce XSAVES supervisor states Yu-cheng Yu
2020-03-28 16:43 ` [PATCH v3 05/10] x86/fpu/xstate: Define new functions for clearing fpregs and xstates Yu-cheng Yu
2020-04-29  9:27   ` Borislav Petkov
2020-04-29 16:09     ` Yu-cheng Yu
2020-04-29 16:06   ` Yu-cheng Yu
2020-04-29 16:39     ` Borislav Petkov
2020-04-29 17:02       ` Yu-cheng Yu
2020-04-29 17:32         ` Borislav Petkov
2020-04-29 17:42   ` Yu-cheng Yu
2020-04-29 19:10   ` Yu-cheng Yu
2020-03-28 16:43 ` [PATCH v3 06/10] x86/fpu/xstate: Update sanitize_restored_xstate() for supervisor xstates Yu-cheng Yu
2020-05-07 13:11   ` Borislav Petkov
2020-05-07 15:55   ` Yu-cheng Yu
2020-03-28 16:43 ` [PATCH v3 07/10] x86/fpu/xstate: Update copy_kernel_to_xregs_err() for XSAVES supervisor states Yu-cheng Yu
2020-05-07 13:28   ` Borislav Petkov
2020-05-07 15:58   ` Yu-cheng Yu
2020-03-28 16:43 ` [PATCH v3 08/10] x86/fpu: Introduce copy_supervisor_to_kernel() Yu-cheng Yu
2020-03-28 16:43 ` [PATCH v3 09/10] x86/fpu/xstate: Preserve supervisor states for slow path of __fpu__restore_sig() Yu-cheng Yu
2020-05-10  8:46   ` Borislav Petkov
2020-05-11 20:16   ` Yu-cheng Yu [this message]
2020-03-28 16:43 ` [PATCH v3 10/10] x86/fpu/xstate: Restore supervisor states for signal return Yu-cheng Yu
2020-05-10  8:49   ` Borislav Petkov
2020-05-11 20:20   ` Yu-cheng Yu
2020-05-11 20:27 ` [PATCH v3 00/10] Support XSAVES supervisor states Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200511201659.10192-1-yu-cheng.yu@intel.com \
    --to=yu-cheng.yu@intel.com \
    --cc=bigeasy@linutronix.de \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=ravi.v.shankar@intel.com \
    --cc=riel@surriel.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).