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
| ||
|
Date: Tue, 21 Apr 2015 10:47:37 +0200 From: Paolo Bonzini <pbonzini@...hat.com> To: linux-kernel@...r.kernel.org, kvm@...r.kernel.org CC: saso.linux@...im.si, lists2009@...rfbargle.com, Nadav Amit <namit@...technion.ac.il>, stable@...r.kernel.org Subject: Re: [PATCH stable] KVM: x86: Fix lost interrupt on irr_pending race On 21/04/2015 09:52, Paolo Bonzini wrote: > From: Nadav Amit <namit@...technion.ac.il> > > [ upstream commit f210f7572bedf3320599e8b2d8e8ec2d96270d0b ] > > apic_find_highest_irr assumes irr_pending is set if any vector in APIC_IRR is > set. If this assumption is broken and apicv is disabled, the injection of > interrupts may be deferred until another interrupt is delivered to the guest. > Ultimately, if no other interrupt should be injected to that vCPU, the pending > interrupt may be lost. > > commit 56cc2406d68c ("KVM: nVMX: fix "acknowledge interrupt on exit" when APICv > is in use") changed the behavior of apic_clear_irr so irr_pending is cleared > after setting APIC_IRR vector. After this commit, if apic_set_irr and > apic_clear_irr run simultaneously, a race may occur, resulting in APIC_IRR > vector set, and irr_pending cleared. In the following example, assume a single > vector is set in IRR prior to calling apic_clear_irr: > > apic_set_irr apic_clear_irr > ------------ -------------- > apic->irr_pending = true; > apic_clear_vector(...); > vec = apic_search_irr(apic); > // => vec == -1 > apic_set_vector(...); > apic->irr_pending = (vec != -1); > // => apic->irr_pending == false > > Nonetheless, it appears the race might even occur prior to this commit: > > apic_set_irr apic_clear_irr > ------------ -------------- > apic->irr_pending = true; > apic->irr_pending = false; > apic_clear_vector(...); > if (apic_search_irr(apic) != -1) > apic->irr_pending = true; > // => apic->irr_pending == false > apic_set_vector(...); > > Fixing this issue by: > 1. Restoring the previous behavior of apic_clear_irr: clear irr_pending, call > apic_clear_vector, and then if APIC_IRR is non-zero, set irr_pending. > 2. On apic_set_irr: first call apic_set_vector, then set irr_pending. > > Signed-off-by: Nadav Amit <namit@...technion.ac.il> > Fixes: 33e4c68656a2e461b296ce714ec322978de85412 > Cc: stable@...r.kernel.org # 2.6.32+ > Signed-off-by: Paolo Bonzini <pbonzini@...hat.com> > --- > The race was reported in 3.17+ by Brad Campbell and in > 2.6.32 by Saso Slavicic, so it qualifies for stable. Patch for kernels before 3.17: diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 6e8ce5a1a05d..e0e5642dae41 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -341,8 +341,12 @@ EXPORT_SYMBOL_GPL(kvm_apic_update_irr); static inline void apic_set_irr(int vec, struct kvm_lapic *apic) { - apic->irr_pending = true; apic_set_vector(vec, apic->regs + APIC_IRR); + /* + * irr_pending must be true if any interrupt is pending; set it after + * APIC_IRR to avoid race with apic_clear_irr + */ + apic->irr_pending = true; } static inline int apic_search_irr(struct kvm_lapic *apic) Thanks, Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists