Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752945AbbBXQS2 (ORCPT ); Tue, 24 Feb 2015 11:18:28 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:27202 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751281AbbBXQS1 (ORCPT ); Tue, 24 Feb 2015 11:18:27 -0500 Date: Tue, 24 Feb 2015 17:20:08 +0100 From: Quentin Casasnovas To: Borislav Petkov Cc: X86 ML , LKML , Quentin Casasnovas Subject: Re: [PATCH 02/13] x86/microcode/intel: Do the mc_saved_src NULL check first Message-ID: <20150224162008.GC4565@chrystal.uk.oracle.com> References: <1424774232-5981-1-git-send-email-bp@alien8.de> <1424774232-5981-3-git-send-email-bp@alien8.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1424774232-5981-3-git-send-email-bp@alien8.de> User-Agent: Mutt/1.5.22 (2013-10-16) X-Source-IP: ucsinet22.oracle.com [156.151.31.94] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1793 Lines: 56 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/