Messages in this thread | | | Date | Tue, 24 Feb 2015 17:20:08 +0100 | From | Quentin Casasnovas <> | Subject | Re: [PATCH 02/13] x86/microcode/intel: Do the mc_saved_src NULL check first |
| |
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
| |