| 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
| ||
|
Message-ID: <27240C0AC20F114CBF8149A2696CBE4A1A4EAD@SHSMSX101.ccr.corp.intel.com>
Date: Fri, 19 Oct 2012 06:23:47 +0000
From: "Liu, Chuansheng" <chuansheng.liu@...el.com>
To: Yinghai Lu <yinghai@...nel.org>
CC: "tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"hpa@...or.com" <hpa@...or.com>,
"Siddha, Suresh B" <suresh.b.siddha@...el.com>,
"mathias.nyman@...ux.intel.com" <mathias.nyman@...ux.intel.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] x86/ioapic: Fix that not all allocated irqs are ioapic
type irqs
> -----Original Message-----
> From: yhlu.kernel@...il.com [mailto:yhlu.kernel@...il.com] On Behalf Of
> Yinghai Lu
> Sent: Friday, October 19, 2012 2:11 PM
> To: Liu, Chuansheng
> Cc: tglx@...utronix.de; mingo@...hat.com; hpa@...or.com; Siddha, Suresh B;
> mathias.nyman@...ux.intel.com; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH] x86/ioapic: Fix that not all allocated irqs are ioapic type
> irqs
>
> On Fri, Oct 19, 2012 at 3:41 AM, Chuansheng Liu
> <chuansheng.liu@...el.com> wrote:
> >
> > When debugging our system issues related with __setup_vector_irq(),
> > found there is a real wrong code that:
> > for_each_active_irq(irq) {
> > cfg = irq_get_chip_data(irq);
> > if (!cfg)
> > continue;
> >
> > These codes presume all allocated irqs are ioapic irqs, but it is not
> > like that, in our system there are many GPIO interrupts also.
> >
> > When one irq is not ioapic type irq, the chip_data will not be the
> > type of struct irq_cfg in most cases.
>
> impossible !
>
> where is the irq_desc coming from. ?
>
> after alloc_irq_from, alloc_irq_cfg get called too.
Maybe I do not describe the issue clearly.
When I offlined CPU3 and online it again, __setup_vector_irq will be called.
But for_each_active_irq(irq) will list all active irqs, many irqs are not IOAPIC type.
And the chip data is not struct irq_cfg at all, it is set by other chips.
For example, in file gpio-omap.c:
irq_set_chip_data(j, bank); the bank is the type of struct gpio_bank.
And in this case, __setup_vector_irq can not go on the below code due to the wrong type variable:
/*
* If it is a legacy IRQ handled by the legacy PIC, this cpu
* will be part of the irq_cfg's domain.
*/
if (irq < legacy_pic->nr_legacy_irqs && !IO_APIC_IRQ(irq))
cpumask_set_cpu(cpu, cfg->domain);
if (!cpumask_test_cpu(cpu, cfg->domain))
continue;
vector = cfg->vector;
per_cpu(vector_irq, cpu)[vector] = irq;
>
> >
> > So in function __setup_vector_irq(), it will cause some strange issues,
> > moreover, if I added some prints(cfg->...) inside it, it can always
> > cause system panic.
> >
> > Here using the struct irq_chip->flags to help identify if the irq
> > is ioapic type or not.
> >
> > Looked forward all codes with for_each_active_irq(), found there is
> > a commit 6fd36ba02 indicates the similar case in print_IO_APICs().
>
> that is for not printing wrong for MSI etc.
No, it is for avoidingcrash if it is not IOAPIC chip. I can reproduce it in my system.
>
> >
> > Signed-off-by: liu chuansheng <chuansheng.liu@...el.com>
> > ---
> > arch/x86/kernel/apic/io_apic.c | 25 ++++++++++++++++++++++---
> > 1 files changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> > index c265593..f0355e6 100644
> > --- a/arch/x86/kernel/apic/io_apic.c
> > +++ b/arch/x86/kernel/apic/io_apic.c
> > @@ -68,6 +68,18 @@
> > #define for_each_irq_pin(entry, head) \
> > for (entry = head; entry; entry = entry->next)
> >
> > +/* need more thoughts ... */
> > +#define CHIP_FLAG_IOAPIC 0x1000
> > +static inline bool is_ioapic_irq(int irq)
> > +{
> > + struct irq_chip *chip;
> > + chip = irq_get_chip(irq);
> > + if ((chip) && (chip->flags == CHIP_FLAG_IOAPIC))
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > #ifdef CONFIG_IRQ_REMAP
> > static void irq_remap_modify_chip_defaults(struct irq_chip *chip);
> > static inline bool irq_remapped(struct irq_cfg *cfg)
> > @@ -1238,6 +1250,9 @@ void __setup_vector_irq(int cpu)
> > raw_spin_lock(&vector_lock);
> > /* Mark the inuse vectors */
> > for_each_active_irq(irq) {
> > + if (!is_ioapic_irq(irq))
> > + continue;
> > +
> > cfg = irq_get_chip_data(irq);
> > if (!cfg)
> > continue;
> > @@ -1641,7 +1656,6 @@ __apicdebuginit(void) print_IO_APICs(void)
> > int ioapic_idx;
> > struct irq_cfg *cfg;
> > unsigned int irq;
> > - struct irq_chip *chip;
> >
> > printk(KERN_DEBUG "number of MP IRQ sources: %d.\n",
> mp_irq_entries);
> > for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++)
> > @@ -1662,8 +1676,7 @@ __apicdebuginit(void) print_IO_APICs(void)
> > for_each_active_irq(irq) {
> > struct irq_pin_list *entry;
> >
> > - chip = irq_get_chip(irq);
> > - if (chip != &ioapic_chip)
> > + if (!is_ioapic_irq(irq))
> > continue;
> >
> > cfg = irq_get_chip_data(irq);
>
> if the irq is not using ioapic_chip such as msi_chip etc, that irq
> should be skip.
Print MSI chip is not making sense?
--
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