linux-kernel - Re: [patch 08/41] x86/fpu: Restrict fpstate sanitizing to legacy components

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:   Fri, 11 Jun 2021 12:03:37 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>
Cc:     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>,
        Borislav Petkov <bp@...e.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Kan Liang <kan.liang@...ux.intel.com>
Subject: Re: [patch 08/41] x86/fpu: Restrict fpstate sanitizing to legacy
 components

On 6/11/21 9:15 AM, Thomas Gleixner wrote:
> xstateregs_get() does not longer use fpstate_sanitize_xstate() and the only

s/does not longer use/no longer uses/

		\
> --- a/arch/x86/kernel/fpu/regset.c
> +++ b/arch/x86/kernel/fpu/regset.c
> @@ -11,6 +11,39 @@
>  
>  #include <linux/sched/task_stack.h>
>  
> +/*
> + * When executing XSAVEOPT (or other optimized XSAVE instructions), if

The kernel doesn't use XSAVEOPT any more.  How about:

When executing XSAVES (or other optimized XSAVE instructions)

> + * a processor implementation detects that an FPU state component is still
> + * (or is again) in its initialized state, it may clear the corresponding
> + * bit in the header.xfeatures field, and can skip the writeout of registers
> + * to the corresponding memory layout.

Additionally, copy_xxx_to_xstate() may result in an xsave buffer with a
bit clear in xfeatures but the corresponding state region not containing
the state's init value.

> + *
> + * This means that when the bit is zero, the state component might still
> + * contain some previous - non-initialized register state.

Maybe say what the function does, e.g.:

This function fills in the init values for the X87 and SSE states if the
corresponding xfeatures bits are clear.

> + *
> + * This is required for the legacy regset functions.
> + */
> +static void fpstate_sanitize_legacy(struct fpu *fpu)
> +{
> +	struct fxregs_state *fx = &fpu->state.fxsave;
> +	u64 xfeatures;
> +
> +	if (!use_xsaveopt())
> +		return;

This is confusing, since we never use xsaveopt.  It's also wrong -- see
above.  How about just removing it?

> +
> +	xfeatures = fpu->state.xsave.header.xfeatures;
> +
> +	/* If FP is in init state, reinitialize it */
> +	if (!(xfeatures & XFEATURE_MASK_FP)) {
> +		memset(fx, 0, sizeof(*fx));
> +		fx->cwd = 0x37f;
> +	}
> +
> +	/* If SSE is in init state, clear the storage */
> +	if (!(xfeatures & XFEATURE_MASK_SSE))
> +		memset(fx->xmm_space, 0, sizeof(fx->xmm_space));
> +}
> +
>  

Does this result in the mxcsr_mask and mxcsr fields being correct?
There is a silly number of special cases there.