linux-kernel - Re: [PATCH 2/2] KVM: nVMX: Defer SVI update to vmcs01 on EOI when L2 is active w/o VID

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: <Zyl9NABhmVTDvnQT@google.com>
Date: Mon, 4 Nov 2024 18:04:36 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	"Markku Ahvenjärvi" <mankku@...il.com>, Janne Karhunen <janne.karhunen@...il.com>
Subject: Re: [PATCH 2/2] KVM: nVMX: Defer SVI update to vmcs01 on EOI when L2
 is active w/o VID

On Mon, Nov 04, 2024, Chao Gao wrote:
> >@@ -6873,6 +6873,23 @@ void vmx_hwapic_isr_update(struct kvm_vcpu *vcpu, int max_isr)
> > 	u16 status;
> > 	u8 old;
> > 
> >+	/*
> >+	 * If L2 is active, defer the SVI update until vmcs01 is loaded, as SVI
> >+	 * is only relevant for if and only if Virtual Interrupt Delivery is
> >+	 * enabled in vmcs12, and if VID is enabled then L2 EOIs affect L2's
> >+	 * vAPIC, not L1's vAPIC.  KVM must update vmcs01 on the next nested
> >+	 * VM-Exit, otherwise L1 with run with a stale SVI.
> >+	 */
> >+	if (is_guest_mode(vcpu)) {
> >+		/*
> >+		 * KVM is supposed to forward intercepted L2 EOIs to L1 if VID
> >+		 * is enabled in vmcs12; as above, the EOIs affect L2's vAPIC.
> >+		 */
> >+		WARN_ON_ONCE(nested_cpu_has_vid(get_vmcs12(vcpu)));
> 
> This function can be called in other scenarios, e.g., from
> kvm_apic_set_state(). Is it possible for the userspace VMM to set the APIC
> state while L2 is running and VID is enabled for L2? If so, this warning could
> be triggered by the userspace VMM.

Ugh, yeah, that's reachable.  Bummer.  Ooh!  I think we can use the recently
added "wants_to_run" to filter out state stuffing.  I.e. this

		WARN_ON_ONCE(vcpu->wants_to_run &&
			     nested_cpu_has_vid(get_vmcs12(vcpu)));

plus an updated comment.