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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 31 Mar 2019 20:20:25 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Andy Lutomirski <luto@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        kvm@...r.kernel.org, "Jason A. Donenfeld" <Jason@...c4.com>,
        Rik van Riel <riel@...riel.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH 18/24] x86/fpu: Prepare copy_fpstate_to_sigframe() for
 TIF_NEED_FPU_LOAD

On Thu, 21 Mar 2019, Sebastian Andrzej Siewior wrote:

> From: Rik van Riel <riel@...riel.com>
> 
> The FPU registers need only to be saved if TIF_NEED_FPU_LOAD is not set.
> Otherwise this has been already done and can be skipped.
> 
> Signed-off-by: Rik van Riel <riel@...riel.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> ---
>  arch/x86/kernel/fpu/signal.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> index f55f16d9e7e4e..97ea6909ede1f 100644
> --- a/arch/x86/kernel/fpu/signal.c
> +++ b/arch/x86/kernel/fpu/signal.c
> @@ -155,7 +155,16 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
>  			sizeof(struct user_i387_ia32_struct), NULL,
>  			(struct _fpstate_32 __user *) buf) ? -1 : 1;
>  
> -	copy_fpregs_to_fpstate(fpu);
> +	fpregs_lock();
> +	/*
> +	 * If we do not need to load the FPU registers at return to userspace
> +	 * then the CPU has the current state and we need to save it. Otherwise
> +	 * it is already done and we can skip it.
> +	 */
> +	if (!test_thread_flag(TIF_NEED_FPU_LOAD))
> +		copy_fpregs_to_fpstate(fpu);

I think this should do the following:

	fpregs_lock();
	if (!test_thread_flag(TIF_NEED_FPU_LOAD)) {
		pagefault_disable();
		ret = copy_fpu_to_user(...);
		pagefault_enable();
		if (!res)
			return 0;
		copy_fpregs_to_fpstate(fpu); 
	}
	fpregs_unlock();

The point is that in most cases the direct store from the FPU registers to
user space will succeed simply because the stack is accessible and you only
do the store in kernel memory and copy when that fails.

Thanks,

	tglx


Powered by blists - more mailing lists