| 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
| ||
|
Message-ID: <20170126165833.evjemhbqzaepirxo@pd.tnic>
Date: Thu, 26 Jan 2017 17:58:33 +0100
From: Borislav Petkov <bp@...en8.de>
To: Andrey Ryabinin <aryabinin@...tuozzo.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>,
"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...nel.org>
Subject: [PATCH] x86/microcode: Do not access the initrd after it has been
freed
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@...tuozzo.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@...e.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@...tuozzo.com>
Signed-off-by: Borislav Petkov <bp@...e.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.
Powered by blists - more mailing lists