| Date | Wed, 23 Jun 2021 10:45:04 +0200 | From | Borislav Petkov <> | Subject | Re: [patch V3 65/66] x86/fpu/signal: Handle #PF in the direct restore path |
| |
On Fri, Jun 18, 2021 at 04:19:28PM +0200, Thomas Gleixner wrote: > If *RSTOR raises an exception, then the slow path is taken. That's wrong > because if the reason was not #PF then going through the slow path is waste > of time because that will end up with the same conclusion that the data is > invalid. > > Now that the wrapper around *RSTOR return an negative error code, which is > the negated trap number, it's possible to differentiate. > > If the *RSTOR raised #PF then handle it directly in the fast path and if it > was some other exception, e.g. #GP, then give up and do not try the fast > path. > > This removes the legacy frame FRSTOR code from the slow path because FRSTOR > is not a ia32_fxstate frame and is therefore handled in the fast path. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/kernel/fpu/signal.c | 65 ++++++++++++++++++++----------------------- > 1 file changed, 31 insertions(+), 34 deletions(-) > > --- a/arch/x86/kernel/fpu/signal.c > +++ b/arch/x86/kernel/fpu/signal.c > @@ -270,11 +270,17 @@ static int restore_hwregs_from_user(void > } > } > > -static int restore_fpregs_from_user(void __user *buf, u64 xrestore, bool fx_only) > +/* > + * Attempt to restore the FPU registers directly from user memory. > + * Pagefaults are handled and any errors returned are fatal. > + */ > +static int restore_fpregs_from_user(void __user *buf, u64 xrestore, > + bool fx_only, unsigned int size) > { > struct fpu *fpu = ¤t->thread.fpu; > int ret ; > > +retry: > fpregs_lock(); > pagefault_disable(); > ret = restore_hwregs_from_user(buf, xrestore, fx_only); > @@ -291,15 +297,16 @@ static int restore_fpregs_from_user(void > * 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; > + > + if (ret == -EFAULT) > + ret = fault_in_pages_readable(buf, size); > + if (!ret) > + goto retry; > + return ret == -EFAULT ? ret : -EINVAL;
Uuuh, this is gonna make people stop and think, with all those different ret assignments and cases. :)
In any case, __fpu_restore_sig() is starting to become somewhat saner again, nice!
Reviewed-by: Borislav Petkov <bp@suse.de>
-- Regards/Gruss, Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
|