linux-kernel - Re: [PATCH 7/9] x86/sev-es: Include XSS value in GHCB CPUID request

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: <20231012125924.GFZSftrGx43ALVCtfS@fat_crate.local>
Date:   Thu, 12 Oct 2023 14:59:24 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     John Allen <john.allen@....com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        pbonzini@...hat.com, weijiang.yang@...el.com,
        rick.p.edgecombe@...el.com, seanjc@...gle.com, x86@...nel.org,
        thomas.lendacky@....com
Subject: Re: [PATCH 7/9] x86/sev-es: Include XSS value in GHCB CPUID request

On Tue, Oct 10, 2023 at 08:02:18PM +0000, John Allen wrote:
> When a guest issues a cpuid instruction for Fn0000000D_x0B (CetUserOffset), the
> hypervisor may intercept and access the guest XSS value. For SEV-ES, this is
> encrypted and needs to be included in the GHCB to be visible to the hypervisor.
> The rdmsr instruction needs to be called directly as the code may be used in
> early boot in which case the rdmsr wrappers should be avoided as they are
> incompatible with the decompression boot phase.
> 
> Signed-off-by: John Allen <john.allen@....com>
> ---
>  arch/x86/kernel/sev-shared.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index 2eabccde94fb..e38a1d049bc1 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -890,6 +890,21 @@ static enum es_result vc_handle_cpuid(struct ghcb *ghcb,
>  		/* xgetbv will cause #GP - use reset value for xcr0 */
>  		ghcb_set_xcr0(ghcb, 1);
>  
> +	if (has_cpuflag(X86_FEATURE_SHSTK) && regs->ax == 0xd && regs->cx <= 1) {
> +		unsigned long lo, hi;
> +		u64 xss;
> +
> +		/*
> +		 * Since vc_handle_cpuid may be used during early boot, the
> +		 * rdmsr wrappers are incompatible and should not be used.
> +		 * Invoke the instruction directly.
> +		 */
> +		asm volatile("rdmsr" : "=a" (lo), "=d" (hi)
> +			     : "c" (MSR_IA32_XSS));

Does __rdmsr() work too?

I know it has exception handling but a SEV-ES guest should not fault
when accessing MSR_IA32_XSS anyway, especially if it has shadow stack
enabled. And if it does fault, your version would explode too but
__rdmsr() would be at least less code. :)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette