public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Ingo Molnar <mingo@elte.hu>, Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: [PATCH] x86: Plug racy xAPIC access of CPU hotplug code
Date: Mon, 27 Jan 2014 20:14:06 +0100	[thread overview]
Message-ID: <52E6AFFE.3030004@siemens.com> (raw)

apic_icr_write and its users in smpboot.c were apparently written under
the assumption that this code would only run during early boot. But
nowadays we also execute it when onlining a CPU later on while the
system is fully running. That will make wakeup_cpu_via_init_nmi and,
thus, also native_apic_icr_write run in plain process context. If we
migrate the caller to a different CPU at the wrong time or interrupt it
and write to ICR/ICR2 to send unrelated IPIs, we can end up sending
INIT, SIPI or NMIs to wrong CPUs.

Fix this by disabling interrupts during the write to the ICR halves and
disable preemption around waiting for ICR availability and using it.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/kernel/apic/apic.c |  4 ++++
 arch/x86/kernel/smpboot.c   | 11 +++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 7f26c9a..06f90b8 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -283,8 +283,12 @@ u32 native_safe_apic_wait_icr_idle(void)
 
 void native_apic_icr_write(u32 low, u32 id)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
 	apic_write(APIC_ICR2, SET_APIC_DEST_FIELD(id));
 	apic_write(APIC_ICR, low);
+	local_irq_restore(flags);
 }
 
 u64 native_apic_icr_read(void)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index a32da80..37e11e5 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -701,11 +701,15 @@ wakeup_cpu_via_init_nmi(int cpu, unsigned long start_ip, int apicid,
 	int id;
 	int boot_error;
 
+	preempt_disable();
+
 	/*
 	 * Wake up AP by INIT, INIT, STARTUP sequence.
 	 */
-	if (cpu)
-		return wakeup_secondary_cpu_via_init(apicid, start_ip);
+	if (cpu) {
+		boot_error = wakeup_secondary_cpu_via_init(apicid, start_ip);
+		goto out;
+	}
 
 	/*
 	 * Wake up BSP by nmi.
@@ -725,6 +729,9 @@ wakeup_cpu_via_init_nmi(int cpu, unsigned long start_ip, int apicid,
 		boot_error = wakeup_secondary_cpu_via_nmi(id, start_ip);
 	}
 
+out:
+	preempt_enable();
+
 	return boot_error;
 }
 
-- 
1.8.1.1.298.ge7eed54

             reply	other threads:[~2014-01-27 19:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-27 19:14 Jan Kiszka [this message]
2014-01-27 20:22 ` [PATCH] x86: Plug racy xAPIC access of CPU hotplug code Andi Kleen
2014-01-28  8:18   ` Jan Kiszka
2014-01-28 11:55     ` Ingo Molnar
2014-01-28 12:09       ` Jan Kiszka
2014-01-28 21:17         ` Andi Kleen
2014-01-29  8:11           ` Jan Kiszka
2014-02-16  9:02 ` Jan Kiszka
2014-03-06 17:51 ` Igor Mammedov
2014-03-11 12:39 ` [tip:x86/apic] x86/apic: " tip-bot for Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52E6AFFE.3030004@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=andi@firstfloor.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox