linux-kernel - [PATCH] KVM: x86: Update irr_pending when setting APIC state with APICv disabled

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-next>] [day] [month] [year] [list]

Message-ID: <20241101193532.1817004-1-seanjc@google.com>
Date: Fri,  1 Nov 2024 12:35:32 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Yong He <zhuangel570@...il.com>
Subject: [PATCH] KVM: x86: Update irr_pending when setting APIC state with
 APICv disabled

Explicitly set apic->irr_pending after stuffing the vIRR when userspace
sets APIC state and APICv is disabled, otherwise KVM will skip scanning
the vIRR in subsequent calls to apic_find_highest_irr(), and ultimately
fail to inject the interrupt until another interrupt happens to be added
to the vIRR.

Only the APICv-disabled case is flawed, as KVM forces apic->irr_pending to
be true if APICv is enabled, because not all vIRR updates will be visible
to KVM.

Note, irr_pending is intentionally not updated in kvm_apic_update_apicv(),
because when APICv is being inhibited/disabled, KVM needs to keep the flag
set until the next emulated EOI so that KVM will correctly handle any
in-flight updates to the vIRR from hardware.  But when setting APIC state,
neither the VM nor the VMM can assume specific ordering between an update
from hardware and overwriting all state in kvm_apic_set_state(), thus KVM
can safely clear irr_pending if the vIRR is empty.

Reported-by: Yong He <zhuangel570@...il.com>
Closes: https://lkml.kernel.org/r/20241023124527.1092810-1-alexyonghe%40tencent.com
Signed-off-by: Sean Christopherson <seanjc@...gle.com>
---
 arch/x86/kvm/lapic.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 65412640cfc7..deb73aea2c06 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -3086,6 +3086,15 @@ int kvm_apic_set_state(struct kvm_vcpu *vcpu, struct kvm_lapic_state *s)
 		kvm_x86_call(hwapic_irr_update)(vcpu,
 						apic_find_highest_irr(apic));
 		kvm_x86_call(hwapic_isr_update)(apic_find_highest_isr(apic));
+	} else {
+		/*
+		 * Note, kvm_apic_update_apicv() is responsible for updating
+		 * isr_count and highest_isr_cache.  irr_pending is somewhat
+		 * special because it mustn't be cleared when APICv is disabled
+		 * at runtime, and only state restore can cause an IRR bit to
+		 * be set without also refreshing irr_pending.
+		 */
+		apic->irr_pending = apic_search_irr(apic) != -1;
 	}
 	kvm_make_request(KVM_REQ_EVENT, vcpu);
 	if (ioapic_in_kernel(vcpu->kvm))

base-commit: 5cb1659f412041e4780f2e8ee49b2e03728a2ba6
-- 
2.47.0.163.g1226f6d8fa-goog