From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id AFD0BA0524; Thu, 2 Jul 2020 11:20:47 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9D2471D707; Thu, 2 Jul 2020 11:20:46 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 44EE81D6E3; Thu, 2 Jul 2020 11:20:44 +0200 (CEST) IronPort-SDR: qBtX7CFJXQBVIYYxuorlPphJqqPRnjPN6wpvxNC0+oj3SCI7gzD+N9DX5LUNDblJO7WYJUhvoS xE/CDzcyl3tw== X-IronPort-AV: E=McAfee;i="6000,8403,9669"; a="211879444" X-IronPort-AV: E=Sophos;i="5.75,303,1589266800"; d="scan'208";a="211879444" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Jul 2020 02:20:44 -0700 IronPort-SDR: 4rbha3MnIqByzj16+j+HZ2xfzpos8xzMWpKjDhmi/X8mPZT50yUsgHCJ+YXtNvJYXGozGsn7wB /oMQ1g+QiReA== X-IronPort-AV: E=Sophos;i="5.75,303,1589266800"; d="scan'208";a="455458438" Received: from unknown (HELO bricha3-MOBL.ger.corp.intel.com) ([10.252.18.120]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 02 Jul 2020 02:20:42 -0700 Date: Thu, 2 Jul 2020 10:20:39 +0100 From: Bruce Richardson To: Stephen Hemminger Cc: cristian.dumitrescu@intel.com, dev@dpdk.org, jacekx.piasecki@intel.com, stable@dpdk.org Message-ID: <20200702092039.GC611@bricha3-MOBL.ger.corp.intel.com> References: <20200702030558.17852-1-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200702030558.17852-1-stephen@networkplumber.org> Subject: Re: [dpdk-dev] [PATCH] cfgfile: avoid stack buffer underflow X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Jul 01, 2020 at 08:05:58PM -0700, Stephen Hemminger wrote: > If cfgfile is give a line with comment character at the start > of the line, it will dereference outside of the buffer. > > Detected with address sanitizer: > > SUMMARY: AddressSanitizer: stack-buffer-underflow lib/librte_cfgfile/rte_cfgfile.c:194 in rte_cfgfile_load_with_params > Shadow bytes around the buggy address: > 0x200fff79f6a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fff79f6b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fff79f6c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fff79f6d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fff79f6e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > =>0x200fff79f6f0: 00 00 00 00 f1 f1 f1[f1]00 00 00 00 00 00 00 00 > 0x200fff79f700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fff79f710: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fff79f720: 04 f2 f2 f2 f3 f3 f3 f3 00 00 00 00 00 00 00 00 > 0x200fff79f730: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 f2 > 0x200fff79f740: f2 f2 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 > Shadow byte legend (one shadow byte represents 8 application bytes): > Addressable: 00 > Partially addressable: 01 02 03 04 05 06 07 > Heap left redzone: fa > Freed heap region: fd > Stack left redzone: f1 > Stack mid redzone: f2 > Stack right redzone: f3 > Stack after return: f5 > Stack use after scope: f8 > Global redzone: f9 > Global init order: f6 > Poisoned by user: f7 > Container overflow: fc > Array cookie: ac > Intra object redzone: bb > ASan internal: fe > Left alloca redzone: ca > Right alloca redzone: cb > ==2189==ABORTING > > Fixes: a6a47ac9c2c9 ("cfgfile: rework load function") > Cc: jacekx.piasecki@intel.com > CC: stable@dpdk.org > Signed-off-by: Stephen Hemminger > --- > lib/librte_cfgfile/rte_cfgfile.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c > index 714717dd9007..160d78826e7c 100644 > --- a/lib/librte_cfgfile/rte_cfgfile.c > +++ b/lib/librte_cfgfile/rte_cfgfile.c > @@ -191,7 +191,8 @@ rte_cfgfile_load_with_params(const char *filename, int flags, > } > /* skip parsing if comment character found */ > pos = memchr(buffer, params->comment_character, len); > - if (pos != NULL && (*(pos-1) != '\\')) { > + if (pos != NULL && > + (pos == buffer || *(pos-1) != '\\')) { > *pos = '\0'; > len = pos - buffer; > } > -- Good catch by the tool. Reviewed-by: Bruce Richardson