lkml.org 
[lkml]   [2014]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86: Plug racy xAPIC access of CPU hotplug code
On 2014-01-27 20:14, Jan Kiszka wrote:
> 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;
> }
>
>

What's the status of this? Waiting for further review, or is it queued
somewhere by now? Would be good to have in 3.14, and then also in stable
kernels.

Jan

[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2014-02-16 10:41    [W:0.175 / U:0.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site