* [PATCH] x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs @ 2012-07-25 9:17 Tomoki Sekiyama 2012-07-25 23:28 ` Siddha, Suresh B 0 siblings, 1 reply; 7+ messages in thread From: Tomoki Sekiyama @ 2012-07-25 9:17 UTC (permalink / raw) To: tglx, mingo Cc: hpa, suresh.b.siddha, yinghai, agordeev, x86, linux-kernel, yrl.pp-manager.tt Hi, In current Linux, percpu variable `vector_irq' is not always cleared when a CPU is offlined. If the cpu that has the disabled irqs in vector_irq is hotplugged again, __setup_vector_irq() hits invalid irq vector and may crash. Commit f6175f5bfb4c partially fixes this, but was not enough in environments with IOMMU IRQ remapper. This bug can be reproduced as following; # echo 0 > /sys/devices/system/cpu/cpu7/online # modprobe -r some_driver_using_interrupts # vector_irq@cpu7 uncleared # echo 1 > /sys/devices/system/cpu/cpu7/online # kernel may crash This patch fixes this bug by clearing vector_irq in __fixup_irqs() when the cpu is offlined. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Suresh Siddha <suresh.b.siddha@intel.com> Cc: Yinghai Lu <yinghai@kernel.org> Cc: Alexander Gordeev <agordeev@redhat.com> --- arch/x86/kernel/irq.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index 3dafc60..d27b27d 100644 --- a/arch/x86/kernel/irq.c +++ b/arch/x86/kernel/irq.c @@ -328,6 +328,7 @@ void fixup_irqs(void) chip->irq_retrigger(data); raw_spin_unlock(&desc->lock); } + __this_cpu_write(vector_irq[vector], -1); } } #endif -- 1.7.7.6 -- Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH] x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs 2012-07-25 9:17 [PATCH] x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs Tomoki Sekiyama @ 2012-07-25 23:28 ` Siddha, Suresh B 2012-07-26 9:38 ` Tomoki Sekiyama 2012-07-26 9:43 ` Tomoki Sekiyama 0 siblings, 2 replies; 7+ messages in thread From: Siddha, Suresh B @ 2012-07-25 23:28 UTC (permalink / raw) To: Tomoki Sekiyama, tglx@linutronix.de, mingo@kernel.org Cc: hpa@zytor.com, yinghai@kernel.org, agordeev@redhat.com, x86@kernel.org, linux-kernel@vger.kernel.org, yrl.pp-manager.tt@hitachi.com Tomoki wrote: > In current Linux, percpu variable `vector_irq' is not always cleared when > a CPU is offlined. If the cpu that has the disabled irqs in vector_irq is > hotplugged again, __setup_vector_irq() hits invalid irq vector and may > crash. > > Commit f6175f5bfb4c partially fixes this, but was not enough in > environments with IOMMU IRQ remapper. So, this patch essentially makes the commit f6175f5bfb4c unnecessary, right? Can you revert that too as part of this new proposed patch? thanks, suresh ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs 2012-07-25 23:28 ` Siddha, Suresh B @ 2012-07-26 9:38 ` Tomoki Sekiyama 2012-07-26 9:43 ` Tomoki Sekiyama 1 sibling, 0 replies; 7+ messages in thread From: Tomoki Sekiyama @ 2012-07-26 9:38 UTC (permalink / raw) To: suresh.b.siddha Cc: tglx, mingo, hpa, yinghai, agordeev, x86, linux-kernel, yrl.pp-manager.tt Hi, thanks for your comment. On 2012/07/26 8:28, Siddha, Suresh B wrote: > Tomoki wrote: >> In current Linux, percpu variable `vector_irq' is not always cleared when >> a CPU is offlined. If the cpu that has the disabled irqs in vector_irq is >> hotplugged again, __setup_vector_irq() hits invalid irq vector and may >> crash. >> >> Commit f6175f5bfb4c partially fixes this, but was not enough in >> environments with IOMMU IRQ remapper. > > So, this patch essentially makes the commit f6175f5bfb4c unnecessary, right? > > Can you revert that too as part of this new proposed patch? > > thanks, > suresh OK, I will include a reverse patch of f6175f5bfb4c and resend the patch. Thanks, -- Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs 2012-07-25 23:28 ` Siddha, Suresh B 2012-07-26 9:38 ` Tomoki Sekiyama @ 2012-07-26 9:43 ` Tomoki Sekiyama 2012-07-26 10:21 ` Ingo Molnar 1 sibling, 1 reply; 7+ messages in thread From: Tomoki Sekiyama @ 2012-07-26 9:43 UTC (permalink / raw) To: tglx, mingo, suresh.b.siddha Cc: hpa, yinghai, agordeev, x86, linux-kernel, yrl.pp-manager.tt In current Linux, percpu variable `vector_irq' is not always cleared when a CPU is offlined. If the CPU that has the disabled irqs in vector_irq is hotplugged again, __setup_vector_irq() hits invalid irq vector and may crash. This bug can be reproduced as following; # echo 0 > /sys/devices/system/cpu/cpu7/online # modprobe -r some_driver_using_interrupts # vector_irq@cpu7 uncleared # echo 1 > /sys/devices/system/cpu/cpu7/online # kernel may crash To fix this problem, this patch clears vector_irq in __fixup_irqs() when the CPU is offlined. This also reverts commit f6175f5bfb4c, which partially fixes this bug by clearing vector in __clear_irq_vector(). But in environments with IOMMU IRQ remapper, it could fail because cfg->domain doesn't contain offlined CPUs. With this patch, the fix in __clear_irq_vector() can be reverted because every vector_irq is already cleared in __fixup_irqs() on offlined CPUs. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Suresh Siddha <suresh.b.siddha@intel.com> Cc: Yinghai Lu <yinghai@kernel.org> Cc: Alexander Gordeev <agordeev@redhat.com> --- arch/x86/kernel/apic/io_apic.c | 4 ++-- arch/x86/kernel/irq.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 5f0ff59..ac96561 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -1195,7 +1195,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg) BUG_ON(!cfg->vector); vector = cfg->vector; - for_each_cpu(cpu, cfg->domain) + for_each_cpu_and(cpu, cfg->domain, cpu_online_mask) per_cpu(vector_irq, cpu)[vector] = -1; cfg->vector = 0; @@ -1203,7 +1203,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg) if (likely(!cfg->move_in_progress)) return; - for_each_cpu(cpu, cfg->old_domain) { + for_each_cpu_and(cpu, cfg->old_domain, cpu_online_mask) { for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) { if (per_cpu(vector_irq, cpu)[vector] != irq) diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index 3dafc60..d27b27d 100644 --- a/arch/x86/kernel/irq.c +++ b/arch/x86/kernel/irq.c @@ -328,6 +328,7 @@ void fixup_irqs(void) chip->irq_retrigger(data); raw_spin_unlock(&desc->lock); } + __this_cpu_write(vector_irq[vector], -1); } } #endif -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs 2012-07-26 9:43 ` Tomoki Sekiyama @ 2012-07-26 10:21 ` Ingo Molnar 2012-07-26 10:47 ` [RESEND PATCH] " Tomoki Sekiyama 0 siblings, 1 reply; 7+ messages in thread From: Ingo Molnar @ 2012-07-26 10:21 UTC (permalink / raw) To: Tomoki Sekiyama Cc: tglx, mingo, suresh.b.siddha, hpa, yinghai, agordeev, x86, linux-kernel, yrl.pp-manager.tt * Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> wrote: > In current Linux, percpu variable `vector_irq' is not always cleared when > a CPU is offlined. If the CPU that has the disabled irqs in vector_irq is > hotplugged again, __setup_vector_irq() hits invalid irq vector and may > crash. > > This bug can be reproduced as following; > # echo 0 > /sys/devices/system/cpu/cpu7/online > # modprobe -r some_driver_using_interrupts # vector_irq@cpu7 uncleared > # echo 1 > /sys/devices/system/cpu/cpu7/online # kernel may crash > > To fix this problem, this patch clears vector_irq in __fixup_irqs() when > the CPU is offlined. > > This also reverts commit f6175f5bfb4c, which partially fixes this bug by > clearing vector in __clear_irq_vector(). But in environments with IOMMU IRQ > remapper, it could fail because cfg->domain doesn't contain offlined CPUs. > With this patch, the fix in __clear_irq_vector() can be reverted because > every vector_irq is already cleared in __fixup_irqs() on offlined CPUs. > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Suresh Siddha <suresh.b.siddha@intel.com> > Cc: Yinghai Lu <yinghai@kernel.org> > Cc: Alexander Gordeev <agordeev@redhat.com> > --- > arch/x86/kernel/apic/io_apic.c | 4 ++-- > arch/x86/kernel/irq.c | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c > index 5f0ff59..ac96561 100644 > --- a/arch/x86/kernel/apic/io_apic.c > +++ b/arch/x86/kernel/apic/io_apic.c > @@ -1195,7 +1195,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg) > BUG_ON(!cfg->vector); > vector = cfg->vector; > - for_each_cpu(cpu, cfg->domain) > + for_each_cpu_and(cpu, cfg->domain, cpu_online_mask) > per_cpu(vector_irq, cpu)[vector] = -1; > cfg->vector = 0; > @@ -1203,7 +1203,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg) > if (likely(!cfg->move_in_progress)) > return; > - for_each_cpu(cpu, cfg->old_domain) { that's not a valid diff - something in your mailer ate lines or such. See Documentation/email-clients.txt. Thanks, Ingo ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RESEND PATCH] x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs 2012-07-26 10:21 ` Ingo Molnar @ 2012-07-26 10:47 ` Tomoki Sekiyama 2012-07-26 15:16 ` [tip:x86/urgent] " tip-bot for Tomoki Sekiyama 0 siblings, 1 reply; 7+ messages in thread From: Tomoki Sekiyama @ 2012-07-26 10:47 UTC (permalink / raw) Cc: linux-kernel, x86, yrl.pp-manager.tt, Tomoki Sekiyama, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Suresh Siddha, Yinghai Lu, Alexander Gordeev (Previous patch was broken, sorry. Resending with another mailer.) In current Linux, percpu variable `vector_irq' is not always cleared when a CPU is offlined. If the CPU that has the disabled irqs in vector_irq is hotplugged again, __setup_vector_irq() hits invalid irq vector and may crash. This bug can be reproduced as following; # echo 0 > /sys/devices/system/cpu/cpu7/online # modprobe -r some_driver_using_interrupts # vector_irq@cpu7 uncleared # echo 1 > /sys/devices/system/cpu/cpu7/online # kernel may crash To fix this problem, this patch clears vector_irq in __fixup_irqs() when the CPU is offlined. This also reverts commit f6175f5bfb4c, which partially fixes this bug by clearing vector in __clear_irq_vector(). But in environments with IOMMU IRQ remapper, it could fail because cfg->domain doesn't contain offlined CPUs. With this patch, the fix in __clear_irq_vector() can be reverted because every vector_irq is already cleared in __fixup_irqs() on offlined CPUs. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Suresh Siddha <suresh.b.siddha@intel.com> Cc: Yinghai Lu <yinghai@kernel.org> Cc: Alexander Gordeev <agordeev@redhat.com> --- arch/x86/kernel/apic/io_apic.c | 4 ++-- arch/x86/kernel/irq.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 5f0ff59..ac96561 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -1195,7 +1195,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg) BUG_ON(!cfg->vector); vector = cfg->vector; - for_each_cpu(cpu, cfg->domain) + for_each_cpu_and(cpu, cfg->domain, cpu_online_mask) per_cpu(vector_irq, cpu)[vector] = -1; cfg->vector = 0; @@ -1203,7 +1203,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg) if (likely(!cfg->move_in_progress)) return; - for_each_cpu(cpu, cfg->old_domain) { + for_each_cpu_and(cpu, cfg->old_domain, cpu_online_mask) { for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) { if (per_cpu(vector_irq, cpu)[vector] != irq) diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index 3dafc60..d27b27d 100644 --- a/arch/x86/kernel/irq.c +++ b/arch/x86/kernel/irq.c @@ -328,6 +328,7 @@ void fixup_irqs(void) chip->irq_retrigger(data); raw_spin_unlock(&desc->lock); } + __this_cpu_write(vector_irq[vector], -1); } } #endif ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:x86/urgent] x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs 2012-07-26 10:47 ` [RESEND PATCH] " Tomoki Sekiyama @ 2012-07-26 15:16 ` tip-bot for Tomoki Sekiyama 0 siblings, 0 replies; 7+ messages in thread From: tip-bot for Tomoki Sekiyama @ 2012-07-26 15:16 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, agordeev, hpa, mingo, yinghai, tomoki.sekiyama.qu, suresh.b.siddha, tglx Commit-ID: 1d44b30f35a9873a65b320dd5300088fa995fd94 Gitweb: http://git.kernel.org/tip/1d44b30f35a9873a65b320dd5300088fa995fd94 Author: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> AuthorDate: Thu, 26 Jul 2012 19:47:32 +0900 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Thu, 26 Jul 2012 15:01:17 +0200 x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs In the current kernel, percpu variable `vector_irq' is not always cleared when a CPU is offlined. If the CPU that has the disabled irqs in vector_irq is hotplugged again, __setup_vector_irq() hits invalid irq vector and may crash. This bug can be reproduced as following; # echo 0 > /sys/devices/system/cpu/cpu7/online # modprobe -r some_driver_using_interrupts # vector_irq@cpu7 uncleared # echo 1 > /sys/devices/system/cpu/cpu7/online # kernel may crash To fix this problem, this patch clears vector_irq in __fixup_irqs() when the CPU is offlined. This also reverts commit f6175f5bfb4c, which partially fixes this bug by clearing vector in __clear_irq_vector(). But in environments with IOMMU IRQ remapper, it could fail because cfg->domain doesn't contain offlined CPUs. With this patch, the fix in __clear_irq_vector() can be reverted because every vector_irq is already cleared in __fixup_irqs() on offlined CPUs. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> Acked-by: Suresh Siddha <suresh.b.siddha@intel.com> Cc: yrl.pp-manager.tt@hitachi.com Cc: Yinghai Lu <yinghai@kernel.org> Cc: Alexander Gordeev <agordeev@redhat.com> Link: http://lkml.kernel.org/r/20120726104732.2889.19144.stgit@kvmdev Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/kernel/apic/io_apic.c | 4 ++-- arch/x86/kernel/irq.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 406eee7..a6c64aa 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -1204,7 +1204,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg) BUG_ON(!cfg->vector); vector = cfg->vector; - for_each_cpu(cpu, cfg->domain) + for_each_cpu_and(cpu, cfg->domain, cpu_online_mask) per_cpu(vector_irq, cpu)[vector] = -1; cfg->vector = 0; @@ -1212,7 +1212,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg) if (likely(!cfg->move_in_progress)) return; - for_each_cpu(cpu, cfg->old_domain) { + for_each_cpu_and(cpu, cfg->old_domain, cpu_online_mask) { for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) { if (per_cpu(vector_irq, cpu)[vector] != irq) diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index 1f5f1d5..7ad683d 100644 --- a/arch/x86/kernel/irq.c +++ b/arch/x86/kernel/irq.c @@ -328,6 +328,7 @@ void fixup_irqs(void) chip->irq_retrigger(data); raw_spin_unlock(&desc->lock); } + __this_cpu_write(vector_irq[vector], -1); } } #endif ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-07-26 15:16 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-25 9:17 [PATCH] x86/ioapic: Fix NULL pointer dereference on CPU hotplug after disabling irqs Tomoki Sekiyama 2012-07-25 23:28 ` Siddha, Suresh B 2012-07-26 9:38 ` Tomoki Sekiyama 2012-07-26 9:43 ` Tomoki Sekiyama 2012-07-26 10:21 ` Ingo Molnar 2012-07-26 10:47 ` [RESEND PATCH] " Tomoki Sekiyama 2012-07-26 15:16 ` [tip:x86/urgent] " tip-bot for Tomoki Sekiyama
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.