linux-kernel - Re: [patch V3 63/66] x86/fpu/signal: Split out the direct restore code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YNLsgprAfL1Sy7uu@zn.tnic>
Date:   Wed, 23 Jun 2021 10:10:42 +0200
From:   Borislav Petkov <bp@...e.de>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Yu-cheng Yu <yu-cheng.yu@...el.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Kan Liang <kan.liang@...ux.intel.com>
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@...utronix.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 = &current->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