On Sat, Mar 28, 2020 at 09:43:07AM -0700, Yu-cheng Yu wrote:
> v3:
> - Change copy_xregs_to_kernel() to copy_supervisor_to_kernel(), which is
> introduced in a previous patch.
> - Update commit log.
>
> Signed-off-by: Yu-cheng Yu <[email protected]>
> Reviewed-by: Dave Hansen <[email protected]>
> ---
> arch/x86/kernel/fpu/signal.c | 27 ++++++++++++++++++++++-----
> 1 file changed, 22 insertions(+), 5 deletions(-)
Took Sebastian and me a while to grok the whole situation so pls add the
comments below to that patch when sending it in your next revision.
Thx.
---
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 4dad5afc938d..06e88d6ebb07 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -347,7 +347,19 @@ static int __fpu__restore_sig(void __user *buf, void
__user *buf_fx, int size)
ret = copy_user_to_fpregs_zeroing(buf_fx, user_xfeatures,
fx_only);
pagefault_enable();
if (!ret) {
- /* Restore supervisor states */
+
+ /*
+ * Restore supervisor states: previous context switch
+ * etc has done XSAVES and saved the supervisor states
+ * in the kernel buffer from which they can be restored
+ * now.
+ *
+ * We cannot do a single XRSTORS here - which would
+ * be nice - because the rest of the FPU registers are
+ * being restored from a user buffer directly. The
+ * single XRSTORS happens below, when the user buffer
+ * has been copied to the kernel one.
+ */
if (test_thread_flag(TIF_NEED_FPU_LOAD) &&
xfeatures_mask_supervisor())
copy_kernel_to_xregs(&fpu->state.xsave,
@@ -369,15 +381,19 @@ static int __fpu__restore_sig(void __user *buf, void
__user *buf_fx, int size)
}
/*
- * Supervisor states are not modified by user space input. Save
- * current supervisor states first.
* By setting TIF_NEED_FPU_LOAD it is ensured that 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).
*/
fpregs_lock();
+
if (!test_thread_flag(TIF_NEED_FPU_LOAD)) {
+
+ /*
+ * Supervisor states are not modified by user space input. Save
+ * current supervisor states first and invalidate the FPU regs.
+ */
if (xfeatures_mask_supervisor())
copy_supervisor_to_kernel(&fpu->state.xsave);
set_thread_flag(TIF_NEED_FPU_LOAD);
@@ -405,6 +421,7 @@ static int __fpu__restore_sig(void __user *buf, void __user
*buf_fx, int size)
fpregs_lock();
if (unlikely(init_bv))
copy_kernel_to_xregs(&init_fpstate.xsave, init_bv);
+
/*
* Restore previously saved supervisor xstates along with
* copied-in user xstates.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
