[PATCH v2 8/8] x86/fpu/xstate: Restore supervisor xstates for __fpu__restore_sig()

Yu-cheng Yu
2020-01-21T20:20:14Z

When returning from a signal, there is user state in a userspace memory
buffer and supervisor state in registers. The in-kernel buffer has neither
valid user or supervisor state. To restore both, save supervisor fpregs
first (and protect them across context switches), then restore it along
with user state.

This patch introduces saving and restoring of supervisor xstates for a
sigreturn in the following steps:

- Preserve supervisor register values by saving the whole fpu xstates.
Saving the whole is necessary because doing XSAVES with a partial
requested-feature bitmap (RFBM) changes xcomp_bv. Since user xstates are
to be restored from a user buffer, saved user xstates are discarded.

- Set TIF_NEED_FPU_LOAD, and do __fpu_invalidate_fpregs_state().
This prevents a context switch from corrupting the saved xstates,
and xstate is considered to be loaded again on return to userland.

- Under fpregs_lock(), restore user xstates (from the user buffer), and
then supervisor xstates (from previously saved content).

- When both parts of the restoration succeed, mark fpregs as valid.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
---
arch/x86/kernel/fpu/signal.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index e3781a4a52a8..0d3e06a772b0 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -327,14 +327,22 @@ 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
+ * Supervisor xstates are not modified by user space input, and
+ * need to be saved and restored. Save the whole because doing
+ * partial XSAVES changes xcomp_bv.
+ * 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).
*/
- set_thread_flag(TIF_NEED_FPU_LOAD);
+ fpregs_lock();
+ if (!test_thread_flag(TIF_NEED_FPU_LOAD)) {
+ if (xfeatures_mask_supervisor())
+ copy_xregs_to_kernel(&fpu->state.xsave);
+ set_thread_flag(TIF_NEED_FPU_LOAD);
+ }
__fpu_invalidate_fpregs_state(fpu);
+ fpregs_unlock();

if ((unsigned long)buf_fx % 64)
fx_only = 1;
@@ -360,6 +368,9 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
ret = copy_user_to_fpregs_zeroing(buf_fx, xfeatures_user, fx_only);
pagefault_enable();
if (!ret) {
+ if (xfeatures_mask_supervisor())
+ copy_kernel_to_xregs(&fpu->state.xsave,
+ xfeatures_mask_supervisor());
fpregs_mark_activate();
fpregs_unlock();
return 0;
@@ -389,7 +400,12 @@ 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);
- ret = copy_kernel_to_xregs_err(&fpu->state.xsave, xfeatures_user);
+ /*
+ * Restore previously saved supervisor xstates along with
+ * copied-in user xstates.
+ */
+ ret = copy_kernel_to_xregs_err(&fpu->state.xsave,
+ xfeatures_user | xfeatures_mask_supervisor());

} else if (use_fxsr()) {
ret = __copy_from_user(&fpu->state.fxsave, buf_fx, state_size);
--
2.21.0