Re: [GIT PULL] microcode loader updates

[Quentin Casasnovas <quentin.casasnovas@oracle.com>]

On Mon, Mar 02, 2015 at 02:29:50PM +0100, Borislav Petkov wrote:
> On Mon, Mar 02, 2015 at 02:03:36PM +0100, Quentin Casasnovas wrote:
> > 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.
> 
> You're correct, but(!)...
> 
> Practically, this is not a problem because @mc_saved_src being handed
> down to save_microcode() is at both call sites initialized up to
> mc_saved_count elements and the loop in save_microcode() only inspects
> this far.
> 
> So actually, this test is not really needed:
> 
>                 if (!mc_saved_src[i]) {
>                         ret = -EINVAL;
>                         goto err;
>                 }
> 
> AFAICT and if I'm not missing anything else, of course.
> 
> In any case, I'd like to keep this series cleanup-only (well, except
> this one) and address your comments later. Don't worry, I haven't
> forgotten them - I want to *not* fix everything in one go.
> 
> Agreed?
> 

Hey up to you, really :)

It's just that this potential-but-very-very-likely-impossible kfree() on
garbage wasn't present in the original code - so I thought changing the
kmalloc() => kcalloc() was small enough to add in your serie.  I'd also be
fine removing the early loop termination condition if you think it's dead
code since that'll make sure this will never happen.  A static analyzer or
maybe some cocinnelle semantic patches are likely to start complaining
about this otherwise, I think.

Quentin