From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753883AbcANKd6 (ORCPT ); Thu, 14 Jan 2016 05:33:58 -0500 Received: from mail.skyhub.de ([78.46.96.112]:33385 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753598AbcANKdn (ORCPT ); Thu, 14 Jan 2016 05:33:43 -0500 Date: Thu, 14 Jan 2016 11:33:26 +0100 From: Borislav Petkov To: Thomas Gleixner Cc: Joe Lawrence , LKML , Ingo Molnar , Peter Anvin , Jiang Liu , Jeremiah Mahler , andy.shevchenko@gmail.com, Guenter Roeck Subject: Re: [patch 00/14] x86/irq: Plug various vector cleanup races Message-ID: <20160114103326.GG8496@pd.tnic> References: <20151231155849.772553760@linutronix.de> <568A9157.9070402@stratus.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 14, 2016 at 09:24:35AM +0100, Thomas Gleixner wrote: > On Mon, 4 Jan 2016, Joe Lawrence wrote: > > No issues running the same PCI device removal and stress tests against > > the patchset. > > Thanks for testing! > > Though there is yet another long standing bug in that area. Fix below. > > Thanks, > > tglx > > 8<-------------------- > > Subject: x86/irq: Call chip->irq_set_affinity in proper context > From: Thomas Gleixner > Date: Thu, 14 Jan 2016 08:43:38 +0100 > > setup_ioapic_dest() calls irqchip->irq_set_affinity() completely > unprotected. That's wrong in several aspects: > > - it triggers a lockdep splat because vector lock is taken with interrupts > enabled. > > - it opens a race window where irq_set_affinity() can be interrupted and the > irq chip left in unconsistent state. > > The proper calling convention is irq descriptor lock held and interrupts > disabled. > > Reported-by: Borislav Petkov > Signed-off-by: Thomas Gleixner > Cc: stable@vger.kernel.org > --- > arch/x86/kernel/apic/io_apic.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > --- a/arch/x86/kernel/apic/io_apic.c > +++ b/arch/x86/kernel/apic/io_apic.c > @@ -2521,6 +2521,7 @@ void __init setup_ioapic_dest(void) > { > int pin, ioapic, irq, irq_entry; > const struct cpumask *mask; > + struct irq_desc *desc; > struct irq_data *idata; > struct irq_chip *chip; > > @@ -2536,7 +2537,9 @@ void __init setup_ioapic_dest(void) > if (irq < 0 || !mp_init_irq_at_boot(ioapic, irq)) > continue; > > - idata = irq_get_irq_data(irq); > + desc = irq_to_desc(irq); > + raw_spin_lock_irq(d&desc->lock); s/d// > + idata = irq_desc_get_irq_data(desc); > > /* > * Honour affinities which have been set in early boot > @@ -2550,6 +2553,7 @@ void __init setup_ioapic_dest(void) > /* Might be lapic_chip for irq 0 */ > if (chip->irq_set_affinity) > chip->irq_set_affinity(idata, mask, false); > + raw_spin_unlock_irq(d&desc->lock); s/d// With those micro-changes: Tested-by: Borislav Petkov :-) -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply.