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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 23 Aug 2020 09:48:49 -0700 From: "Raj, Ashok" <ashok.raj@...el.com> To: linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de> Cc: Sukumar Ghorai <sukumar.ghorai@...el.com>, Srikanth Nandamuri <srikanth.nandamuri@...el.com>, Evan Green <evgreen@...omium.org>, Mathias Nyman <mathias.nyman@...ux.intel.com>, Bjorn Helgaas <bhelgaas@...gle.com>, stable@...r.kernel.org, x86@...nel.org, Ashok Raj <ashok.raj@...el.com> Subject: Re: [PATCH v2] x86/hotplug: Silence APIC only after all irq's are migrated Hi Thomas, I was wondering if you got a chance to take a look at this fix? I had some mail issues recently and they showed up at lore after 2 days. I wasn't sure if you got the original mail, or maybe it didn't make it. If you had a different way to fix it, we can try those out. On Thu, Aug 20, 2020 at 05:42:03PM -0700, Ashok Raj wrote: > When offlining CPUs, fixup_irqs() migrates all interrupts away from the > outgoing CPU to an online CPU. It's always possible the device sent an > interrupt to the previous CPU destination. Pending interrupt bit in IRR in > LAPIC identifies such interrupts. apic_soft_disable() will not capture any > new interrupts in IRR. This causes interrupts from device to be lost during > CPU offline. The issue was found when explicitly setting MSI affinity to a > CPU and immediately offlining it. It was simple to recreate with a USB > ethernet device and doing I/O to it while the CPU is offlined. Lost > interrupts happen even when Interrupt Remapping is enabled. > > Current code does apic_soft_disable() before migrating interrupts. > > native_cpu_disable() > { > ... > apic_soft_disable(); > cpu_disable_common(); > --> fixup_irqs(); // Too late to capture anything in IRR. > } > > Just flipping the above call sequence seems to hit the IRR checks > and the lost interrupt is fixed for both legacy MSI and when > interrupt remapping is enabled. On another note, we have tested both with and without the read after write when programming MSI addr/data on the device. It didn't seem to change the results. But I think its a useful one to add for correctness. https://lore.kernel.org/lkml/878si6rx7f.fsf@nanos.tec.linutronix.de/ This bug been eluding for a while. Looking for your feedback. > > Fixes: 60dcaad5736f ("x86/hotplug: Silence APIC and NMI when CPU is dead") > Link: https://lore.kernel.org/lkml/875zdarr4h.fsf@nanos.tec.linutronix.de/ > Reported-by: Evan Green <evgreen@...omium.org> > Tested-by: Mathias Nyman <mathias.nyman@...ux.intel.com> > Tested-by: Evan Green <evgreen@...omium.org> > Reviewed-by: Evan Green <evgreen@...omium.org> > Signed-off-by: Ashok Raj <ashok.raj@...el.com> > --- > v2: > - Typos and fixes suggested by Randy Dunlap > > To: linux-kernel@...r.kernel.org > To: Thomas Gleixner <tglx@...utronix.de> > Cc: Sukumar Ghorai <sukumar.ghorai@...el.com> > Cc: Srikanth Nandamuri <srikanth.nandamuri@...el.com> > Cc: Evan Green <evgreen@...omium.org> > Cc: Mathias Nyman <mathias.nyman@...ux.intel.com> > Cc: Bjorn Helgaas <bhelgaas@...gle.com> > Cc: stable@...r.kernel.org > --- > arch/x86/kernel/smpboot.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c > index 27aa04a95702..3016c3b627ce 100644 > --- a/arch/x86/kernel/smpboot.c > +++ b/arch/x86/kernel/smpboot.c > @@ -1594,13 +1594,20 @@ int native_cpu_disable(void) > if (ret) > return ret; > > + cpu_disable_common(); > /* > * Disable the local APIC. Otherwise IPI broadcasts will reach > * it. It still responds normally to INIT, NMI, SMI, and SIPI > - * messages. > + * messages. It's important to do apic_soft_disable() after > + * fixup_irqs(), because fixup_irqs() called from cpu_disable_common() > + * depends on IRR being set. After apic_soft_disable() CPU preserves > + * currently set IRR/ISR but new interrupts will not set IRR. > + * This causes interrupts sent to outgoing CPU before completion > + * of IRQ migration to be lost. Check SDM Vol 3 "10.4.7.2 Local > + * APIC State after It Has been Software Disabled" section for more > + * details. > */ > apic_soft_disable(); > - cpu_disable_common(); > > return 0; > } > -- > 2.7.4 >
Powered by blists - more mailing lists