linux-kernel - Re: [PATCH] x86: Plug racy xAPIC access of CPU hotplug code

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 16 Feb 2014 10:02:21 +0100
From:	Jan Kiszka <jan.kiszka@....de>
To:	Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>
CC:	Andi Kleen <andi@...stfloor.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [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@...mens.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


Download attachment "signature.asc" of type "application/pgp-signature" (264 bytes)