From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 412D4374C for ; Tue, 26 Apr 2016 13:42:44 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 26 Apr 2016 04:42:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,536,1455004800"; d="scan'208";a="953147607" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by fmsmga001.fm.intel.com with ESMTP; 26 Apr 2016 04:42:42 -0700 Received: from irsmsx103.ger.corp.intel.com ([169.254.3.12]) by IRSMSX152.ger.corp.intel.com ([169.254.6.15]) with mapi id 14.03.0248.002; Tue, 26 Apr 2016 12:42:41 +0100 From: "Mrozowicz, SlawomirX" To: "Gonzalez Monroy, Sergio" , "Richardson, Bruce" CC: "david.marchand@6wind.com" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] eal: out-of-bounds write Thread-Index: AQHRn49e4dl4FtJqW0SZmY5h5orbgp+b4mQAgAAOGQCAACurQA== Date: Tue, 26 Apr 2016 11:42:40 +0000 Message-ID: <158888A50F43E34AAE179517F56C97455985DA@IRSMSX103.ger.corp.intel.com> References: <1461656687-5396-1-git-send-email-slawomirx.mrozowicz@intel.com> <20160426085343.GA17164@bricha3-MOBL3> <571F386C.3070102@intel.com> In-Reply-To: <571F386C.3070102@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="iso-8859-2" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] eal: out-of-bounds write X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Apr 2016 11:42:44 -0000 >-----Original Message----- >From: Gonzalez Monroy, Sergio >Sent: Tuesday, April 26, 2016 11:44 AM >To: Richardson, Bruce ; Mrozowicz, SlawomirX > >Cc: david.marchand@6wind.com; dev@dpdk.org >Subject: Re: [dpdk-dev] [PATCH] eal: out-of-bounds write > >On 26/04/2016 09:53, Bruce Richardson wrote: >> On Tue, Apr 26, 2016 at 09:44:47AM +0200, Slawomir Mrozowicz wrote: >>> Fix issue reported by Coverity. >>> >>> Coverity ID 13282: Out-of-bounds write >>> overrun-local: Overrunning array mcfg->memseg of 256 44-byte elements >>> at element index 257 using index j. >>> >>> Fixes: af75078fece3 ("first public release") >>> >>> Signed-off-by: Slawomir Mrozowicz >>> --- >>> lib/librte_eal/linuxapp/eal/eal_memory.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c >>> b/lib/librte_eal/linuxapp/eal/eal_memory.c >>> index 5b9132c..1e737e4 100644 >>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c >>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c >>> @@ -1333,7 +1333,7 @@ rte_eal_hugepage_init(void) >>> >>> if (new_memseg) { >>> j +=3D 1; >>> - if (j =3D=3D RTE_MAX_MEMSEG) >>> + if (j >=3D RTE_MAX_MEMSEG) >>> break; >>> >>> mcfg->memseg[j].phys_addr =3D hugepage[i].physaddr; >>> -- >> This does appear to be a valid fix for the issue. However, looking at >> the code, it appears that the only way we could actually hit the >> problem is if j =3D=3D RTE_MAX_MEMSEG on exiting the previous loop. Woul= d >> a check there be a better fix for this issue (or perhaps we want both fi= xes). >> >> Thoughts? > >It doesn't make sense to go into the loop if we don't have free memsegs. >Either way we should print the error indicating that we reached >MAX_MEMSEG. > >Sergio > It is possible to add additional checking available memseg before the loop.= =20 In this case it will be checked twice before loop and inside loop.=20 In my opinion it is not necessary. Anyway it is valuable to add in line 1336 error message if the MAX_MEMSEG i= s reached. S=B3awomir > >> /Bruce