linux-kernel - Re: [PATCH]Fix early microcode loading on AMD

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: <20130724141927.GI30777@pd.tnic>
Date:	Wed, 24 Jul 2013 16:19:27 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	Torsten Kaiser <just.for.lkml@...glemail.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, Jacob Shin <jacob.shin@....com>,
	Johannes Hirte <johannes.hirte@....tu-ilmenau.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH]Fix early microcode loading on AMD

On Tue, Jul 23, 2013 at 06:57:12PM +0200, Torsten Kaiser wrote:
> > The other problem I see is not updating c->microcode since it is going
> > to be overwritten by smp_store_cpu_info, which is unfortunate.
> >
> > And I don't see where Intel are updating that cpuinfo_x86.microcode
> > field on early load too.
> >
> > So, AFAICT, c->microcode would remain unset when we only do early
> > microcode load. But that is something we should fix as a later patch.
> 
> I don't see a problem with that staying unset.
> apply_microcode_amd() directly reads the rev from
> MSR_AMD64_PATCH_LEVEL so it does not depend on that being correct.
> And smp_store_(boot)_cpu_info will also read the current rev directly
> from the CPU to fill ->microcode.

We need to store the actual microcode revision to c->microcode for
/proc/cpuinfo and MCE.

> 
> > So I think you should switch load_ucode_amd_ap to __apply_microcode_amd:
> >
> >         p = find_patch()
> >
> >         __apply_microcode_amd(p->mc_data);
> >
> > which should take care of the issue you're seeing, IMHO.
> 
> The issue I'm seeing is that collect_cpu_info_amd_early() fills c->x86
> but not c->x86_vendor.
> Which breaks cpu_has_amd_erratum() and then Erratum 400 breaks the boot.
> 
> I did not really want to switch from apply_microcode_amd() to
> __apply_microcode_amd() because then I would lose the check if the new
> microcode is really an upgrade.

Well, if the BSP has already loaded the pcache, there's no need for
the AP to parse and load the same microcode blobs file for the initrd,
right?

> >> * load_ucode_ap(): Quick exit for !cpu, because without load_microcode_amd()
> >> getting called apply_microcode_amd() can't do anything. Exit, if no microcode
> >> could be loaded.
> >
> > This could probably be a WARN_ON(!cpu) to catch errors...
> 
> No, load_ucode_ap() will be called for cpu == 0.

This needs fixing IMO...

Btw, thanks for looking at this and asking critical questions!

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/