Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751525AbdH0XJR convert rfc822-to-8bit (ORCPT ); Sun, 27 Aug 2017 19:09:17 -0400 Received: from mga04.intel.com ([192.55.52.120]:11759 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751401AbdH0XJQ (ORCPT ); Sun, 27 Aug 2017 19:09:16 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,438,1498546800"; d="scan'208";a="142536604" From: "Waskiewicz Jr, Peter" To: Christophe JAILLET , "Kirsher, Jeffrey T" CC: "intel-wired-lan@lists.osuosl.org" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "kernel-janitors@vger.kernel.org" Subject: Re: [PATCH] igb: check memory allocation failure Thread-Topic: [PATCH] igb: check memory allocation failure Thread-Index: AQHTHv+fnj5ml27UL0anrhb/WN5ubA== Date: Sun, 27 Aug 2017 23:09:12 +0000 Message-ID: References: <20170827063951.5975-1-christophe.jaillet@wanadoo.fr> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.82.203] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1260 Lines: 31 On 8/27/17 2:42 AM, Christophe JAILLET wrote: > Check memory allocation failures and return -ENOMEM in such cases, as > already done for other memory allocations in this function. > > This avoids NULL pointers dereference. > > Signed-off-by: Christophe JAILLET > --- > drivers/net/ethernet/intel/igb/igb_main.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c > index fd4a46b03cc8..837d9b46a390 100644 > --- a/drivers/net/ethernet/intel/igb/igb_main.c > +++ b/drivers/net/ethernet/intel/igb/igb_main.c > @@ -3162,6 +3162,8 @@ static int igb_sw_init(struct igb_adapter *adapter) > /* Setup and initialize a copy of the hw vlan table array */ > adapter->shadow_vfta = kcalloc(E1000_VLAN_FILTER_TBL_SIZE, sizeof(u32), > GFP_ATOMIC); > + if (!adapter->shadow_vfta) > + return -ENOMEM; Looks reasonable to me. A larger issue though I see in this function is that if we return -ENOMEM here, and if we return -ENOMEM from igb_init_interrupt_scheme() below on failure, we leak adapter->mac_table (and adapter->shadow_vfta in the latter). We should add a proper unwind to free up the memory on failure. -PJ