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