Messages in this thread | | | Date | Wed, 24 Jul 2013 20:06:07 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH]Fix early microcode loading on AMD |
| |
On Wed, Jul 24, 2013 at 04:44:45PM +0200, Torsten Kaiser wrote: > Then it would probably be the best to kill free_cache() completely. > Which would mean cleanup() should also go. > Which will make unloading microcode_amd.ko impossible. > But that is probably a good idea anyway: If you unload the module > there is no way to keep pcache. > > But I still have another way to kill you: free_equiv_cpu_table() > Without that table find_patch() can't work and will not return the > correct information. > > And that can be triggered by: > * start of load_microcode_amd(): If you reach that function (Only > UCODE_MAGIC needs to be in the file) that table is dead. > * __load_microcode_amd(): If the file only contains the table but no > patches ("invalid type field in container file section header\n")
If you have root, there are a gazillion ways to kill the system. Those are just a couple more, and a bit complicated even :)
> But it already called free_equiv_cpu_table() and so pcache is inaccessible.
That's the old table. __load_microcode_amd() installs the current one.
Ok, I remember now, the situation is a bit different: the container file has all patches. If we load a new container file, it contains newer replacements + old ones which remained unchanged:
http://marc.info/?l=linux-kernel&m=137427483928693
So actually, when we encounter an error when loading a new blob, we have to *keep* the old pcache and equiv_table. Which means load_microcode_amd() shouldn't free the table *before* doing __load_microcode_amd() but *after* verifying it succeeded.
> And I don't think just preserving equiv_cpu_table for restoring in > the error case will be the right solution: If the new firmware file > contains a new table with fewer entries (or different entries!) some > of the patches in pcache might become inaccessible.
Yes, see above.
> >> That copying already in load_microcode also is suspicious if someone > >> would only load the microcode but not apply it. But I did not find > >> a codepath in arch/x86/kernel/microcode_core.c to load it without a > >> followup apply. > > > > Yeah, we always load and apply. > > > > So now back to the original problem - load_microcode_amd() shouldn't > > clear the pcache and, in that case, a subsequent find_patch() would > > always give the right patch. > > Not if equiv_cpu_table got mangled. > So should install_equiv_cpu_table() be turned into > add_to_equiv_cpu_table() or should pcache save all cpu_sig with each > patch, so that find_patch() no longer needs equiv_cpu_table? > I suspect saving that in struct ucode_patch might be better, to > prevent changes in equiv_id <-> cpu_sig mapping to make a patch > inaccessible.
Yeah. I remembered :) See above.
Thanks.
-- Regards/Gruss, Boris.
Sent from a fat crate under my desk. Formatting is fine. --
| |