All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: syzbot <syzbot@kernel.org>
Cc: syzkaller-bugs@googlegroups.com, Borislav Petkov <bp@alien8.de>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	kvm@vger.kernel.org,  Ingo Molnar <mingo@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	 Thomas Gleixner <tglx@kernel.org>,
	x86@kernel.org, hpa@zytor.com,  linux-kernel@vger.kernel.org,
	syzbot@lists.linux.dev
Subject: Re: [PATCH] KVM: x86: Drop WARN_ON_ONCE() for concurrently disappearing interrupts
Date: Wed, 24 Jun 2026 07:00:19 -0700	[thread overview]
Message-ID: <ajvi89dlwvirl4BI@google.com> (raw)
In-Reply-To: <345e9d6c-d7d9-4bab-adb3-d6a7bd27599f@mail.kernel.org>

On Wed, Jun 24, 2026, syzbot wrote:
> From: Alexander Potapenko <glider@google.com>
> 
> A warning can be triggered in kvm_check_and_inject_events() when an
> interrupt disappears between the time it is checked via
> kvm_cpu_has_injectable_intr() and the time it is fetched via
> kvm_cpu_get_interrupt(). This occurs because the warning incorrectly
> assumes that if an interrupt is injectable, fetching it must always return
> a valid interrupt vector (i.e., not -1).
> 
> However, this assumption is broken by level-triggered interrupts that are
> deasserted concurrently by another thread. For example, if a misconfigured
> PIT or a PCI device asserts and then immediately deasserts a
> level-triggered interrupt, the VCPU thread might see the pending interrupt
> during the check but find it gone during the fetch, resulting in
> kvm_cpu_get_interrupt() returning -1.

I think this the race is limited to the PIC case, no?  Because for all other
cases, mucking with pending IRQs requires holding vcpu->mutex.

So rather than removing the WARN entirely, can't we fix this by exempting the
in-kernel PIC case?  Given that we're pushing hard to move to a split IRQCHIP
model, this would preserve the WARN for the use case we care most about.

diff --git arch/x86/kvm/x86.c arch/x86/kvm/x86.c
index 3a2e4493516f..64f6592f5b23 100644
--- arch/x86/kvm/x86.c
+++ arch/x86/kvm/x86.c
@@ -7694,7 +7694,7 @@ 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)) {
+                       if (!WARN_ON_ONCE(irq == -1 && !pic_in_kernel(vcpu->kvm))) {
                                kvm_queue_interrupt(vcpu, irq, false);
                                kvm_x86_call(inject_irq)(vcpu, false);
                                WARN_ON(kvm_x86_call(interrupt_allowed)(vcpu, true) < 0);

> The warning manifests as follows:
> 
> ------------[ cut here ]------------
> irq == -1
> WARNING: arch/x86/kvm/x86.c:10860 at kvm_check_and_inject_events
> arch/x86/kvm/x86.c:10860 [inline]
> WARNING: arch/x86/kvm/x86.c:10860 at vcpu_enter_guest
> arch/x86/kvm/x86.c:11356 [inline]
> WARNING: arch/x86/kvm/x86.c:10860 at vcpu_run+0x57ec/0x7950
> arch/x86/kvm/x86.c:11770
> RIP: 0010:kvm_check_and_inject_events arch/x86/kvm/x86.c:10860 [inline]
> RIP: 0010:vcpu_enter_guest arch/x86/kvm/x86.c:11356 [inline]
> RIP: 0010:vcpu_run+0x57ec/0x7950 arch/x86/kvm/x86.c:11770
> Call Trace:
>  <TASK>
>  kvm_arch_vcpu_ioctl_run+0x1193/0x2070 arch/x86/kvm/x86.c:12125
>  kvm_vcpu_ioctl+0xa61/0xfd0 virt/kvm/kvm_main.c:4470
>  vfs_ioctl fs/ioctl.c:51 [inline]
>  __do_sys_ioctl fs/ioctl.c:597 [inline]
>  __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:583
>  do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>  do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>  </TASK>
> 
> Since this is a legitimate Time-Of-Check to Time-Of-Use (TOCTOU) race
> condition that can occur during normal operation, WARN_ON_ONCE() must not
> be used for conditions that can legitimately happen. The patch removes the
> WARN_ON_ONCE() in kvm_check_and_inject_events() and replaces it with a
> pr_err_ratelimited() to log the event instead.

If we can't salvage the WARN, I don't want to log anything to dmesg.  The purpose
of the WARN is to find KVM bugs.  A ratelimited printk isn't going to help on that
front.

> Fixes: bf672720e83c ("KVM: x86: check the kvm_cpu_get_interrupt result before using it")
> Assisted-by: Gemini:gemini-3.1-pro-preview Gemini:gemini-3-flash-preview syzbot
> Reported-by: syzbot+dd769db18693736eee89@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=dd769db18693736eee89
> Link: https://syzkaller.appspot.com/ai_job?id=35cad3cd-95fd-4c0d-8ca8-812f58d56e59
> Signed-off-by: Alexander Potapenko <glider@google.com>
> 
> ---
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0550359ed..c5b4cddd9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -10857,10 +10857,13 @@ 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)) {
> +			if (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);
> +			} else {
> +				pr_err_ratelimited(
> +					"KVM: interrupt disappeared between checking and fetching\n");
>  			}
>  		}
>  		if (kvm_cpu_has_injectable_intr(vcpu))
> 
> 
> base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
> -- 
> See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
> You can comment on the patch as usual, syzbot will try to address
> the comments and send a new version of the patch if necessary.
> syzbot engineers can be reached at syzkaller@googlegroups.com.

      parent reply	other threads:[~2026-06-24 14:00 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-24 13:15 [PATCH] KVM: x86: Drop WARN_ON_ONCE() for concurrently disappearing interrupts syzbot
2026-06-24 13:23 ` sashiko-bot
2026-06-24 14:00 ` Sean Christopherson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajvi89dlwvirl4BI@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=syzbot@kernel.org \
    --cc=syzbot@lists.linux.dev \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=tglx@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.