Re: [PATCH] x86/microcode/AMD: check microcode file sanity before loading it

From: Maciej S. Szmigiero
Date: Sat Mar 10 2018 - 11:16:48 EST


On 10.03.2018 01:34, Maciej S. Szmigiero wrote:
> Currently, it is very easy to make the AMD microcode update driver crash
> or spin on a malformed microcode file since it does very little
> consistency checking on data loaded from such file.
>
> This commit introduces various checks, mostly on length-type fields, so
> all corrupted microcode files are (hopefully) correctly rejected instead.
>
> Signed-off-by: Maciej S. Szmigiero <mail@xxxxxxxxxxxxxxxxxxxxx>

To make sure that it is clear what this patch is about:
It *isn't* about verifying the actual microcode update, that is, the
blob that gets sent to the CPU as the new microcode.
Such verification is (hopefully) done by the CPU itself.

It is about verifying *this driver-specific* container file that
includes many microcode updates for different CPUs of a particular CPU
family, along with metadata that helps the driver select the right
microcode update to actually sent to the CPU.

The microcode container files in linux-firmware don't contain the
newest microcode versions, even for older CPUs.
They are generally updated VERY rarely - I can see only three updates
for them: on 2016-03-18, 2014-11-30 and 2013-07-11.
There is no container file at all for family 17h (Zen) so
distributions like OpenSUSE that include this file must have gotten it
from some other source (or made from raw updates themselves).

That's why to get things like IBPB it is sometimes necessary to use
a newer microcode version than included in linux-firmware, sourced for
example from a BIOS update.
Since BIOS updates contain only actual (raw) microcode updates one
has to place it in a microcode container file so this driver can parse
it.

As far I know there is no tool to automate this work so one has to
manually tweak the container metadata.
And this is prone to mistakes this patch protects against.

Sorry for not making this 100% clear in the commit log.

Maciej