From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6E883A0A02; Mon, 17 May 2021 10:50:58 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E041B4014E; Mon, 17 May 2021 10:50:57 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mails.dpdk.org (Postfix) with ESMTP id BFCAF40041; Mon, 17 May 2021 10:50:56 +0200 (CEST) IronPort-SDR: sGKzQdP3av09xlkT/quk9+KzBlCSxOPcJOcLvMQuluf9wRRmz0/lHcQZtCeE2Gh4eUN+NfSnMg HFeRKG2qFj4w== X-IronPort-AV: E=McAfee;i="6200,9189,9986"; a="285946600" X-IronPort-AV: E=Sophos;i="5.82,306,1613462400"; d="scan'208";a="285946600" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2021 01:50:55 -0700 IronPort-SDR: fp4Vca8a+WgopbjWJPrieI+tEJZCK6AdPF/ug9/cZHS7c04plzs5Ei8Pe7LungoQiIk8ZxGKX7 AHjZKVUkFxmg== X-IronPort-AV: E=Sophos;i="5.82,306,1613462400"; d="scan'208";a="472305357" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.193.42]) ([10.213.193.42]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2021 01:50:53 -0700 To: "Zhang, Tianfei" , "Huang, Wei" , "Xu, Rosen" Cc: David Marchand , "stable@dpdk.org" , "Zhang, Qi Z" , "dev@dpdk.org" , "Mcnamara, John" , Aaron Conole References: <20210408085151.54996-1-wei.huang@intel.com> <20210408085151.54996-2-wei.huang@intel.com> From: Ferruh Yigit X-User: ferruhy Message-ID: <386fb56f-a46e-ce20-47ba-b56f0f480ab9@intel.com> Date: Mon, 17 May 2021 09:50:49 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2 1/1] raw/ifpga/base: check size before assigning X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 4/14/2021 3:46 AM, Zhang, Tianfei wrote: > > >> -----Original Message----- >> From: Aaron Conole >> Sent: 2021年4月9日 22:56 >> To: Yigit, Ferruh >> Cc: David Marchand ; stable@dpdk.org; >> Zhang, Tianfei ; Huang, Wei >> ; Zhang, Qi Z ; Xu, Rosen >> ; dev@dpdk.org; Mcnamara, John >> >> Subject: Re: [PATCH v2 1/1] raw/ifpga/base: check size before assigning >> >> Ferruh Yigit writes: >> >>> On 4/8/2021 9:51 AM, Wei Huang wrote: >>>> In max10_staging_area_init(), variable "size" from fdt_get_reg() may >>>> be invalid, it should be checked before assigning to member variable >>>> "staging_area_size" of structure "intel_max10_device". >>>> >>>> Coverity issue: 367480, 367482 >>>> Fixes: 96ebfcf8125c ("raw/ifpga/base: add SPI and MAX10 device >>>> driver") >>>> >>>> Signed-off-by: Wei Huang >>>> --- >>>> v2: check size before assigning to staging_area_size >>>> --- >>>> drivers/raw/ifpga/base/opae_intel_max10.c | 2 +- >>>> drivers/raw/ifpga/base/opae_intel_max10.h | 1 + >>>> 2 files changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/raw/ifpga/base/opae_intel_max10.c >>>> b/drivers/raw/ifpga/base/opae_intel_max10.c >>>> index 443e248fb3..c223fafa03 100644 >>>> --- a/drivers/raw/ifpga/base/opae_intel_max10.c >>>> +++ b/drivers/raw/ifpga/base/opae_intel_max10.c >>>> @@ -593,7 +593,7 @@ static int max10_staging_area_init(struct >> intel_max10_device *dev) >>>> continue; >>>> ret = fdt_get_reg(fdt_root, offset, 0, &start, &size); >>>> -if (!ret) { >>>> +if (!ret && (size <= MAX_STAGING_AREA_SIZE)) { >>>> dev->staging_area_base = start; >>>> dev->staging_area_size = size; >>>> } >>>> diff --git a/drivers/raw/ifpga/base/opae_intel_max10.h >>>> b/drivers/raw/ifpga/base/opae_intel_max10.h >>>> index 670683f017..e7142d6f0d 100644 >>>> --- a/drivers/raw/ifpga/base/opae_intel_max10.h >>>> +++ b/drivers/raw/ifpga/base/opae_intel_max10.h >>>> @@ -182,6 +182,7 @@ struct opae_retimer_status { >>>> #define SBUS_VERSIONGENMASK(31, 16) >>>> #define DFT_MAX_SIZE0x7e0000 >>>> +#define MAX_STAGING_AREA_SIZE0x3800000 >>>> int max10_reg_read(struct intel_max10_device *dev, >>>> unsigned int reg, unsigned int *val); >>>> >>> >>> Hi Aaron, David, >>> >>> The data flow is complex for this coverity issues [1], at least I >>> can't confirm that change fixes the issue. >>> >>> Are you aware of any way to confirm this coverity issue before merging it? >> >> Not generically. :-/ >> >> We need someone that understands the data flow and the coverity splat to >> know that the fix is correct. Coverity even ratelimits how many outstanding >> submissions we can post, iirc, so we don't get to push patch sets (unless we >> pay? I don't recall if there's an option for that). > > This fix is looks good for me. The fdt_get_reg() function just read out the content of some items from DTS file, > We call the libfdt library API to do this. > The Coverity just assume some attacker broken the DTS file or invoke the function with arbitrary values, it is not safety, > So this patch add some checking after the function return. > Hi Tianfei, Wei, Rosen, The defect is reported in Coverity, as concerned, can you please look at it? >> >>> [1] >>> https://scan4.coverity.com/reports.htm#v26325/p10075/fileInstanceId=10 >>> 0181086&defectInstanceId=14238477&mergedDefectId=367480 >