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

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

[Sean Christopherson]

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@google.com>
---
 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;
+
 			if (test_bit(vcpu->arch.pending_ioapic_eoi,
 				     vcpu->arch.ioapic_handled_vectors)) {
 				vcpu->run->exit_reason = KVM_EXIT_IOAPIC_EOI;

base-commit: 9d4853b044beefa21c4ee3e18c40653601a64ced
-- 
2.55.0.rc0.738.g0c8ab3ebcc-goog

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

[Huang, Kai]

On Thu, 2026-06-18 at 11:55 -0700, 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.

255 is a valid vector.  How about use a CPU reserved one instead (e.g., vector
0) and hope for the best?

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

[Sean Christopherson]

On Fri, Jun 19, 2026, Kai Huang wrote:
> On Thu, 2026-06-18 at 11:55 -0700, 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.
> 
> 255 is a valid vector.  How about use a CPU reserved one instead (e.g., vector
> 0) and hope for the best?

I was thinking it would be better to err on the side of spuriously exiting to
userspace, versus suppressing an exit?  And I wanted to keep the vector legal,
in case something else in KVM cares about legal vectors?  Hmm, but using 255 is
bad because it likely never be cleared, and thus will block other EOI exits due
to 255 being the highest priority vector.

Ah, and the field is never explicitly initialized beyond the structutre being,
so it's starting state is '0' as well.  My only hesitation with zero is that in
the unlikely case bit 0 is set in ioapic_handled_vectors, userspace will be extra
confused.  But that's easy enough to deal with, just skip the check.

This?

		if (kvm_check_request(KVM_REQ_IOAPIC_EOI_EXIT, vcpu)) {
			if (WARN_ON_ONCE(vcpu->arch.pending_ioapic_eoi < 0 ||
					 vcpu->arch.pending_ioapic_eoi > 255))
				vcpu->arch.pending_ioapic_eoi = 0;
			else if (test_bit(vcpu->arch.pending_ioapic_eoi,
					  vcpu->arch.ioapic_handled_vectors)) {
				vcpu->run->exit_reason = KVM_EXIT_IOAPIC_EOI;
				vcpu->run->eoi.vector =
						vcpu->arch.pending_ioapic_eoi;
				r = 0;
				goto out;
			}
		}

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

[Huang, Kai]

On Mon, 2026-06-22 at 16:55 -0700, Sean Christopherson wrote:
> On Fri, Jun 19, 2026, Kai Huang wrote:
> > On Thu, 2026-06-18 at 11:55 -0700, 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.
> > 
> > 255 is a valid vector.  How about use a CPU reserved one instead (e.g., vector
> > 0) and hope for the best?
> 
> I was thinking it would be better to err on the side of spuriously exiting to
> userspace, versus suppressing an exit?  And I wanted to keep the vector legal,
> in case something else in KVM cares about legal vectors?  Hmm, but using 255 is
> bad because it likely never be cleared, and thus will block other EOI exits due
> to 255 being the highest priority vector.
> 
> Ah, and the field is never explicitly initialized beyond the structutre being,
> so it's starting state is '0' as well.  My only hesitation with zero is that in
> the unlikely case bit 0 is set in ioapic_handled_vectors, userspace will be extra
> confused.  
> 

I was actually thinking 0 may be less confusing than 255.  A sane userspace
should just know 0 is a bad vector thus should report an error to admin, if not
kill the guest.  On the other hand, 255 is a valid vector so userspace may
wrongly EOI an incorrect IRQ, which could be more confusing to the guest or
userspace itself in the end?

Btw, I think killing the guest should be acceptable if such bug happens?  The
existing behaviour is to panic the host anyway ..

> But that's easy enough to deal with, just skip the check.

I am not sure ignoring the IOAPIC EOI exit (in case of this bug) is better than
reporting a invalid vector to userspace.  I guess it's fine, since the worst
case is userspace loses the EOI for an IRQ AFAICT, but I am not sure this is
better?

> 
> This?
> 
> 		if (kvm_check_request(KVM_REQ_IOAPIC_EOI_EXIT, vcpu)) {
> 			if (WARN_ON_ONCE(vcpu->arch.pending_ioapic_eoi < 0 ||
> 					 vcpu->arch.pending_ioapic_eoi > 255))
> 				vcpu->arch.pending_ioapic_eoi = 0;
> 			else if (test_bit(vcpu->arch.pending_ioapic_eoi,
> 					  vcpu->arch.ioapic_handled_vectors)) {
> 				vcpu->run->exit_reason = KVM_EXIT_IOAPIC_EOI;
> 				vcpu->run->eoi.vector =
> 						vcpu->arch.pending_ioapic_eoi;
> 				r = 0;
> 				goto out;
> 			}
> 		}

Either way works for me.  I am starting to think we care about this too much --
it's definitely better than BUG_ON() for sure :-)

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@google.com>
> ---
>   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

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

[Sean Christopherson]

On Wed, Jun 24, 2026, Paolo Bonzini wrote:
> 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@google.com>
> > ---
> >   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!

LOL.  Well, well, well, if it isn't the consequences of my own actions.

> 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?

Ya, that's waaay better, especially since pending_ioapic_eoi defaults to '0'
anyways.