linux-kernel - Re: [PATCH v6 06/25] x86/fpu/xstate: Opt-in kernel dynamic bits when calculate guest xstate 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]

Date:   Thu, 14 Sep 2023 10:40:55 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Yang Weijiang <weijiang.yang@...el.com>, seanjc@...gle.com,
        pbonzini@...hat.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     peterz@...radead.org, chao.gao@...el.com,
        rick.p.edgecombe@...el.com, john.allen@....com
Subject: Re: [PATCH v6 06/25] x86/fpu/xstate: Opt-in kernel dynamic bits when
 calculate guest xstate size

On 9/13/23 23:33, Yang Weijiang wrote:
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -1636,9 +1636,17 @@ static int __xstate_request_perm(u64 permitted, u64 requested, bool guest)
>  
>  	/* Calculate the resulting kernel state size */
>  	mask = permitted | requested;
> -	/* Take supervisor states into account on the host */
> +	/*
> +	 * Take supervisor states into account on the host. And add
> +	 * kernel dynamic xfeatures to guest since guest kernel may
> +	 * enable corresponding CPU feaures and the xstate registers
> +	 * need to be saved/restored properly.
> +	 */
>  	if (!guest)
>  		mask |= xfeatures_mask_supervisor();
> +	else
> +		mask |= fpu_kernel_dynamic_xfeatures;
> +
>  	ksize = xstate_calculate_size(mask, compacted);

Heh, you changed the "guest" naming in "fpu_kernel_dynamic_xfeatures"
but didn't change the logic.

As it's coded at the moment *ALL* "fpu_kernel_dynamic_xfeatures" are
guest xfeatures.  So, they're different in name only.

If you want to change the rules for guests, we have *ONE* place that's
done: fpstate_reset().  It establishes the permissions and the sizes for
the default guest FPU.  Start there.  If you want to make the guest
defaults include XFEATURE_CET_USER, then you need to put the bit in *there*.

The other option is to have the KVM code actually go and "request" that
the dynamic states get added to 'fpu->guest_perm'.  Would there ever be
any reason for KVM to be on a system which supports a dynamic kernel
feature but where it doesn't get enabled for guest use, or at least
shouldn't have the FPU space allocated?