linux-kernel - Re: [PATCH 2/2] KVM: x86: Fix split-irqchip vs interrupt injection window request

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

Date:   Thu, 15 Apr 2021 16:06:26 +0800
From:   Lai Jiangshan <jiangshanlai+lkml@...il.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Sean Christopherson <seanjc@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>, kvm@...r.kernel.org,
        Filippo Sironi <sironi@...zon.de>,
        David Woodhouse <dwmw@...zon.co.uk>,
        "v4.7+" <stable@...r.kernel.org>,
        Wanpeng Li <wanpengli@...cent.com>
Subject: Re: [PATCH 2/2] KVM: x86: Fix split-irqchip vs interrupt injection
 window request

On Thu, Apr 15, 2021 at 2:07 PM Paolo Bonzini <pbonzini@...hat.com> wrote:
>
> On 15/04/21 02:59, Lai Jiangshan wrote:
> > The next call to inject_pending_event() will reach here AT FIRST with
> > vcpu->arch.exception.injected==false and vcpu->arch.exception.pending==false
> >
> >>           ... if (!vcpu->arch.exception.pending) {
> >>                   if (vcpu->arch.nmi_injected) {
> >>                           static_call(kvm_x86_set_nmi)(vcpu);
> >>                           can_inject = false;
> >>                   } else if (vcpu->arch.interrupt.injected) {
> >>                           static_call(kvm_x86_set_irq)(vcpu);
> >>                           can_inject = false;
> >
> > And comes here and vcpu->arch.interrupt.injected is true for there is
> > an interrupt queued by KVM_INTERRUPT for pure user irqchip. It then does
> > the injection of the interrupt without checking the EFLAGS.IF.
>
> Ok, understood now.  Yeah, that could be a problem for userspace irqchip
> so we should switch it to use pending_external_vector instead.  Are you
> going to write the patch or should I?
>

I wish you do it.  I haven't figured out how to write a clean test for
it and confirm it in upstream.  But I will backport your patch and test it.

My fix is changing the behavior back to before 664f8e26b00c7 where
arch.exception.pending=true would prevent ready_for_interrupt_injection
to be non-zero.  So that KVM_INTERRUPT maintains the original behavior
that it can immediately inject IRQ into guests. (Userspace may regret
an unearthly injected IRQ for it has no right to revise the IRQ or cancel
it.)  But your fix will unify the behaviors of all kinds of irqchips.

Thanks
Lai


> Thanks!
>
> Paolo
>
> > My question is that what stops the next call to inject_pending_event()
> > to reach here when KVM_INTERRUPT is called with exepction pending.
>