Re: [PATCH 02/13] x86/microcode/intel: Do the mc_saved_src NULL check first

[Quentin Casasnovas]

On Tue, Feb 24, 2015 at 11:37:01AM +0100, Borislav Petkov wrote:
> @@ -213,39 +213,46 @@ save_microcode(struct mc_saved_data *mc_saved_data,
>  	/*
>  	 * Copy new microcode data.
>  	 */
> -	mc_saved_p = kmalloc(mc_saved_count*sizeof(struct microcode_intel *),
> +	saved_ptr = kmalloc(mc_saved_count * sizeof(struct microcode_intel *),
>  			     GFP_KERNEL);

I'd be tempted to use kcalloc() in these cases but I suppose it's just
personnal preference - it avoids having to make sure your multiplication
cannot overflow when reviewing.

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

... though in this particular case, I think using kcalloc() above would
also prevent the introduction of a kfree() on random junk if
!mc_saved_src[0] since you'll jump straight to err which will
kfree(saved_ptr[0]), which isn't initialized yet.  So it might be worth the
change :)

>  
>  err:
>  	for (j = 0; j <= i; j++)
> -		kfree(mc_saved_p[j]);
> -	kfree(mc_saved_p);
> +		kfree(saved_ptr[j]);
> +	kfree(saved_ptr);
>  

Quentin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/