lkml.org 
[lkml]   [2015]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [GIT PULL] microcode loader updates
Hi Boris!

On Mon, Mar 02, 2015 at 01:34:41PM +0100, Borislav Petkov wrote:
> Hi guys,
>
> here's the first pile of microcode loader cleanups for 4.1.
>
> Please pull,
> thanks.
>
> ---
> The following changes since commit c517d838eb7d07bbe9507871fab3931deccff539:
>
> Linux 4.0-rc1 (2015-02-22 18:21:14 -0800)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git tags/intel_microcode_cleanup_p1
>
> for you to fetch changes up to 94a7cbf93996711a32e286488b8523740d6a19f0:
>
> x86/microcode/intel: Fix printing of microcode blobs in show_saved_mc() (2015-03-02 10:07:36 +0100)
>
> ----------------------------------------------------------------
> The first part of the scrubbing of the intel early microcode loader.
> There's more work to come but let's unload this pile first.
>
> ----------------------------------------------------------------
> Borislav Petkov (13):
> x86/microcode/intel: Check if microcode was found before applying
> x86/microcode/intel: Do the mc_saved_src NULL check first

I don't know if you missed a comment I had on my initial review but I think
you might be introducing a kfree() on garbage data in one edge case here.
Sorry if I'm missing something, as usual :)

In save_microcode(), if at some loop iteration mc_saved_src[i] == NULL,
we'll jump to label `err`:

> saved_ptr = kmalloc(mc_saved_count * sizeof(struct microcode_intel *),
> GFP_KERNEL);
> if (!saved_ptr)
> return -ENOMEM;
>
> for (i = 0; i < mc_saved_count; i++) {
> struct microcode_header_intel *mc_hdr;
> struct microcode_intel *mc;
> unsigned long size;
>
> if (!mc_saved_src[i]) {
> ret = -EINVAL;
> goto err;
> }
> mc = mc_saved_src[i];
> mc_hdr = &mc->hdr;
> size = get_totalsize(mc_hdr);
>
> saved_ptr[i] = kmalloc(size, GFP_KERNEL);
> if (!saved_ptr[i]) {
> ret = -ENOMEM;
> goto err;
> }
>
> memcpy(saved_ptr[i], mc, size);
> }

which does:

> for (j = 0; j <= i; j++)
> kfree(saved_ptr[j]);
> kfree(saved_ptr);

So at the last loop iteration for j == i, we'll do kfree(saved_ptr[j])
which AFAICT hasn't been initialized yet. Using a kcalloc() your first
allocation for saved_ptr should just work since the memory will be cleared
and kfree(NULL) doesn't do anything.

Quentin


\
 
 \ /
  Last update: 2015-03-02 14:41    [W:0.108 / U:0.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site