linux-kernel - Re: [PATCH v15 29/41] KVM: SEV: Synchronize MSR_IA32

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: <aMnY7NqhhnMYqu7m@google.com>
Date: Tue, 16 Sep 2025 14:38:52 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: John Allen <john.allen@....com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Tom Lendacky <thomas.lendacky@....com>, Mathias Krause <minipli@...ecurity.net>, 
	Rick Edgecombe <rick.p.edgecombe@...el.com>, Chao Gao <chao.gao@...el.com>, 
	Maxim Levitsky <mlevitsk@...hat.com>, Xiaoyao Li <xiaoyao.li@...el.com>
Subject: Re: [PATCH v15 29/41] KVM: SEV: Synchronize MSR_IA32_XSS from the
 GHCB when it's valid

On Tue, Sep 16, 2025, John Allen wrote:
> On Tue, Sep 16, 2025 at 12:53:58PM -0700, Sean Christopherson wrote:
> > On Tue, Sep 16, 2025, John Allen wrote:
> > > On Fri, Sep 12, 2025 at 04:23:07PM -0700, Sean Christopherson wrote:
> > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > > index 0cd77a87dd84..0cd32df7b9b6 100644
> > > > --- a/arch/x86/kvm/svm/sev.c
> > > > +++ b/arch/x86/kvm/svm/sev.c
> > > > @@ -3306,6 +3306,9 @@ static void sev_es_sync_from_ghcb(struct vcpu_svm *svm)
> > > >  	if (kvm_ghcb_xcr0_is_valid(svm))
> > > >  		__kvm_set_xcr(vcpu, 0, kvm_ghcb_get_xcr0(ghcb));
> > > >  
> > > > +	if (kvm_ghcb_xss_is_valid(svm))
> > > > +		__kvm_emulate_msr_write(vcpu, MSR_IA32_XSS, kvm_ghcb_get_xss(ghcb));
> > > > +
> > > 
> > > It looks like this is the change that caused the selftest regression
> > > with sev-es. It's not yet clear to me what the problem is though.
> > 
> > Do you see any WARNs in the guest kernel log?
> > 
> > The most obvious potential bug is that KVM is missing a CPUID update, e.g. due
> > to dropping an XSS write, consuming stale data, not setting cpuid_dynamic_bits_dirty,
> > etc.  But AFAICT, CPUID.0xD.1.EBX (only thing that consumes the current XSS) is
> > only used by init_xstate_size(), and I would expect the guest kernel's sanity
> > checks in paranoid_xstate_size_valid() to yell if KVM botches CPUID emulation.
> 
> Yes, actually that looks to be the case:
> 
> [    0.463504] ------------[ cut here ]------------
> [    0.464443] XSAVE consistency problem: size 880 != kernel_size 840
> [    0.465445] WARNING: CPU: 0 PID: 0 at arch/x86/kernel/fpu/xstate.c:638 paranoid_xstate_size_valid+0x101/0x140

Can you run with the below printk tracing in the host (and optionally tracing in
the guest for its updates)?  Compile tested only.

There should be very few XSS updates, so this _shouldn't_ spam/crash your host :-)

---
 arch/x86/kvm/svm/sev.c |  6 ++++--
 arch/x86/kvm/x86.c     | 15 ++++++++++++---
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 0cd32df7b9b6..8ac87d623767 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3306,8 +3306,10 @@ static void sev_es_sync_from_ghcb(struct vcpu_svm *svm)
 	if (kvm_ghcb_xcr0_is_valid(svm))
 		__kvm_set_xcr(vcpu, 0, kvm_ghcb_get_xcr0(ghcb));
 
-	if (kvm_ghcb_xss_is_valid(svm))
-		__kvm_emulate_msr_write(vcpu, MSR_IA32_XSS, kvm_ghcb_get_xss(ghcb));
+	if (kvm_ghcb_xss_is_valid(svm)) {
+		if (__kvm_emulate_msr_write(vcpu, MSR_IA32_XSS, kvm_ghcb_get_xss(ghcb));
+			pr_warn("Dropped XSS update, val = %llx\n", data);
+	}
 
 	/* Copy the GHCB exit information into the VMCB fields */
 	exit_code = kvm_ghcb_get_sw_exit_code(ghcb);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c78acab2ff3f..a846ed69ce2c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4118,13 +4118,22 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		}
 		break;
 	case MSR_IA32_XSS:
-		if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVES))
+		if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) {
+			pr_warn("Guest CPUID doesn't have XSAVES\n");
 			return KVM_MSR_RET_UNSUPPORTED;
+		}
 
-		if (data & ~vcpu->arch.guest_supported_xss)
+		if (data & ~vcpu->arch.guest_supported_xss) {
+			pr_warn("Invalid XSS: supported = %llx, val = %llx\n",
+				vcpu->arch.guest_supported_xss, data);
 			return 1;
-		if (vcpu->arch.ia32_xss == data)
+		}
+		if (vcpu->arch.ia32_xss == data) {
+			pr_warn("XSS already set to val = %llx, eliding updates\n", data);
 			break;
+		}
+
+		pr_warn("XSS updated to val = %llx, marking CPUID dirty\n", data);
 		vcpu->arch.ia32_xss = data;
 		vcpu->arch.cpuid_dynamic_bits_dirty = true;
 		break;

base-commit: 14298d819d5a6b7180a4089e7d2121ca3551dc6c
--