Re: [PATCH] KVM: x86: Clamp the EOI vector if its OOB instead of bugging the kernel

[Paolo Bonzini]

On 6/18/26 20:55, Sean Christopherson wrote:
> If KVM handles an I/O APIC EOI exit request with a bad vector, clamp the
> vector to 255 and hope for the best instead of bugging the host.  In all
> likelihood, a missed EOI is survivable for the guest, and it's most
> definitely not remotely fatal to the host, i.e. potentially panicking the
> host is completely unjustified.  Arbitrarily use 255 for the dummy vector,
> the goal is purely to ensure the vector is covered by the bitmap.
>
> Opportunistically ensure the EOI vector isn't negative, as it's a signed
> integer, i.e. the "greater than 255" check won't guard against setting the
> vector to a negative value (KVM uses -1 to say "no IRQ" in many flows).
>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>   arch/x86/kvm/x86.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index d9d51803b7b2..fda09e03b960 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -11212,7 +11212,10 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>   		if (kvm_check_request(KVM_REQ_NMI, vcpu))
>   			process_nmi(vcpu);
>   		if (kvm_check_request(KVM_REQ_IOAPIC_EOI_EXIT, vcpu)) {
> -			BUG_ON(vcpu->arch.pending_ioapic_eoi > 255);
> +			if (WARN_ON_ONCE(vcpu->arch.pending_ioapic_eoi < 0 ||
> +					 vcpu->arch.pending_ioapic_eoi > 255))
> +				vcpu->arch.pending_ioapic_eoi = 255;
> +

Yay, it's my turn to say "why?!?" I'm not going to go full Linus on
it :) but I've been waiting for the moment for years!

If this happens we have a much bigger problem: the vector is set in
kvm_ioapic_send_eoi() and ultimately comes from apic_find_highest_isr().
It is simply not going to happen.

Unlike pending_external_vector or highest_stale_pending_ioapic_eoi, this
cannot even be -1...255 so make it u8 and call it a day?

Something like this:

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index eee473717c0e..9c444dfa7342 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1082,7 +1082,7 @@ struct kvm_vcpu_arch {
 		bool pv_unhalted;
 	} pv;
 
-	int pending_ioapic_eoi;
+	u8 pending_ioapic_eoi;
 	int pending_external_vector;
 	int highest_stale_pending_ioapic_eoi;
 
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 9d2df8623f6d..fca9be6e536d 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1541,7 +1541,7 @@ static bool kvm_ioapic_handles_vector(struct kvm_lapic *apic, int vector)
 	return test_bit(vector, apic->vcpu->arch.ioapic_handled_vectors);
 }
 
-static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector)
+static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, u8 vector)
 {
 	int __maybe_unused trigger_mode;
 
@@ -1611,7 +1611,7 @@ static int apic_set_eoi(struct kvm_lapic *apic)
  * this interface assumes a trap-like exit, which has already finished
  * desired side effect including vISR and vPPR update.
  */
-void kvm_apic_set_eoi_accelerated(struct kvm_vcpu *vcpu, int vector)
+void kvm_apic_set_eoi_accelerated(struct kvm_vcpu *vcpu, u8 vector)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index a1a5edb39a7e..57333d7bb32c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5866,7 +5866,7 @@ static int handle_apic_access(struct kvm_vcpu *vcpu)
 static int handle_apic_eoi_induced(struct kvm_vcpu *vcpu)
 {
 	unsigned long exit_qualification = vmx_get_exit_qual(vcpu);
-	int vector = exit_qualification & 0xff;
+	u8 vector = exit_qualification & 0xff;
 
 	/* EOI-induced VM exit is trap-like and thus no need to adjust IP */
 	kvm_apic_set_eoi_accelerated(vcpu, vector);

and then obviously the BUG_ON goes away.

Paolo