Messages in this thread | | | Date | Wed, 23 Jun 2021 10:10:42 +0200 | From | Borislav Petkov <> | Subject | Re: [patch V3 63/66] x86/fpu/signal: Split out the direct restore code |
| |
On Fri, Jun 18, 2021 at 04:19:26PM +0200, Thomas Gleixner wrote: > Prepare for smarter failure handling of the direct restore. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/kernel/fpu/signal.c | 110 +++++++++++++++++++++---------------------- > 1 file changed, 56 insertions(+), 54 deletions(-) > > --- a/arch/x86/kernel/fpu/signal.c > +++ b/arch/x86/kernel/fpu/signal.c > @@ -249,10 +249,7 @@ sanitize_restored_user_xstate(union fpre > } > } > > -/* > - * Restore the FPU state directly from the userspace signal frame. > - */ > -static int restore_fpregs_from_user(void __user *buf, u64 xrestore, bool fx_only) > +static int restore_hwregs_from_user(void __user *buf, u64 xrestore, bool fx_only)
Or simply
__restore_fpregs_from_user
to denote it is the low-level helper like the rest of the code does around here.
> { > if (use_xsave()) { > u64 init_bv = xfeatures_mask_uabi() & ~xrestore; > @@ -273,6 +270,56 @@ static int restore_fpregs_from_user(void > } > } > > +static int restore_fpregs_from_user(void __user *buf, u64 xrestore, bool fx_only) > +{ > + struct fpu *fpu = ¤t->thread.fpu; > + int ret ; ^
superfluous space.
> + fpregs_lock(); > + pagefault_disable(); > + ret = restore_hwregs_from_user(buf, xrestore, fx_only); > + pagefault_enable(); > + > + if (unlikely(ret)) { > + /* > + * The above did an FPU restore operation, restricted to > + * the user portion of the registers, and failed, but the > + * microcode might have modified the FPU registers > + * nevertheless. > + * > + * If the FPU registers do not belong to current, then > + * invalidate the FPU register state otherwise the task > + * might preempt current and return to user space with > + * corrupted FPU registers. > + * > + * In case current owns the FPU registers then no further > + * action is required. The fixup in the slow path will > + * handle it correctly. > + */ > + if (test_thread_flag(TIF_NEED_FPU_LOAD)) > + __cpu_invalidate_fpregs_state(); > + fpregs_unlock(); > + return ret; > + } > + > + /* > + * 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 -
Might wanna fix up the "We" brotherhood formulation, while at it. :)
> + * 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()) > + os_xrstor(&fpu->state.xsave, xfeatures_mask_supervisor()); > + > + fpregs_mark_activate(); > + fpregs_unlock(); > + return 0;
-- Regards/Gruss, Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
| |