From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751959AbcGXPGH (ORCPT ); Sun, 24 Jul 2016 11:06:07 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:36456 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751510AbcGXPGB (ORCPT ); Sun, 24 Jul 2016 11:06:01 -0400 From: Nicolai Stange To: Borislav Petkov Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Nicolai Stange Subject: [PATCH] arch/x86/kernel/cpu/microcode/intel: don't store initrd's start Date: Sun, 24 Jul 2016 17:05:49 +0200 Message-Id: <20160724150549.2833-1-nicstange@gmail.com> X-Mailer: git-send-email 2.9.2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On x86_64 with CONFIG_RANDOMIZE_MEMORY and CONFIG_MICROCODE_INTEL, I get the following splat upon booting on an Intel i7-4800MQ: Call Trace: [] ? find_microcode_patch+0x4a/0xa0 [] load_microcode.isra.1.constprop.12+0x37/0xa0 [...] [] load_ucode_intel_ap+0x5d/0x80 [] load_ucode_ap+0x94/0xa0 [] cpu_init+0x58/0x3e0 [] ? set_pte_vaddr+0x5c/0x90 [] trap_init+0x2b6/0x328 [] start_kernel+0x224/0x47f [] ? early_idt_handler_array+0x120/0x120 [] x86_64_start_reservations+0x29/0x2b [] x86_64_start_kernel+0x14d/0x170 [...] RIP [] has_newer_microcode+0x5/0x20 [...] ---[ end trace b163fd3960fd46fb ]--- Kernel panic - not syncing: Attempted to kill the idle task! ---[ end Kernel panic - not syncing: Attempted to kill the idle task! It can be bisected to commit 21ef9a5c3164 ("Merge branch 'x86/microcode'"). Both of its parents, i.e. commit f5846c92b0a5 ("Merge branch 'x86/headers'") and commit eb06158ee145 ("x86/microcode: Remove unused symbol exports") work fine by themselves. "Cross-bisecting" between v4.7-rc6..f5846c92b0a5 and v4.7-rc6..eb06158ee145 reveals the conflicting commits: commit 021182e52fe0 ("x86/mm: Enable KASLR for physical mapping memory regions") and commit 6c5456474e7f ("x86/microcode: Fix loading precedence"). Thus, the above crash can be explained as follows: 1. The __scan_microcode_initrd() invoked from the early load_ucode_intel_bsp() sets the static blobs.start to the __va of the initrd's start. 2. kernel_randomize_memory() called from setup_arch() renders the just set blobs.start meaningless. 3. load_ucode_ap(), invoked indirectly through trap_init(), adds the now meaningless blobs.start back to ucode offsets in order to get their addresses. Note that blobs.start is redundant in the sense that blobs.valid tells us whether offsets are taken w.r.t to zero or to the initrd's start address. Thus: - Get rid of blobs.start. - Calculate the offset on the fly as needed. Introduce the helper get_ucode_offset() to facilitate this. In the case of ucode blobs taken from the initrd, PAGE_OFFSET is added. This takes into account any KASLR randomization of the physical memory map. Signed-off-by: Nicolai Stange --- A possible ollow up patch might be to turn the blobs struct into a 'static bool ucode_is_from_initrd'? Applicable to next-20160722. On i386, this is compile-tested only. For x86_64, it boots fine (together with the patch from http://lkml.kernel.org/g/tip-530dd8d4b9daf77e3e5d145a26210d91ced954c7@git.kernel.org ). arch/x86/kernel/cpu/microcode/intel.c | 65 ++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 6515c80..3e88077 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -57,10 +57,45 @@ static struct mc_saved_data { /* Microcode blobs within the initrd. 0 if builtin. */ static struct ucode_blobs { - unsigned long start; bool valid; } blobs; +#ifdef CONFIG_X86_32 +static unsigned long get_ucode_offset(bool is_from_initrd) +{ +#ifdef CONFIG_BLK_DEV_INITRD + struct boot_params *params; + + if (!is_from_initrd) + return 0; + + /* We cannot use initrd_start because it is not set that early yet. */ + params = (struct boot_params *)__pa_nodebug(&boot_params); + return params->hdr.ramdisk_image; +#else /* CONFIG_BLK_DEV_INITRD */ + return 0; +#endif +} +#else /* CONFIG_X86_64 */ +static unsigned long get_ucode_offset(bool is_from_initrd) +{ +#ifdef CONFIG_BLK_DEV_INITRD + unsigned long start; + + if (!is_from_initrd) + return 0; + + /* We cannot use initrd_start because it is not set that early yet. */ + start = (u64)boot_params.ext_ramdisk_image << 32; + start |= boot_params.hdr.ramdisk_image; + return (unsigned long)__va(start); + return start + PAGE_OFFSET; +#else /* CONFIG_BLK_DEV_INITRD */ + return 0; +#endif +} +#endif /* CONFIG_X86_32 */ + /* Go through saved patches and find the one suitable for the current CPU. */ static enum ucode_state find_microcode_patch(struct microcode_intel **saved, @@ -687,36 +722,23 @@ __scan_microcode_initrd(struct cpio_data *cd, struct ucode_blobs *blbp) static __initdata char ucode_name[] = "kernel/x86/microcode/GenuineIntel.bin"; char *p = IS_ENABLED(CONFIG_X86_32) ? (char *)__pa_nodebug(ucode_name) : ucode_name; -# ifdef CONFIG_X86_32 unsigned long start = 0, size; + +# ifdef CONFIG_X86_32 struct boot_params *params; params = (struct boot_params *)__pa_nodebug(&boot_params); size = params->hdr.ramdisk_size; - /* - * Set start only if we have an initrd image. We cannot use initrd_start - * because it is not set that early yet. - */ - start = (size ? params->hdr.ramdisk_image : 0); - # else /* CONFIG_X86_64 */ - unsigned long start = 0, size; - size = (u64)boot_params.ext_ramdisk_size << 32; size |= boot_params.hdr.ramdisk_size; - - if (size) { - start = (u64)boot_params.ext_ramdisk_image << 32; - start |= boot_params.hdr.ramdisk_image; - - start += PAGE_OFFSET; - } # endif + /* Set start only if we have an initrd image. */ + start = (size ? get_ucode_offset(true) : 0); *cd = find_cpio_data(p, (void *)start, size, NULL); if (cd->data) { - blbp->start = start; blbp->valid = true; return UCODE_OK; @@ -747,7 +769,8 @@ scan_microcode(struct mc_saved_data *mcs, unsigned long *mc_ptrs, return ret; } - return get_matching_model_microcode(blbp->start, cd.data, cd.size, + return get_matching_model_microcode(get_ucode_offset(blbp->valid), + cd.data, cd.size, mcs, mc_ptrs, uci); } @@ -764,7 +787,7 @@ _load_ucode_intel_bsp(struct mc_saved_data *mcs, unsigned long *mc_ptrs, if (ret != UCODE_OK) return; - ret = load_microcode(mcs, mc_ptrs, blbp->start, &uci); + ret = load_microcode(mcs, mc_ptrs, get_ucode_offset(blbp->valid), &uci); if (ret != UCODE_OK) return; @@ -816,7 +839,7 @@ void load_ucode_intel_ap(void) return; collect_cpu_info_early(&uci); - ret = load_microcode(mcs, ptrs, blobs_p->start, &uci); + ret = load_microcode(mcs, ptrs, get_ucode_offset(blobs_p->valid), &uci); if (ret != UCODE_OK) return; -- 2.9.2