linux-kernel - Re: [PATCH 6/9] KVM: SVM: Add MSR_IA32_XSS to the GHCB for hypervisor kernel

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: <Zc5MRqmspThUoB+n@AUS-L1-JOHALLEN.amd.com>
Date: Thu, 15 Feb 2024 11:39:18 -0600
From: John Allen <john.allen@....com>
To: Sean Christopherson <seanjc@...gle.com>, mlevitsk@...hat.com
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, pbonzini@...hat.com,
	weijiang.yang@...el.com, rick.p.edgecombe@...el.com, x86@...nel.org,
	thomas.lendacky@....com, bp@...en8.de
Subject: Re: [PATCH 6/9] KVM: SVM: Add MSR_IA32_XSS to the GHCB for
 hypervisor kernel

On Tue, Nov 07, 2023 at 08:20:52PM +0200, Maxim Levitsky wrote:
> On Thu, 2023-11-02 at 16:22 -0700, Sean Christopherson wrote:
> > On Thu, Nov 02, 2023, Maxim Levitsky wrote:
> > > On Tue, 2023-10-10 at 20:02 +0000, John Allen wrote:
> > > > @@ -3032,6 +3037,9 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
> > > >  		if (guest_cpuid_has(&svm->vcpu, X86_FEATURE_RDTSCP))
> > > >  			svm_clr_intercept(svm, INTERCEPT_RDTSCP);
> > > >  	}
> > > > +
> > > > +	if (kvm_caps.supported_xss)
> > > > +		set_msr_interception(vcpu, svm->msrpm, MSR_IA32_XSS, 1, 1);
> > > 
> > > This is not just a virtualization hole. This allows the guest to set MSR_IA32_XSS
> > > to whatever value it wants, and thus it might allow XSAVES to access some host msrs
> > > that guest must not be able to access.
> > > 
> > > AMD might not yet have such msrs, but on Intel side I do see various components
> > > like 'HDC State', 'HWP state' and such.
> > 
> > The approach AMD has taken with SEV-ES+ is to have ucode context switch everything
> > that the guest can access.  So, in theory, if/when AMD adds more XCR0/XSS-based
> > features, that state will also be context switched.
> > 
> > Don't get me wrong, I hate this with a passion, but it's not *quite* fatally unsafe,
> > just horrific.
> > 
> > > I understand that this is needed so that #VC handler could read this msr, and
> > > trying to read it will cause another #VC which is probably not allowed (I
> > > don't know this detail of SEV-ES)
> > > 
> > > I guess #VC handler should instead use a kernel cached value of this msr
> > > instead, or at least KVM should only allow reads and not writes to it.
> > 
> > Nope, doesn't work.  In addition to automatically context switching state, SEV-ES
> > also encrypts the guest state, i.e. KVM *can't* correctly virtualize XSS (or XCR0)
> > for the guest, because KVM *can't* load the guest's desired value into hardware.
> > 
> > The guest can do #VMGEXIT (a.k.a. VMMCALL) all it wants to request a certain XSS
> > or XCR0, and there's not a damn thing KVM can do to service the request.
> > 
> 
> Ah, I understand now. Everything makes sense, and yes, this is really ugly.

Hi Maxim and Sean,

It looks as though there are some broad changes that will need to happen
over the long term WRT to SEV-ES/SEV-SNP. In the short term, how would
you suggest I proceed with the SVM shstk series? Can we omit the SEV-ES
changes for now with an additional patch that disallows guest shstk when
SEV-ES is enabled? Subsequently, when we have a proper solution for the
concerns discussed here, we could submit another series for SEV-ES
support.

Thanks,
John