linux-kernel - Re: [PATCH 16/22] x86/fpu: Always store the registers in copy_fpstate_to

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 30 Jan 2019 12:43:22 +0100
From:   Borislav Petkov <bp@...en8.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 16/22] x86/fpu: Always store the registers in
 copy_fpstate_to_sigframe()

On Wed, Jan 09, 2019 at 12:47:38PM +0100, Sebastian Andrzej Siewior wrote:
> From: Rik van Riel <riel@...riel.com>
> 
> copy_fpstate_to_sigframe() stores the registers directly to user space.
> This is okay because the FPU register are valid and saving it directly
> avoids saving it into kernel memory and making a copy.
> However… We can't keep doing this if we are going to restore the FPU
> registers on the return to userland. It is possible that the FPU
> registers will be invalidated in the middle of the save operation and
> this should be done with disabled preemption / BH.
> 
> Save the FPU registers to task's FPU struct and copy them to the user
> memory later on.
> 
> This code is extracted from an earlier version of the patchset while
> there still was lazy-FPU on x86.
> 
> Signed-off-by: Rik van Riel <riel@...riel.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> ---
>  arch/x86/include/asm/fpu/internal.h | 45 -----------------------------
>  arch/x86/kernel/fpu/signal.c        | 29 +++++++------------
>  2 files changed, 10 insertions(+), 64 deletions(-)

...

> @@ -171,9 +156,15 @@ 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;
>  
> -	/* Save the live register state to the user directly. */
> -	if (copy_fpregs_to_sigframe(buf_fx))
> -		return -1;
> +	copy_fpregs_to_fpstate(fpu);
> +
> +	if (using_compacted_format()) {
> +		copy_xstate_to_user(buf_fx, xsave, 0, size);
> +	} else {
> +		fpstate_sanitize_xstate(fpu);
> +		if (__copy_to_user(buf_fx, xsave, fpu_user_xstate_size))
> +			return -1;
> +	}
>  
>  	/* Save the fsave header for the 32-bit frames. */
>  	if ((ia32_fxstate || !use_fxsr()) && save_fsave_header(tsk, buf))

Comments above that function need updating.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.