Message ID | 1461321661-30272-1-git-send-email-michalx.kobylinski@intel.com (mailing list archive) |
---|---|
State | Rejected, archived |
Delegated to: | Thomas Monjalon |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 93DAC2E83; Fri, 22 Apr 2016 13:11:48 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id EC0772C4B for <dev@dpdk.org>; Fri, 22 Apr 2016 13:11:46 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP; 22 Apr 2016 04:11:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,516,1455004800"; d="scan'208";a="964218220" Received: from gklab-246-020.igk.intel.com (HELO Sent) ([10.217.246.20]) by fmsmga002.fm.intel.com with SMTP; 22 Apr 2016 04:11:44 -0700 Received: by Sent (sSMTP sendmail emulation); Fri, 22 Apr 2016 12:41:03 +0200 From: Michal Kobylinski <michalx.kobylinski@intel.com> To: cristian.dumitrescu@intel.com, dev@dpdk.org Cc: Michal Kobylinski <michalx.kobylinski@intel.com> Date: Fri, 22 Apr 2016 12:41:01 +0200 Message-Id: <1461321661-30272-1-git-send-email-michalx.kobylinski@intel.com> X-Mailer: git-send-email 1.9.1 Subject: [dpdk-dev] [PATCH] cfgfile: fix integer overflow X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Michal Kobylinski
April 22, 2016, 10:41 a.m. UTC
Fix issue reported by Coverity.
Coverity ID 13289: Integer overflowed argument: The argument will be too
small or even negative, likely resulting in unexpected behavior (for
example, under-allocation in a memory allocation function).
In rte_cfgfile_load: An integer overflow occurs, with the overflowed
value used as an argument to a function
Fixes: eaafbad419bf ("cfgfile: library to interpret config files")
Signed-off-by: Michal Kobylinski <michalx.kobylinski@intel.com>
---
lib/librte_cfgfile/rte_cfgfile.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Fri, 22 Apr 2016 12:41:01 +0200 Michal Kobylinski <michalx.kobylinski@intel.com> wrote: > Fix issue reported by Coverity. > > Coverity ID 13289: Integer overflowed argument: The argument will be too > small or even negative, likely resulting in unexpected behavior (for > example, under-allocation in a memory allocation function). > In rte_cfgfile_load: An integer overflow occurs, with the overflowed > value used as an argument to a function > > Fixes: eaafbad419bf ("cfgfile: library to interpret config files") > > Signed-off-by: Michal Kobylinski <michalx.kobylinski@intel.com> > --- > lib/librte_cfgfile/rte_cfgfile.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c > index 75625a2..0a5a279 100644 > --- a/lib/librte_cfgfile/rte_cfgfile.c > +++ b/lib/librte_cfgfile/rte_cfgfile.c > @@ -135,7 +135,7 @@ rte_cfgfile_load(const char *filename, int flags) > goto error1; > } > *end = '\0'; > - _strip(&buffer[1], end - &buffer[1]); > + _strip(&buffer[1], (unsigned)(end - &buffer[1])); > The cast doesn't actually fix any potential bug. It just causes the function to get an signed overflow value.
> -----Original Message----- > From: Kobylinski, MichalX > Sent: Friday, April 22, 2016 11:41 AM > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org > Cc: Kobylinski, MichalX <michalx.kobylinski@intel.com> > Subject: [PATCH] cfgfile: fix integer overflow > > Fix issue reported by Coverity. > > Coverity ID 13289: Integer overflowed argument: The argument will be too > small or even negative, likely resulting in unexpected behavior (for > example, under-allocation in a memory allocation function). > In rte_cfgfile_load: An integer overflow occurs, with the overflowed > value used as an argument to a function > > Fixes: eaafbad419bf ("cfgfile: library to interpret config files") > > Signed-off-by: Michal Kobylinski <michalx.kobylinski@intel.com> > --- > lib/librte_cfgfile/rte_cfgfile.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c > index 75625a2..0a5a279 100644 > --- a/lib/librte_cfgfile/rte_cfgfile.c > +++ b/lib/librte_cfgfile/rte_cfgfile.c > @@ -135,7 +135,7 @@ rte_cfgfile_load(const char *filename, int flags) > goto error1; > } > *end = '\0'; > - _strip(&buffer[1], end - &buffer[1]); > + _strip(&buffer[1], (unsigned)(end - &buffer[1])); > > /* close off old section and add start new one */ > if (curr_section >= 0) > -- > 1.9.1 I don't understand the root issue here, can you please explain? It looks to me that "end" is always going to point to a location bigger or equal to &buffer[1]. So the second parameter of _strip function is always going to be a positive number (0 included).
2016-04-28 11:09, Dumitrescu, Cristian: > From: Kobylinski, MichalX > > Fix issue reported by Coverity. > > > > Coverity ID 13289: Integer overflowed argument: The argument will be too > > small or even negative, likely resulting in unexpected behavior (for > > example, under-allocation in a memory allocation function). > > In rte_cfgfile_load: An integer overflow occurs, with the overflowed > > value used as an argument to a function > > > > Fixes: eaafbad419bf ("cfgfile: library to interpret config files") > > > > Signed-off-by: Michal Kobylinski <michalx.kobylinski@intel.com> > > I don't understand the root issue here, can you please explain? > > It looks to me that "end" is always going to point to a location bigger or equal to &buffer[1]. So the second parameter of _strip function is always going to be a positive number (0 included). Michal, any answer please?
> -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Monday, May 16, 2016 12:06 PM > To: Kobylinski, MichalX <michalx.kobylinski@intel.com> > Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com> > Subject: Re: [dpdk-dev] [PATCH] cfgfile: fix integer overflow > Importance: High > > 2016-04-28 11:09, Dumitrescu, Cristian: > > From: Kobylinski, MichalX > > > Fix issue reported by Coverity. > > > > > > Coverity ID 13289: Integer overflowed argument: The argument will be > > > too small or even negative, likely resulting in unexpected behavior > > > (for example, under-allocation in a memory allocation function). > > > In rte_cfgfile_load: An integer overflow occurs, with the overflowed > > > value used as an argument to a function > > > > > > Fixes: eaafbad419bf ("cfgfile: library to interpret config files") > > > > > > Signed-off-by: Michal Kobylinski <michalx.kobylinski@intel.com> > > > > I don't understand the root issue here, can you please explain? > > > > It looks to me that "end" is always going to point to a location bigger or > equal to &buffer[1]. So the second parameter of _strip function is always > going to be a positive number (0 included). > > Michal, any answer please? Hi Thomas, Cristian Coverity show that there is overflowed value. But the second parameter will never be greater than 254 (its range is 0 - 254). I used cast this parameter to unsigned in order that resolved bug reported by static analysis.
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Kobylinski, MichalX > Sent: Monday, May 16, 2016 1:51 PM > To: Thomas Monjalon <thomas.monjalon@6wind.com> > Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com> > Subject: Re: [dpdk-dev] [PATCH] cfgfile: fix integer overflow > ... > Coverity show that there is overflowed value. > But the second parameter will never be greater than 254 (its range is 0 - > 254). > I used cast this parameter to unsigned in order that resolved bug reported > by static analysis. Hi, If the error cannot happen in a real application then you can mark the defect as a false positive in Coverity. Add some of the information provided above as to why it is a false positive to the comments section of the defect or add a link to the patchwork discussion. John
diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c index 75625a2..0a5a279 100644 --- a/lib/librte_cfgfile/rte_cfgfile.c +++ b/lib/librte_cfgfile/rte_cfgfile.c @@ -135,7 +135,7 @@ rte_cfgfile_load(const char *filename, int flags) goto error1; } *end = '\0'; - _strip(&buffer[1], end - &buffer[1]); + _strip(&buffer[1], (unsigned)(end - &buffer[1])); /* close off old section and add start new one */ if (curr_section >= 0)