lkml.org 
[lkml]   [2021]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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 = &current->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

\
 
 \ /
  Last update: 2021-06-23 10:45    [W:1.018 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site