linux-kernel - Re: [PATCH v10 06/28] x86/fpu/xstate: Add new variables to indicate dynamic XSTATE buffer size

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]

Message-ID: <878rzc6fbs.ffs@tglx>
Date:   Fri, 01 Oct 2021 15:32:23 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Chang S. Bae" <chang.seok.bae@...el.com>, bp@...e.de,
        luto@...nel.org, mingo@...nel.org, x86@...nel.org
Cc:     len.brown@...el.com, lenb@...nel.org, dave.hansen@...el.com,
        thiago.macieira@...el.com, jing2.liu@...el.com,
        ravi.v.shankar@...el.com, linux-kernel@...r.kernel.org,
        chang.seok.bae@...el.com, kvm@...r.kernel.org
Subject: Re: [PATCH v10 06/28] x86/fpu/xstate: Add new variables to indicate
 dynamic XSTATE buffer size

On Wed, Aug 25 2021 at 08:53, Chang S. Bae wrote:
> +/**
> + * struct fpu_xstate_buffer_config - xstate buffer configuration
> + * @max_size:			The CPUID-enumerated all-feature "maximum" size
> + *				for xstate per-task buffer.
> + * @min_size:			The size to fit into the statically-allocated
> + *				buffer. With dynamic states, this buffer no longer
> + *				contains all the enabled state components.
> + * @user_size:			The size of user-space buffer for signal and
> + *				ptrace frames, in the non-compacted format.
> + */

>  void fpstate_init(struct fpu *fpu)
>  {
>  	union fpregs_state *state;
> +	unsigned int size;
> +	u64 mask;
>  
> -	if (likely(fpu))
> +	if (likely(fpu)) {
>  		state = &fpu->state;
> -	else
> +		/* The dynamic user states are not prepared yet. */
> +		mask = xfeatures_mask_all & ~xfeatures_mask_user_dynamic;

The patch ordering is really odd. Why aren't you adding

     fpu->state_mask
and
     fpu->state_size

first and initialize state_mask and state_size to the fixed mode and
then add the dynamic sizing on top?

>  	/*
>  	 * If the target FPU state is not resident in the CPU registers, just
>  	 * memcpy() from current, else save CPU state directly to the target.
> +	 *
> +	 * KVM does not support dynamic user states yet. Assume the buffer
> +	 * always has the minimum size.
>  	 */
>  	if (test_thread_flag(TIF_NEED_FPU_LOAD))
>  		memcpy(&fpu->state, &current->thread.fpu.state,
> -		       fpu_kernel_xstate_size);
> +		       fpu_buf_cfg.min_size);

Which completely avoids the export of fpu_buf_cfg for KVM because the
information is just available via struc fpu. As a bonus the export of
fpu_kernel_xstate_size can be removed as well.

Hmm?

Thanks,

        tglx