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

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]

Date:   Sat, 10 Mar 2018 13:26:00 +0100
From:   "Maciej S. Szmigiero" <mail@...iej.szmigiero.name>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/microcode/AMD: check microcode file sanity before
 loading it

On 10.03.2018 10:18, Borislav Petkov wrote:
> On Sat, Mar 10, 2018 at 01:34:45AM +0100, 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.
> 
> Sorry but if one has enough permissions to install malformed microcode,
> crashing the loader is the least of your problems. IOW, I don't see the
> justification for the unnecessary complication with all those checks.

While I agree this is not a security problem, I cannot agree that these
checks are unnecessary driver complication.

First, these checks are really just very basic checks like "check
whether the loaded file is long enough to actually contain some
structure before accessing it" or "don't iterate an array in file
without checking if it actually has a terminating element" or "check
whether microcode patch length isn't something like 2GB before allocating
memory for it".

Without them, it is easy to crash the driver when just playing with
microcode files (and it turns out that AMD-released microcode files in
linux-firmware actually don't contain the newest microcode versions,
even for older CPUs).

Second, since these checks happen only on a microcode file load
(something that 99.9% of systems probably will do just once at boot
time) it is hardly a performance-critical path.

Third, we still do check consistency of data provided to various
root-only syscalls (and these might be much more performance-critical
than this code).

> Thx.
> 

Thanks,
Maciej