linux-kernel - Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by virtual address

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]

Message-ID: <20161220144012.lc4cwrg6dphqbyqu@pd.tnic>
Date:   Tue, 20 Dec 2016 15:40:12 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Boris Ostrovsky <boris.ostrovsky@...cle.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/microcode: Adjust ramdisk address when accessing by
 virtual address

On Mon, Dec 19, 2016 at 08:40:00PM -0500, Boris Ostrovsky wrote:
> Sorry, forgot about this: I see the first line but not the other two (so the
> relocation did not occur).

Good, that's what I expected. Now it makes sense.

So the reason why it didn't trigger here is that my Intel 32-bit .config
always relocates the ramdisk. And that was taken care of. But not the
case where it didn't relocate it. I.e., your case.

The fix below should generalize the situation to *always*
read initrd_start when we're running late, with VAs and after
reserve_initrd() has run and potentially moved the ramdisk.

> > I also thought it might be better but I haven't gone through the code to
> > make sure this would always work.
> > 
> > I can run more tests tomorrow if you want.

Yes please. Here's a minimal patch for 4.10. Please run it on your setup
to verify it fixes your issue. It boots fine on my boxes here, FWIW.

---
From: Borislav Petkov <bp@...e.de>
Date: Tue, 20 Dec 2016 11:54:30 +0100
Subject: [PATCH] x86/microcode/AMD: Reload proper initrd start address

When we switch to virtual addresses and, especially after
reserve_initrd()->relocate_initrd() have run, we have the updated initrd
address in initrd_start. Use initrd_start then instead of the address
which has been passed to us through boot params. (That still gets used
when we're running the very early routines on the BSP).

Reported-by: Boris Ostrovsky <boris.ostrovsky@...cle.com>
Signed-off-by: Borislav Petkov <bp@...e.de>
---
 arch/x86/kernel/cpu/microcode/core.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index c4bb2f7169f6..2af69d27da62 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -243,14 +243,12 @@ struct cpio_data find_microcode_in_initrd(const char *path, bool use_pa)
 # endif

 	/*
-	 * Did we relocate the ramdisk?
-	 *
-	 * So we possibly relocate the ramdisk *after* applying microcode on the
-	 * BSP so we rely on use_pa (use physical addresses) - even if it is not
-	 * absolutely correct - to determine whether we've done the ramdisk
-	 * relocation already.
+	 * Fixup the start address: after reserve_initrd() runs, initrd_start
+	 * 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.
 	 */
-	if (!use_pa && relocated_ramdisk)
+	if (!use_pa && initrd_start)
 		start = initrd_start;

 	return find_cpio_data(path, (void *)start, size, NULL);
-- 
2.11.0

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.