[PATCH] KVM: x86: Handle -1 return from kvm_cpu_get_interrupt

[PATCH] KVM: x86: Handle -1 return from kvm_cpu_get_interrupt

[Deepanshu Kartikey]

kvm_check_and_inject_events() checks for a pending PIC
interrupt using kvm_cpu_has_injectable_intr() and then
fetches it using kvm_cpu_get_interrupt(). These two
operations are not atomic with respect to vpic->output.

Between the check and fetch, another thread running on
a different CPU can call kvm_pic_read_irq() which sets
output=0 before taking pic_lock(), making it immediately
visible to all other threads:

  Thread A: kvm_cpu_has_injectable_intr() reads output=1
  Thread B: kvm_pic_read_irq() sets output=0 before lock
  Thread A: kvm_cpu_get_interrupt() reads output=0
  Thread A: returns -1, WARN_ON_ONCE fires

The -1 return is a valid result indicating another thread
already consumed the interrupt between the check and fetch.
Replace WARN_ON_ONCE with a graceful goto out to handle
this race condition correctly.

Reported-by: syzbot+9dcd0a11dc9703a49511@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=9dcd0a11dc9703a49511
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 arch/x86/kvm/x86.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index afcac1042947..36bf479a1164 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10797,11 +10797,11 @@ static int kvm_check_and_inject_events(struct kvm_vcpu *vcpu,
 		if (r) {
 			int irq = kvm_cpu_get_interrupt(vcpu);
 
-			if (!WARN_ON_ONCE(irq == -1)) {
-				kvm_queue_interrupt(vcpu, irq, false);
-				kvm_x86_call(inject_irq)(vcpu, false);
-				WARN_ON(kvm_x86_call(interrupt_allowed)(vcpu, true) < 0);
-			}
+			if (irq == -1)
+				goto out;
+			kvm_queue_interrupt(vcpu, irq, false);
+			kvm_x86_call(inject_irq)(vcpu, false);
+			WARN_ON(kvm_x86_call(interrupt_allowed)(vcpu, true) < 0);
 		}
 		if (kvm_cpu_has_injectable_intr(vcpu))
 			kvm_x86_call(enable_irq_window)(vcpu);
-- 
2.43.0

Re: [PATCH] KVM: x86: Handle -1 return from kvm_cpu_get_interrupt

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces a control flow bypass regression. By replacing `WARN_ON_ONCE(irq == -1)` with an early `goto out;`, the function `kvm_check_and_inject_events()` prematurely aborts. This inadvertently skips the critical trailing check for nested events, potentially breaking nested virtualization.
--

--- Patch [1]: [PATCH] KVM: x86: Handle -1 return from kvm_cpu_get_interrupt ---
commit 5f633807782c397266e8aeeaaf39fb310a7b09ee
Author: Deepanshu Kartikey <kartikey406@gmail.com>

KVM: x86: Handle -1 return from kvm_cpu_get_interrupt

This commit modifies kvm_check_and_inject_events() to gracefully handle a
valid race condition where kvm_cpu_get_interrupt() returns -1. It replaces
a WARN_ON_ONCE with an early goto out to handle situations where another
thread has already consumed the interrupt.

Link: https://syzkaller.appspot.com/bug?extid=9dcd0a11dc9703a49511

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ff8812f3a1293..ec449e9a3f954 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -10765,11 +10765,11 @@ static int kvm_check_and_inject_events(struct kvm_vcpu *vcpu,
>  		if (r) {
>  			int irq = kvm_cpu_get_interrupt(vcpu);
>  
> -			if (!WARN_ON_ONCE(irq == -1)) {
> -				kvm_queue_interrupt(vcpu, irq, false);
> -				kvm_x86_call(inject_irq)(vcpu, false);
> -				WARN_ON(kvm_x86_call(interrupt_allowed)(vcpu, true) < 0);
> -			}
> +			if (irq == -1)
> +				goto out;

[Severity: High]
Does this early goto out prematurely skip the remaining checks in
kvm_check_and_inject_events()?

By jumping directly to out, the code skips the enable_irq_window() call and
the trailing nested event evaluation block:

	if (is_guest_mode(vcpu) &&
	    kvm_x86_ops.nested_ops->has_events &&
	    kvm_x86_ops.nested_ops->has_events(vcpu, true))
		*req_immediate_exit = true;

When a concurrently consumed PIC interrupt results in irq == -1, wouldn't
bypassing this logic prevent KVM from setting req_immediate_exit when
required, potentially causing the vCPU to incorrectly continue executing L2
guest code instead of exiting to L1?

> +			kvm_queue_interrupt(vcpu, irq, false);
> +			kvm_x86_call(inject_irq)(vcpu, false);
> +			WARN_ON(kvm_x86_call(interrupt_allowed)(vcpu, true) < 0);
>  		}
>  		if (kvm_cpu_has_injectable_intr(vcpu))
>  			kvm_x86_call(enable_irq_window)(vcpu);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260712090039.36038-1-kartikey406@gmail.com?part=1