* x86/microcode: use-after-free after cpu offline/online @ 2017-01-25 16:58 Andrey Ryabinin 2017-01-25 17:23 ` Borislav Petkov 0 siblings, 1 reply; 19+ messages in thread From: Andrey Ryabinin @ 2017-01-25 16:58 UTC (permalink / raw) To: Borislav Petkov, Thomas Gleixner; +Cc: LKML, H. Peter Anvin, Ingo Molnar On 4.10-rc5 # echo 0 > /sys/devices/system/cpu/cpu1/online # echo 1 > /sys/devices/system/cpu/cpu1/online triggers use-after-free (probably caused by 06b8534cb72 "x86/microcode: Rework microcode loading"). __load_ucode_intel() accesses initrd which is obviously gone at this point: [ 62.347662] ================================================================== [ 62.347670] BUG: KASAN: use-after-free in find_cpio_data+0x779/0x850 at addr ffff880036e75000 [ 62.347672] Read of size 1 by task swapper/1/0 [ 62.347675] page:ffffea0000db9d40 count:0 mapcount:0 mapping: (null) index:0x1 [ 62.347677] flags: 0x100000000000000() [ 62.347680] raw: 0100000000000000 0000000000000000 0000000000000001 00000000ffffffff [ 62.347682] raw: dead000000000100 dead000000000200 0000000000000000 0000000000000000 [ 62.347682] page dumped because: kasan: bad access detected [ 62.347685] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G W 4.10.0-rc5-debug-00075-g2dbde22 #3 [ 62.347686] Hardware name: Dell Inc. XPS 13 9360/0839Y6, BIOS 1.2.3 12/01/2016 [ 62.347687] Call Trace: [ 62.347690] dump_stack+0xb1/0x10c [ 62.347693] ? _atomic_dec_and_lock+0xc4/0xc4 [ 62.347696] ? __dump_page+0x529/0x760 [ 62.347699] kasan_report_error+0x5ba/0x8b0 [ 62.347702] ? pointer+0xe70/0xe70 [ 62.347704] ? find_cpio_data+0x779/0x850 [ 62.347706] __asan_report_load1_noabort+0x59/0x80 [ 62.347708] ? find_cpio_data+0x779/0x850 [ 62.347711] find_cpio_data+0x779/0x850 [ 62.347713] ? vsprintf+0x20/0x20 [ 62.347716] ? dump_stack+0x10c/0x10c [ 62.347718] ? get_ucode_user+0x50/0x50 [ 62.347721] ? print_usage_bug+0x6e0/0x6e0 [ 62.347724] find_microcode_in_initrd+0x25f/0x330 [ 62.347726] __load_ucode_intel+0xde/0x120 [ 62.347728] ? collect_cpu_info_early+0x360/0x360 [ 62.347731] ? debug_check_no_locks_freed+0x330/0x330 [ 62.347733] load_ucode_intel_ap+0x8b/0xc0 [ 62.347735] ? collect_cpu_info+0x4e0/0x4e0 [ 62.347737] ? trace_hardirqs_on+0xd/0x10 [ 62.347740] ? flat_send_IPI_mask_allbutself+0xf0/0xf0 [ 62.347741] load_ucode_ap+0x15d/0x180 [ 62.347743] ? get_builtin_firmware+0x160/0x160 [ 62.347746] ? flush_tlb_func+0x690/0x690 [ 62.347749] ? do_raw_spin_trylock+0x110/0x110 [ 62.347752] ? cpumask_weight+0x50/0x50 [ 62.347754] cpu_init+0x7b7/0x1580 [ 62.347756] ? trace_hardirqs_off+0xd/0x10 [ 62.347758] ? play_dead_common+0x30/0x40 [ 62.347760] ? native_play_dead+0x76/0x1c0 [ 62.347762] ? hlt_play_dead+0x40/0x40 [ 62.347764] ? syscall_init+0x140/0x140 [ 62.347766] ? arch_cpu_idle_dead+0x2d/0x40 [ 62.347769] ? do_idle+0x1da/0x2f0 [ 62.347771] start_secondary+0x14/0x370 [ 62.347774] start_cpu+0x14/0x14 [ 62.347776] Memory state around the buggy address: [ 62.347778] ffff880036e74f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 62.347780] ffff880036e74f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 62.347782] >ffff880036e75000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 62.347783] ^ [ 62.347785] ffff880036e75080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 62.347786] ffff880036e75100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 62.347787] ================================================================== ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: x86/microcode: use-after-free after cpu offline/online 2017-01-25 16:58 x86/microcode: use-after-free after cpu offline/online Andrey Ryabinin @ 2017-01-25 17:23 ` Borislav Petkov 2017-01-25 19:14 ` Andrey Ryabinin 0 siblings, 1 reply; 19+ messages in thread From: Borislav Petkov @ 2017-01-25 17:23 UTC (permalink / raw) To: Andrey Ryabinin; +Cc: Thomas Gleixner, LKML, H. Peter Anvin, Ingo Molnar On Wed, Jan 25, 2017 at 07:58:39PM +0300, Andrey Ryabinin wrote: > On 4.10-rc5 > # echo 0 > /sys/devices/system/cpu/cpu1/online > # echo 1 > /sys/devices/system/cpu/cpu1/online > > triggers use-after-free (probably caused by 06b8534cb72 "x86/microcode: Rework microcode loading"). > > __load_ucode_intel() accesses initrd which is obviously gone at this point: Does that help? --- diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index 2af69d27da62..fdbf8d29ffcf 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -248,8 +248,12 @@ struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa) * possibly relocates the ramdisk. In either case, initrd_start contains * the updated address so use that instead. */ - if (!use_pa && initrd_start) - start = initrd_start; + if (!use_pa) { + if (initrd_start) + start = initrd_start; + else + return (struct cpio_data){ NULL, 0, "" }; + } return find_cpio_data(path, (void *)start, size, NULL); #else /* !CONFIG_BLK_DEV_INITRD */ -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: x86/microcode: use-after-free after cpu offline/online 2017-01-25 17:23 ` Borislav Petkov @ 2017-01-25 19:14 ` Andrey Ryabinin 2017-01-25 19:23 ` Borislav Petkov 0 siblings, 1 reply; 19+ messages in thread From: Andrey Ryabinin @ 2017-01-25 19:14 UTC (permalink / raw) To: Borislav Petkov; +Cc: Thomas Gleixner, LKML, H. Peter Anvin, Ingo Molnar On 01/25/2017 08:23 PM, Borislav Petkov wrote: > On Wed, Jan 25, 2017 at 07:58:39PM +0300, Andrey Ryabinin wrote: >> On 4.10-rc5 >> # echo 0 > /sys/devices/system/cpu/cpu1/online >> # echo 1 > /sys/devices/system/cpu/cpu1/online >> >> triggers use-after-free (probably caused by 06b8534cb72 "x86/microcode: Rework microcode loading"). >> >> __load_ucode_intel() accesses initrd which is obviously gone at this point: > > Does that help? Yup. Tested-by: Andrey Ryabinin <aryabinin@virtuozzo.com> > > --- > diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c > index 2af69d27da62..fdbf8d29ffcf 100644 > --- a/arch/x86/kernel/cpu/microcode/core.c > +++ b/arch/x86/kernel/cpu/microcode/core.c > @@ -248,8 +248,12 @@ struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa) > * possibly relocates the ramdisk. In either case, initrd_start contains > * the updated address so use that instead. > */ > - if (!use_pa && initrd_start) > - start = initrd_start; > + if (!use_pa) { > + if (initrd_start) > + start = initrd_start; > + else > + return (struct cpio_data){ NULL, 0, "" }; > + } > > return find_cpio_data(path, (void *)start, size, NULL); > #else /* !CONFIG_BLK_DEV_INITRD */ > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: x86/microcode: use-after-free after cpu offline/online 2017-01-25 19:14 ` Andrey Ryabinin @ 2017-01-25 19:23 ` Borislav Petkov 2017-01-26 16:58 ` [PATCH] x86/microcode: Do not access the initrd after it has been freed Borislav Petkov 0 siblings, 1 reply; 19+ messages in thread From: Borislav Petkov @ 2017-01-25 19:23 UTC (permalink / raw) To: Andrey Ryabinin; +Cc: Thomas Gleixner, LKML, H. Peter Anvin, Ingo Molnar On Wed, Jan 25, 2017 at 10:14:50PM +0300, Andrey Ryabinin wrote: > > Does that help? > > Yup. > Tested-by: Andrey Ryabinin <aryabinin@virtuozzo.com> Thanks, I'll run it through the test boxes here too. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-25 19:23 ` Borislav Petkov @ 2017-01-26 16:58 ` Borislav Petkov 2017-01-27 8:14 ` Andrey Ryabinin 2017-01-30 8:49 ` [tip:x86/microcode] " tip-bot for Borislav Petkov 0 siblings, 2 replies; 19+ messages in thread From: Borislav Petkov @ 2017-01-26 16:58 UTC (permalink / raw) To: Andrey Ryabinin; +Cc: Thomas Gleixner, LKML, H. Peter Anvin, Ingo Molnar On Wed, Jan 25, 2017 at 08:23:36PM +0100, Borislav Petkov wrote: > On Wed, Jan 25, 2017 at 10:14:50PM +0300, Andrey Ryabinin wrote: > > > Does that help? > > > > Yup. > > Tested-by: Andrey Ryabinin <aryabinin@virtuozzo.com> > > Thanks, I'll run it through the test boxes here too. Yeah, the fix isn't that simple, I had to do a bit more. Please test that one instead. Thanks. --- From: Borislav Petkov <bp@suse.de> Date: Wed, 25 Jan 2017 21:00:48 +0100 Subject: [PATCH] x86/microcode: Do not access the initrd after it has been freed When we look for microcode blobs, we first try builtin and if that doesn't succeed, we fallback to the initrd supplied to the kernel. However, at some point doing boot, that initrd gets jettisoned and we shouldn't access it anymore. But we do, as the below KASAN report shows. That's because find_microcode_in_initrd() doesn't check whether the initrd is still valid or not. So do that. ================================================================== BUG: KASAN: use-after-free in find_cpio_data Read of size 1 by task swapper/1/0 page:ffffea0000db9d40 count:0 mapcount:0 mapping: (null) index:0x1 flags: 0x100000000000000() raw: 0100000000000000 0000000000000000 0000000000000001 00000000ffffffff raw: dead000000000100 dead000000000200 0000000000000000 0000000000000000 page dumped because: kasan: bad access detected CPU: 1 PID: 0 Comm: swapper/1 Tainted: G W 4.10.0-rc5-debug-00075-g2dbde22 #3 Hardware name: Dell Inc. XPS 13 9360/0839Y6, BIOS 1.2.3 12/01/2016 Call Trace: dump_stack ? _atomic_dec_and_lock ? __dump_page kasan_report_error ? pointer ? find_cpio_data __asan_report_load1_noabort ? find_cpio_data find_cpio_data ? vsprintf ? dump_stack ? get_ucode_user ? print_usage_bug find_microcode_in_initrd __load_ucode_intel ? collect_cpu_info_early ? debug_check_no_locks_freed load_ucode_intel_ap ? collect_cpu_info ? trace_hardirqs_on ? flat_send_IPI_mask_allbutself load_ucode_ap ? get_builtin_firmware ? flush_tlb_func ? do_raw_spin_trylock ? cpumask_weight cpu_init ? trace_hardirqs_off ? play_dead_common ? native_play_dead ? hlt_play_dead ? syscall_init ? arch_cpu_idle_dead ? do_idle start_secondary start_cpu Memory state around the buggy address: ffff880036e74f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ffff880036e74f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff >ffff880036e75000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ^ ffff880036e75080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ffff880036e75100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ================================================================== Reported-by: Andrey Ryabinin <aryabinin@virtuozzo.com> Signed-off-by: Borislav Petkov <bp@suse.de> --- arch/x86/include/asm/microcode.h | 1 + arch/x86/kernel/cpu/microcode/amd.c | 5 +++-- arch/x86/kernel/cpu/microcode/core.c | 22 +++++++++++++++++----- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h index 38711df3bcb5..2266f864b747 100644 --- a/arch/x86/include/asm/microcode.h +++ b/arch/x86/include/asm/microcode.h @@ -140,6 +140,7 @@ extern void __init load_ucode_bsp(void); extern void load_ucode_ap(void); void reload_early_microcode(void); extern bool get_builtin_firmware(struct cpio_data *cd, const char *name); +extern bool initrd_gone; #else static inline int __init microcode_init(void) { return 0; }; static inline void __init load_ucode_bsp(void) { } diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c index 6a31e2691f3a..079e81733a58 100644 --- a/arch/x86/kernel/cpu/microcode/amd.c +++ b/arch/x86/kernel/cpu/microcode/amd.c @@ -384,8 +384,9 @@ void load_ucode_amd_ap(unsigned int family) reget: if (!get_builtin_microcode(&cp, family)) { #ifdef CONFIG_BLK_DEV_INITRD - cp = find_cpio_data(ucode_path, (void *)initrd_start, - initrd_end - initrd_start, NULL); + if (!initrd_gone) + cp = find_cpio_data(ucode_path, (void *)initrd_start, + initrd_end - initrd_start, NULL); #endif if (!(cp.data && cp.size)) { /* diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index 2af69d27da62..73102d932760 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -46,6 +46,8 @@ static struct microcode_ops *microcode_ops; static bool dis_ucode_ldr = true; +bool initrd_gone; + LIST_HEAD(microcode_cache); /* @@ -190,21 +192,24 @@ void load_ucode_ap(void) static int __init save_microcode_in_initrd(void) { struct cpuinfo_x86 *c = &boot_cpu_data; + int ret = -EINVAL; switch (c->x86_vendor) { case X86_VENDOR_INTEL: if (c->x86 >= 6) - return save_microcode_in_initrd_intel(); + ret = save_microcode_in_initrd_intel(); break; case X86_VENDOR_AMD: if (c->x86 >= 0x10) - return save_microcode_in_initrd_amd(c->x86); + ret = save_microcode_in_initrd_amd(c->x86); break; default: break; } - return -EINVAL; + initrd_gone = true; + + return ret; } struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa) @@ -247,9 +252,16 @@ struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa) * has the virtual address of the beginning of the initrd. It also * possibly relocates the ramdisk. In either case, initrd_start contains * the updated address so use that instead. + * + * initrd_gone is for the hotplug case where we've thrown out initrd + * already. */ - if (!use_pa && initrd_start) - start = initrd_start; + if (!use_pa) { + if (initrd_gone) + return (struct cpio_data){ NULL, 0, "" }; + if (initrd_start) + start = initrd_start; + } return find_cpio_data(path, (void *)start, size, NULL); #else /* !CONFIG_BLK_DEV_INITRD */ -- 2.11.0 -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-26 16:58 ` [PATCH] x86/microcode: Do not access the initrd after it has been freed Borislav Petkov @ 2017-01-27 8:14 ` Andrey Ryabinin 2017-01-27 9:09 ` Borislav Petkov 2017-01-30 8:49 ` [tip:x86/microcode] " tip-bot for Borislav Petkov 1 sibling, 1 reply; 19+ messages in thread From: Andrey Ryabinin @ 2017-01-27 8:14 UTC (permalink / raw) To: Borislav Petkov; +Cc: Thomas Gleixner, LKML, H. Peter Anvin, Ingo Molnar On 01/26/2017 07:58 PM, Borislav Petkov wrote: > On Wed, Jan 25, 2017 at 08:23:36PM +0100, Borislav Petkov wrote: >> On Wed, Jan 25, 2017 at 10:14:50PM +0300, Andrey Ryabinin wrote: >>>> Does that help? >>> >>> Yup. >>> Tested-by: Andrey Ryabinin <aryabinin@virtuozzo.com> >> >> Thanks, I'll run it through the test boxes here too. > > Yeah, the fix isn't that simple, I had to do a bit more. Please test > that one instead. > Works for me as well. Tested-by: Andrey Ryabinin <aryabinin@virtuozzo.com> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-27 8:14 ` Andrey Ryabinin @ 2017-01-27 9:09 ` Borislav Petkov 2017-01-30 8:46 ` Ingo Molnar 0 siblings, 1 reply; 19+ messages in thread From: Borislav Petkov @ 2017-01-27 9:09 UTC (permalink / raw) To: Andrey Ryabinin; +Cc: Thomas Gleixner, LKML, H. Peter Anvin, Ingo Molnar On Fri, Jan 27, 2017 at 11:14:06AM +0300, Andrey Ryabinin wrote: > Works for me as well. > Tested-by: Andrey Ryabinin <aryabinin@virtuozzo.com> Thanks! -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-27 9:09 ` Borislav Petkov @ 2017-01-30 8:46 ` Ingo Molnar 2017-01-30 9:35 ` Borislav Petkov 0 siblings, 1 reply; 19+ messages in thread From: Ingo Molnar @ 2017-01-30 8:46 UTC (permalink / raw) To: Borislav Petkov; +Cc: Andrey Ryabinin, Thomas Gleixner, LKML, H. Peter Anvin * Borislav Petkov <bp@alien8.de> wrote: > On Fri, Jan 27, 2017 at 11:14:06AM +0300, Andrey Ryabinin wrote: > > Works for me as well. > > Tested-by: Andrey Ryabinin <aryabinin@virtuozzo.com> > > Thanks! Ok, I have applied this to tip:x86/urgent. Note that there are new conflicts with your pending work in tip:x86/microcode, and I fixed them up in: 7c5b4112040e Merge branch 'x86/urgent' into x86/microcode, to resolve conflicts Could you please double-check my conflict resolution? Thanks, Ingo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-30 8:46 ` Ingo Molnar @ 2017-01-30 9:35 ` Borislav Petkov 2017-01-31 7:43 ` Ingo Molnar 0 siblings, 1 reply; 19+ messages in thread From: Borislav Petkov @ 2017-01-30 9:35 UTC (permalink / raw) To: Ingo Molnar; +Cc: Andrey Ryabinin, Thomas Gleixner, LKML, H. Peter Anvin On Mon, Jan 30, 2017 at 09:46:32AM +0100, Ingo Molnar wrote: > Ok, I have applied this to tip:x86/urgent. > > Note that there are new conflicts with your pending work in tip:x86/microcode, and > I fixed them up in: > > 7c5b4112040e Merge branch 'x86/urgent' into x86/microcode, to resolve conflicts > > Could you please double-check my conflict resolution? Almost, this part is wrong: --------------------- arch/x86/kernel/cpu/microcode/amd.c --------------------- index 7889ae492af0,079e81733a58..73082365ed1c @@@ -268,20 -316,43 +268,20 @@@ void __load_ucode_amd(unsigned int cpui use_pa = false; } - if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) - if (!get_builtin_microcode(&cp, family)) ++ if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)) && !initrd_gone) cp = find_microcode_in_initrd(path, use_pa); -- Btw, I did experiment with the merging because I knew it'll cause trouble due to the urgent fix and here's what I did: You're merging tip/x86/urgent into tip/x86/microcode so I checked out the microcode branch and did: $ git checkout -b tip-microcode tip/x86/microcode $ git merge -s recursive -X ours tip/x86/urgent This way I'm favouring our changes in the conflicting files. It merges cleanly and the resulting diff is below. The logic behind it is is that tip/x86/microcode does away with a bunch of code and the urgent change touches some of that code but that's only for 4.10. It goes away in 4.11 and that's why we should prefer "ours" as the merge option. [ Btw, I'll send a patch for 4.11 later to make initrd_gone static as it is going to be used only in microcode/core.c after the cleanup. ] However, I still haven't figured out how to say "prefer ours but only for specific files or subtree" because the diff has that hunk in arch/x86/kernel/fpu/core.c too which should definitely not be "ours" as it is a fix and there the urgent version should be the one going in. Hmmm. --- $ git diff HEAD~1.. diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h index 90b22bbdfce9..daadeeea00b1 100644 --- a/arch/x86/include/asm/microcode.h +++ b/arch/x86/include/asm/microcode.h @@ -139,6 +139,7 @@ extern void __init load_ucode_bsp(void); extern void load_ucode_ap(void); void reload_early_microcode(void); extern bool get_builtin_firmware(struct cpio_data *cd, const char *name); +extern bool initrd_gone; #else static inline int __init microcode_init(void) { return 0; }; static inline void __init load_ucode_bsp(void) { } diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index 3b74d2f315d3..b4a4cd39b358 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -46,6 +46,8 @@ static struct microcode_ops *microcode_ops; static bool dis_ucode_ldr = true; +bool initrd_gone; + LIST_HEAD(microcode_cache); /* @@ -219,11 +221,12 @@ void load_ucode_ap(void) static int __init save_microcode_in_initrd(void) { struct cpuinfo_x86 *c = &boot_cpu_data; + int ret = -EINVAL; switch (c->x86_vendor) { case X86_VENDOR_INTEL: if (c->x86 >= 6) - return save_microcode_in_initrd_intel(); + ret = save_microcode_in_initrd_intel(); break; case X86_VENDOR_AMD: if (c->x86 >= 0x10) @@ -233,7 +236,9 @@ static int __init save_microcode_in_initrd(void) break; } - return -EINVAL; + initrd_gone = true; + + return ret; } struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa) @@ -276,9 +281,16 @@ struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa) * has the virtual address of the beginning of the initrd. It also * possibly relocates the ramdisk. In either case, initrd_start contains * the updated address so use that instead. + * + * initrd_gone is for the hotplug case where we've thrown out initrd + * already. */ - if (!use_pa && initrd_start) - start = initrd_start; + if (!use_pa) { + if (initrd_gone) + return (struct cpio_data){ NULL, 0, "" }; + if (initrd_start) + start = initrd_start; + } return find_cpio_data(path, (void *)start, size, NULL); #else /* !CONFIG_BLK_DEV_INITRD */ diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index e4e97a5355ce..de7234401275 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -9,6 +9,7 @@ #include <asm/fpu/regset.h> #include <asm/fpu/signal.h> #include <asm/fpu/types.h> +#include <asm/fpu/xstate.h> #include <asm/traps.h> #include <linux/hardirq.h> @@ -183,7 +184,8 @@ void fpstate_init(union fpregs_state *state) * it will #GP. Make sure it is replaced after the memset(). */ if (static_cpu_has(X86_FEATURE_XSAVES)) - state->xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT; + state->xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | + xfeatures_mask; if (static_cpu_has(X86_FEATURE_FXSR)) fpstate_init_fxstate(&state->fxsave); -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-30 9:35 ` Borislav Petkov @ 2017-01-31 7:43 ` Ingo Molnar 2017-01-31 10:01 ` Borislav Petkov 0 siblings, 1 reply; 19+ messages in thread From: Ingo Molnar @ 2017-01-31 7:43 UTC (permalink / raw) To: Borislav Petkov, Mike Galbraith Cc: Andrey Ryabinin, Thomas Gleixner, LKML, H. Peter Anvin (Cc:-ed Mike as this could explain his early boot crash/hang? Mike: please try -tip f18a8a0143b1 that I just pushed out. ) * Borislav Petkov <bp@alien8.de> wrote: > On Mon, Jan 30, 2017 at 09:46:32AM +0100, Ingo Molnar wrote: > > Ok, I have applied this to tip:x86/urgent. > > > > Note that there are new conflicts with your pending work in tip:x86/microcode, and > > I fixed them up in: > > > > 7c5b4112040e Merge branch 'x86/urgent' into x86/microcode, to resolve conflicts > > > > Could you please double-check my conflict resolution? > > Almost, this part is wrong: > > --------------------- arch/x86/kernel/cpu/microcode/amd.c --------------------- > index 7889ae492af0,079e81733a58..73082365ed1c > @@@ -268,20 -316,43 +268,20 @@@ void __load_ucode_amd(unsigned int cpui > use_pa = false; > } > > - if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) > - if (!get_builtin_microcode(&cp, family)) > ++ if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)) && !initrd_gone) > cp = find_microcode_in_initrd(path, use_pa); > > -- > > Btw, I did experiment with the merging because I knew it'll cause > trouble due to the urgent fix and here's what I did: > > You're merging tip/x86/urgent into tip/x86/microcode so I checked out > the microcode branch and did: > > $ git checkout -b tip-microcode tip/x86/microcode > $ git merge -s recursive -X ours tip/x86/urgent > > This way I'm favouring our changes in the conflicting files. It merges > cleanly and the resulting diff is below. Nice - I've updated the branch with your resolution. Could you please double-check the double checked resolution? > The logic behind it is is that tip/x86/microcode does away with a bunch > of code and the urgent change touches some of that code but that's only > for 4.10. > > It goes away in 4.11 and that's why we should prefer "ours" as the merge > option. > > [ Btw, I'll send a patch for 4.11 later to make initrd_gone static as > it is going to be used only in microcode/core.c after the cleanup. ] > > However, I still haven't figured out how to say "prefer ours but only > for specific files or subtree" because the diff has that hunk in > arch/x86/kernel/fpu/core.c too which should definitely not be "ours" as > it is a fix and there the urgent version should be the one going in. > > Hmmm. So the diff between your resolution and mine is attached below - now fpu/core.c changes, so I'm not sure why fpu/core.c is in your diff? Thanks, Ingo diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c index 73082365ed1c..7889ae492af0 100644 --- a/arch/x86/kernel/cpu/microcode/amd.c +++ b/arch/x86/kernel/cpu/microcode/amd.c @@ -268,7 +268,7 @@ void __load_ucode_amd(unsigned int cpuid_1_eax, struct cpio_data *ret) use_pa = false; } - if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)) && !initrd_gone) + if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) cp = find_microcode_in_initrd(path, use_pa); /* Needed in load_microcode_amd() */ diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index e51eeaed8016..b4a4cd39b358 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -230,7 +230,7 @@ static int __init save_microcode_in_initrd(void) break; case X86_VENDOR_AMD: if (c->x86 >= 0x10) - ret = save_microcode_in_initrd_amd(cpuid_eax(1)); + return save_microcode_in_initrd_amd(cpuid_eax(1)); break; default: break; ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-31 7:43 ` Ingo Molnar @ 2017-01-31 10:01 ` Borislav Petkov 2017-01-31 11:31 ` Mike Galbraith 2017-01-31 16:39 ` [PATCH] x86/microcode: Do not access the initrd after it has been freed Ingo Molnar 0 siblings, 2 replies; 19+ messages in thread From: Borislav Petkov @ 2017-01-31 10:01 UTC (permalink / raw) To: Ingo Molnar Cc: Mike Galbraith, Andrey Ryabinin, Thomas Gleixner, LKML, H. Peter Anvin On Tue, Jan 31, 2017 at 08:43:55AM +0100, Ingo Molnar wrote: > (Cc:-ed Mike as this could explain his early boot crash/hang? > Mike: please try -tip f18a8a0143b1 that I just pushed out. ) One other thing to try, Mike, is boot with "dis_ucode_ldr". See whether that makes it go away. > Nice - I've updated the branch with your resolution. Could you please > double-check the double checked resolution? That's like quadruple-checked :-) > So the diff between your resolution and mine is attached below - now fpu/core.c > changes, so I'm not sure why fpu/core.c is in your diff? Hmm, maybe I did the diff wrong? So this is how I generated it: $ git diff tip/x86/microcode~1..tip/x86/microcode meaning that I want to see what the merge commit changed. That's why this pulled in the fpu/core.c hunk which is new in urgent. In any case, the merge looks ok to me, I'll be testing it anyway in the coming days so I'll let you know if anything goes nuts. Thanks! -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-31 10:01 ` Borislav Petkov @ 2017-01-31 11:31 ` Mike Galbraith 2017-01-31 12:31 ` Borislav Petkov 2017-01-31 18:03 ` Thomas Gleixner 2017-01-31 16:39 ` [PATCH] x86/microcode: Do not access the initrd after it has been freed Ingo Molnar 1 sibling, 2 replies; 19+ messages in thread From: Mike Galbraith @ 2017-01-31 11:31 UTC (permalink / raw) To: Borislav Petkov, Ingo Molnar Cc: Andrey Ryabinin, Thomas Gleixner, LKML, H. Peter Anvin On Tue, 2017-01-31 at 11:01 +0100, Borislav Petkov wrote: > On Tue, Jan 31, 2017 at 08:43:55AM +0100, Ingo Molnar wrote: > > (Cc:-ed Mike as this could explain his early boot crash/hang? > > Mike: please try -tip f18a8a0143b1 that I just pushed out. ) > > One other thing to try, Mike, is boot with "dis_ucode_ldr". See whether > that makes it go away. (bisect fingered irqdomain: Avoid activating interrupts more than once) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-31 11:31 ` Mike Galbraith @ 2017-01-31 12:31 ` Borislav Petkov 2017-01-31 17:49 ` Borislav Petkov 2017-01-31 18:03 ` Thomas Gleixner 1 sibling, 1 reply; 19+ messages in thread From: Borislav Petkov @ 2017-01-31 12:31 UTC (permalink / raw) To: Mike Galbraith Cc: Ingo Molnar, Andrey Ryabinin, Thomas Gleixner, LKML, H. Peter Anvin On Tue, Jan 31, 2017 at 12:31:17PM +0100, Mike Galbraith wrote: > (bisect fingered irqdomain: Avoid activating interrupts more than once) Yeah, that one is not kosher on x86. It broke IO-APIC timer on a box here. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-31 12:31 ` Borislav Petkov @ 2017-01-31 17:49 ` Borislav Petkov 2017-01-31 18:05 ` Mike Galbraith 0 siblings, 1 reply; 19+ messages in thread From: Borislav Petkov @ 2017-01-31 17:49 UTC (permalink / raw) To: Mike Galbraith Cc: Ingo Molnar, Andrey Ryabinin, Thomas Gleixner, LKML, H. Peter Anvin On Tue, Jan 31, 2017 at 01:31:00PM +0100, Borislav Petkov wrote: > On Tue, Jan 31, 2017 at 12:31:17PM +0100, Mike Galbraith wrote: > > (bisect fingered irqdomain: Avoid activating interrupts more than once) > > Yeah, that one is not kosher on x86. It broke IO-APIC timer on a box > here. Mike, does the below hunk fix the issue for ya? (Ontop of tip/master, without the revert). It does fix my APIC timer detection failure. --- diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 1e35dd06b090..52f352b063fd 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2117,6 +2117,7 @@ static inline void __init check_timer(void) if (idx != -1 && irq_trigger(idx)) unmask_ioapic_irq(irq_get_chip_data(0)); } + irq_domain_deactivate_irq(irq_data); irq_domain_activate_irq(irq_data); if (timer_irq_works()) { if (disable_timer_pin_1 > 0) @@ -2138,6 +2139,7 @@ static inline void __init check_timer(void) * legacy devices should be connected to IO APIC #0 */ replace_pin_at_irq_node(data, node, apic1, pin1, apic2, pin2); + irq_domain_deactivate_irq(irq_data); irq_domain_activate_irq(irq_data); legacy_pic->unmask(0); if (timer_irq_works()) { diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index 85e87b46c318..dc6ba5bda9fc 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -352,6 +352,7 @@ static int hpet_resume(struct clock_event_device *evt, int timer) } else { struct hpet_dev *hdev = EVT_TO_HPET_DEV(evt); + irq_domain_deactivate_irq(irq_get_irq_data(hdev->irq)); irq_domain_activate_irq(irq_get_irq_data(hdev->irq)); disable_irq(hdev->irq); irq_set_affinity(hdev->irq, cpumask_of(hdev->cpu)); -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-31 17:49 ` Borislav Petkov @ 2017-01-31 18:05 ` Mike Galbraith 0 siblings, 0 replies; 19+ messages in thread From: Mike Galbraith @ 2017-01-31 18:05 UTC (permalink / raw) To: Borislav Petkov Cc: Ingo Molnar, Andrey Ryabinin, Thomas Gleixner, LKML, H. Peter Anvin On Tue, 2017-01-31 at 18:49 +0100, Borislav Petkov wrote: > On Tue, Jan 31, 2017 at 01:31:00PM +0100, Borislav Petkov wrote: > > On Tue, Jan 31, 2017 at 12:31:17PM +0100, Mike Galbraith wrote: > > > (bisect fingered irqdomain: Avoid activating interrupts more than once) > > > > Yeah, that one is not kosher on x86. It broke IO-APIC timer on a box > > here. > > Mike, > > does the below hunk fix the issue for ya? (Ontop of tip/master, without > the revert). > > It does fix my APIC timer detection failure. Yup, need a new doorstop. -Mike ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-31 11:31 ` Mike Galbraith 2017-01-31 12:31 ` Borislav Petkov @ 2017-01-31 18:03 ` Thomas Gleixner 2017-01-31 19:25 ` [tip:irq/urgent] x86/irq: Make irq activate operations symmetric tip-bot for Thomas Gleixner 1 sibling, 1 reply; 19+ messages in thread From: Thomas Gleixner @ 2017-01-31 18:03 UTC (permalink / raw) To: Mike Galbraith Cc: Borislav Petkov, Ingo Molnar, Andrey Ryabinin, LKML, H. Peter Anvin On Tue, 31 Jan 2017, Mike Galbraith wrote: > On Tue, 2017-01-31 at 11:01 +0100, Borislav Petkov wrote: > > On Tue, Jan 31, 2017 at 08:43:55AM +0100, Ingo Molnar wrote: > > > (Cc:-ed Mike as this could explain his early boot crash/hang? > > > Mike: please try -tip f18a8a0143b1 that I just pushed out. ) > > > > One other thing to try, Mike, is boot with "dis_ucode_ldr". See whether > > that makes it go away. > > (bisect fingered irqdomain: Avoid activating interrupts more than once) That commit exposed interesting code in x86 land. Can you please try the patch below? Thanks, tglx 8<-------------- diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 1e35dd06b090..52f352b063fd 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2117,6 +2117,7 @@ static inline void __init check_timer(void) if (idx != -1 && irq_trigger(idx)) unmask_ioapic_irq(irq_get_chip_data(0)); } + irq_domain_deactivate_irq(irq_data); irq_domain_activate_irq(irq_data); if (timer_irq_works()) { if (disable_timer_pin_1 > 0) @@ -2138,6 +2139,7 @@ static inline void __init check_timer(void) * legacy devices should be connected to IO APIC #0 */ replace_pin_at_irq_node(data, node, apic1, pin1, apic2, pin2); + irq_domain_deactivate_irq(irq_data); irq_domain_activate_irq(irq_data); legacy_pic->unmask(0); if (timer_irq_works()) { diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index 85e87b46c318..dc6ba5bda9fc 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -352,6 +352,7 @@ static int hpet_resume(struct clock_event_device *evt, int timer) } else { struct hpet_dev *hdev = EVT_TO_HPET_DEV(evt); + irq_domain_deactivate_irq(irq_get_irq_data(hdev->irq)); irq_domain_activate_irq(irq_get_irq_data(hdev->irq)); disable_irq(hdev->irq); irq_set_affinity(hdev->irq, cpumask_of(hdev->cpu)); ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [tip:irq/urgent] x86/irq: Make irq activate operations symmetric 2017-01-31 18:03 ` Thomas Gleixner @ 2017-01-31 19:25 ` tip-bot for Thomas Gleixner 0 siblings, 0 replies; 19+ messages in thread From: tip-bot for Thomas Gleixner @ 2017-01-31 19:25 UTC (permalink / raw) To: linux-tip-commits Cc: mingo, hpa, linux-kernel, efault, aryabinin, bp, tglx, marc.zyngier Commit-ID: aaaec6fc755447a1d056765b11b24d8ff2b81366 Gitweb: http://git.kernel.org/tip/aaaec6fc755447a1d056765b11b24d8ff2b81366 Author: Thomas Gleixner <tglx@linutronix.de> AuthorDate: Tue, 31 Jan 2017 19:03:21 +0100 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Tue, 31 Jan 2017 20:22:18 +0100 x86/irq: Make irq activate operations symmetric The recent commit which prevents double activation of interrupts unearthed interesting code in x86. The code (ab)uses irq_domain_activate_irq() to reconfigure an already activated interrupt. That trips over the prevention code now. Fix it by deactivating the interrupt before activating the new configuration. Fixes: 08d85f3ea99f1 "irqdomain: Avoid activating interrupts more than once" Reported-and-tested-by: Mike Galbraith <efault@gmx.de> Reported-and-tested-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: stable@vger.kernel.org Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1701311901580.3457@nanos --- arch/x86/kernel/apic/io_apic.c | 2 ++ arch/x86/kernel/hpet.c | 1 + 2 files changed, 3 insertions(+) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 1e35dd0..52f352b 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2117,6 +2117,7 @@ static inline void __init check_timer(void) if (idx != -1 && irq_trigger(idx)) unmask_ioapic_irq(irq_get_chip_data(0)); } + irq_domain_deactivate_irq(irq_data); irq_domain_activate_irq(irq_data); if (timer_irq_works()) { if (disable_timer_pin_1 > 0) @@ -2138,6 +2139,7 @@ static inline void __init check_timer(void) * legacy devices should be connected to IO APIC #0 */ replace_pin_at_irq_node(data, node, apic1, pin1, apic2, pin2); + irq_domain_deactivate_irq(irq_data); irq_domain_activate_irq(irq_data); legacy_pic->unmask(0); if (timer_irq_works()) { diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index 85e87b4..dc6ba5b 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -352,6 +352,7 @@ static int hpet_resume(struct clock_event_device *evt, int timer) } else { struct hpet_dev *hdev = EVT_TO_HPET_DEV(evt); + irq_domain_deactivate_irq(irq_get_irq_data(hdev->irq)); irq_domain_activate_irq(irq_get_irq_data(hdev->irq)); disable_irq(hdev->irq); irq_set_affinity(hdev->irq, cpumask_of(hdev->cpu)); ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed 2017-01-31 10:01 ` Borislav Petkov 2017-01-31 11:31 ` Mike Galbraith @ 2017-01-31 16:39 ` Ingo Molnar 1 sibling, 0 replies; 19+ messages in thread From: Ingo Molnar @ 2017-01-31 16:39 UTC (permalink / raw) To: Borislav Petkov Cc: Mike Galbraith, Andrey Ryabinin, Thomas Gleixner, LKML, H. Peter Anvin * Borislav Petkov <bp@alien8.de> wrote: > On Tue, Jan 31, 2017 at 08:43:55AM +0100, Ingo Molnar wrote: > > (Cc:-ed Mike as this could explain his early boot crash/hang? > > Mike: please try -tip f18a8a0143b1 that I just pushed out. ) > > One other thing to try, Mike, is boot with "dis_ucode_ldr". See whether > that makes it go away. > > > Nice - I've updated the branch with your resolution. Could you please > > double-check the double checked resolution? > > That's like quadruple-checked :-) Yeah, so when I wrote that I was pondering whether that counts as triple or quadruple checked. I couldn't make up my mind ;-) Thanks, Ingo ^ permalink raw reply [flat|nested] 19+ messages in thread
* [tip:x86/microcode] x86/microcode: Do not access the initrd after it has been freed 2017-01-26 16:58 ` [PATCH] x86/microcode: Do not access the initrd after it has been freed Borislav Petkov 2017-01-27 8:14 ` Andrey Ryabinin @ 2017-01-30 8:49 ` tip-bot for Borislav Petkov 1 sibling, 0 replies; 19+ messages in thread From: tip-bot for Borislav Petkov @ 2017-01-30 8:49 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, tglx, aryabinin, torvalds, peterz, bp Commit-ID: 24c2503255d35c269b67162c397a1a1c1e02f6ce Gitweb: http://git.kernel.org/tip/24c2503255d35c269b67162c397a1a1c1e02f6ce Author: Borislav Petkov <bp@suse.de> AuthorDate: Wed, 25 Jan 2017 21:00:48 +0100 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Mon, 30 Jan 2017 09:32:42 +0100 x86/microcode: Do not access the initrd after it has been freed When we look for microcode blobs, we first try builtin and if that doesn't succeed, we fallback to the initrd supplied to the kernel. However, at some point doing boot, that initrd gets jettisoned and we shouldn't access it anymore. But we do, as the below KASAN report shows. That's because find_microcode_in_initrd() doesn't check whether the initrd is still valid or not. So do that. ================================================================== BUG: KASAN: use-after-free in find_cpio_data Read of size 1 by task swapper/1/0 page:ffffea0000db9d40 count:0 mapcount:0 mapping: (null) index:0x1 flags: 0x100000000000000() raw: 0100000000000000 0000000000000000 0000000000000001 00000000ffffffff raw: dead000000000100 dead000000000200 0000000000000000 0000000000000000 page dumped because: kasan: bad access detected CPU: 1 PID: 0 Comm: swapper/1 Tainted: G W 4.10.0-rc5-debug-00075-g2dbde22 #3 Hardware name: Dell Inc. XPS 13 9360/0839Y6, BIOS 1.2.3 12/01/2016 Call Trace: dump_stack ? _atomic_dec_and_lock ? __dump_page kasan_report_error ? pointer ? find_cpio_data __asan_report_load1_noabort ? find_cpio_data find_cpio_data ? vsprintf ? dump_stack ? get_ucode_user ? print_usage_bug find_microcode_in_initrd __load_ucode_intel ? collect_cpu_info_early ? debug_check_no_locks_freed load_ucode_intel_ap ? collect_cpu_info ? trace_hardirqs_on ? flat_send_IPI_mask_allbutself load_ucode_ap ? get_builtin_firmware ? flush_tlb_func ? do_raw_spin_trylock ? cpumask_weight cpu_init ? trace_hardirqs_off ? play_dead_common ? native_play_dead ? hlt_play_dead ? syscall_init ? arch_cpu_idle_dead ? do_idle start_secondary start_cpu Memory state around the buggy address: ffff880036e74f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ffff880036e74f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff >ffff880036e75000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ^ ffff880036e75080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ffff880036e75100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ================================================================== Reported-by: Andrey Ryabinin <aryabinin@virtuozzo.com> Tested-by: Andrey Ryabinin <aryabinin@virtuozzo.com> Signed-off-by: Borislav Petkov <bp@suse.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/20170126165833.evjemhbqzaepirxo@pd.tnic Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/microcode.h | 1 + arch/x86/kernel/cpu/microcode/amd.c | 5 +++-- arch/x86/kernel/cpu/microcode/core.c | 22 +++++++++++++++++----- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h index 38711df..2266f86 100644 --- a/arch/x86/include/asm/microcode.h +++ b/arch/x86/include/asm/microcode.h @@ -140,6 +140,7 @@ extern void __init load_ucode_bsp(void); extern void load_ucode_ap(void); void reload_early_microcode(void); extern bool get_builtin_firmware(struct cpio_data *cd, const char *name); +extern bool initrd_gone; #else static inline int __init microcode_init(void) { return 0; }; static inline void __init load_ucode_bsp(void) { } diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c index 6a31e26..079e817 100644 --- a/arch/x86/kernel/cpu/microcode/amd.c +++ b/arch/x86/kernel/cpu/microcode/amd.c @@ -384,8 +384,9 @@ void load_ucode_amd_ap(unsigned int family) reget: if (!get_builtin_microcode(&cp, family)) { #ifdef CONFIG_BLK_DEV_INITRD - cp = find_cpio_data(ucode_path, (void *)initrd_start, - initrd_end - initrd_start, NULL); + if (!initrd_gone) + cp = find_cpio_data(ucode_path, (void *)initrd_start, + initrd_end - initrd_start, NULL); #endif if (!(cp.data && cp.size)) { /* diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index 2af69d2..73102d9 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -46,6 +46,8 @@ static struct microcode_ops *microcode_ops; static bool dis_ucode_ldr = true; +bool initrd_gone; + LIST_HEAD(microcode_cache); /* @@ -190,21 +192,24 @@ void load_ucode_ap(void) static int __init save_microcode_in_initrd(void) { struct cpuinfo_x86 *c = &boot_cpu_data; + int ret = -EINVAL; switch (c->x86_vendor) { case X86_VENDOR_INTEL: if (c->x86 >= 6) - return save_microcode_in_initrd_intel(); + ret = save_microcode_in_initrd_intel(); break; case X86_VENDOR_AMD: if (c->x86 >= 0x10) - return save_microcode_in_initrd_amd(c->x86); + ret = save_microcode_in_initrd_amd(c->x86); break; default: break; } - return -EINVAL; + initrd_gone = true; + + return ret; } struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa) @@ -247,9 +252,16 @@ struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa) * has the virtual address of the beginning of the initrd. It also * possibly relocates the ramdisk. In either case, initrd_start contains * the updated address so use that instead. + * + * initrd_gone is for the hotplug case where we've thrown out initrd + * already. */ - if (!use_pa && initrd_start) - start = initrd_start; + if (!use_pa) { + if (initrd_gone) + return (struct cpio_data){ NULL, 0, "" }; + if (initrd_start) + start = initrd_start; + } return find_cpio_data(path, (void *)start, size, NULL); #else /* !CONFIG_BLK_DEV_INITRD */ ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-01-31 19:26 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-25 16:58 x86/microcode: use-after-free after cpu offline/online Andrey Ryabinin 2017-01-25 17:23 ` Borislav Petkov 2017-01-25 19:14 ` Andrey Ryabinin 2017-01-25 19:23 ` Borislav Petkov 2017-01-26 16:58 ` [PATCH] x86/microcode: Do not access the initrd after it has been freed Borislav Petkov 2017-01-27 8:14 ` Andrey Ryabinin 2017-01-27 9:09 ` Borislav Petkov 2017-01-30 8:46 ` Ingo Molnar 2017-01-30 9:35 ` Borislav Petkov 2017-01-31 7:43 ` Ingo Molnar 2017-01-31 10:01 ` Borislav Petkov 2017-01-31 11:31 ` Mike Galbraith 2017-01-31 12:31 ` Borislav Petkov 2017-01-31 17:49 ` Borislav Petkov 2017-01-31 18:05 ` Mike Galbraith 2017-01-31 18:03 ` Thomas Gleixner 2017-01-31 19:25 ` [tip:irq/urgent] x86/irq: Make irq activate operations symmetric tip-bot for Thomas Gleixner 2017-01-31 16:39 ` [PATCH] x86/microcode: Do not access the initrd after it has been freed Ingo Molnar 2017-01-30 8:49 ` [tip:x86/microcode] " tip-bot for Borislav Petkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).