From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id D8C7BA00E6 for ; Fri, 17 May 2019 07:43:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A29CD1D7; Fri, 17 May 2019 07:43:56 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 2BFC21D7 for ; Fri, 17 May 2019 07:43:53 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 May 2019 22:43:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,478,1549958400"; d="scan'208";a="172659691" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga002.fm.intel.com with ESMTP; 16 May 2019 22:43:52 -0700 Received: from fmsmsx113.amr.corp.intel.com (10.18.116.7) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 16 May 2019 22:43:51 -0700 Received: from cdsmsx101.ccr.corp.intel.com (172.17.3.36) by FMSMSX113.amr.corp.intel.com (10.18.116.7) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 16 May 2019 22:43:51 -0700 Received: from cdsmsx104.ccr.corp.intel.com ([169.254.4.124]) by CDSMSX101.ccr.corp.intel.com ([169.254.1.179]) with mapi id 14.03.0415.000; Fri, 17 May 2019 13:43:49 +0800 From: "Zhang, Tianfei" To: "Xu, Rosen" CC: "Pei, Andy" , "stable@dpdk.org" Thread-Topic: [PATCH] raw/ifpga: fix use of untrusted scalar value Thread-Index: AQHVDE6fdqcOIvBDPUykv7LwB9dZQaZuOCCAgACWD3A= Date: Fri, 17 May 2019 05:43:48 +0000 Message-ID: References: <20190517090557.31510-1-tianfei.zhang@intel.com> <0E78D399C70DA940A335608C6ED296D73A75314B@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <0E78D399C70DA940A335608C6ED296D73A75314B@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZGM4ODQwMGEtMTA4Ny00ZDNkLTllNTYtOTMyY2U3NzBjYWZhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJjUHFvRXlPMEZLekNiRmFcL2QyXC9CN0JoYSs1N0VYZ1FuTFFhd3VnYXl2NThISVV1MVhCdUhvVW5ubmduQytxc2cifQ== x-ctpclassification: CTP_NT x-originating-ip: [172.17.6.105] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-stable] [PATCH] raw/ifpga: fix use of untrusted scalar value X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" > -----Original Message----- > From: Xu, Rosen > Sent: Friday, May 17, 2019 12:46 PM > To: Zhang, Tianfei > Cc: Pei, Andy ; stable@dpdk.org > Subject: RE: [PATCH] raw/ifpga: fix use of untrusted scalar value >=20 > Hi Tianfei, >=20 > For Coverity issue: 279449, my opinion is to check buffer size not only t= ake > const to project content of buffer. This content of buffer cannot be change, so use const is better. >=20 > > -----Original Message----- > > From: Zhang, Tianfei > > Sent: Friday, May 17, 2019 17:06 > > To: Xu, Rosen > > Cc: Pei, Andy ; Zhang, Tianfei > > ; stable@dpdk.org; Zhang > > Subject: [PATCH] raw/ifpga: fix use of untrusted scalar value > > > > Add checking the buffer size and use > > const char * for buffer declaration. > > > > Coverity issue: 279449 > > Cc: stable@dpdk.org > > > > Signed-off-by: Zhang, Tianfei > > --- > > drivers/raw/ifpga_rawdev/base/ifpga_api.c | 4 +-- > > drivers/raw/ifpga_rawdev/base/ifpga_api.h | 2 +- > > .../raw/ifpga_rawdev/base/ifpga_feature_dev.h | 2 +- > > drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c | 27 +++++++++++-------- > > drivers/raw/ifpga_rawdev/base/opae_hw_api.c | 4 +-- > > drivers/raw/ifpga_rawdev/base/opae_hw_api.h | 4 +-- > > drivers/raw/ifpga_rawdev/ifpga_rawdev.c | 7 ++++- > > 7 files changed, 30 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_api.c > > b/drivers/raw/ifpga_rawdev/base/ifpga_api.c > > index 3ddbcdc2a..53d101daf 100644 > > --- a/drivers/raw/ifpga_rawdev/base/ifpga_api.c > > +++ b/drivers/raw/ifpga_rawdev/base/ifpga_api.c > > @@ -182,7 +182,7 @@ struct opae_bridge_ops ifpga_br_ops =3D { }; > > > > /* Manager APIs */ > > -static int ifpga_mgr_flash(struct opae_manager *mgr, int id, void > > *buf, > > +static int ifpga_mgr_flash(struct opae_manager *mgr, int id, const > > +char *buf, > > u32 size, u64 *status) > > { > > struct ifpga_fme_hw *fme =3D mgr->data; @@ -324,7 +324,7 @@ struct > > opae_adapter_ops ifpga_adapter_ops =3D { > > * - 0: Success, partial reconfiguration finished. > > * - <0: Error code returned in partial reconfiguration. > > **/ > > -int ifpga_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 > > size, > > +int ifpga_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer, > > +u32 size, > > u64 *status) > > { > > if (!is_valid_port_id(hw, port_id)) > > diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_api.h > > b/drivers/raw/ifpga_rawdev/base/ifpga_api.h > > index 4a247698c..051ab8276 100644 > > --- a/drivers/raw/ifpga_rawdev/base/ifpga_api.h > > +++ b/drivers/raw/ifpga_rawdev/base/ifpga_api.h > > @@ -23,7 +23,7 @@ int ifpga_set_irq(struct ifpga_hw *hw, u32 fiu_id, > > u32 port_id, > > u32 feature_id, void *irq_set); > > > > /* FME APIs */ > > -int ifpga_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 > > size, > > +int ifpga_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer, > > +u32 size, > > u64 *status); > > > > #endif /* _IFPGA_API_H_ */ > > diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.h > > b/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.h > > index bb9fcc289..e243d4273 100644 > > --- a/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.h > > +++ b/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.h > > @@ -149,7 +149,7 @@ static inline int fpga_port_reset(struct > > ifpga_port_hw > > *port) > > return ret; > > } > > > > -int do_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 size, > > +int do_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer, u32 > > +size, > > u64 *status); > > > > int fme_get_prop(struct ifpga_fme_hw *fme, struct feature_prop > > *prop); diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c > > b/drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c > > index efa72660f..9997942d2 100644 > > --- a/drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c > > +++ b/drivers/raw/ifpga_rawdev/base/ifpga_fme_pr.c > > @@ -223,8 +223,8 @@ static int fpga_pr_buf_load(struct ifpga_fme_hw > > *fme_dev, > > return 0; > > } > > > > -static int fme_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 = size, > > - u64 *status) > > +static int fme_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer= , > > + u32 size, u64 *status) > > { > > struct feature_fme_header *fme_hdr; > > struct feature_fme_capability fme_capability; @@ -269,7 +269,7 @@ > > static int fme_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 s= ize, > > /* Disable Port before PR */ > > fpga_port_disable(port); > > > > - ret =3D fpga_pr_buf_load(fme, &info, (void *)buffer, size); > > + ret =3D fpga_pr_buf_load(fme, &info, buffer, size); > > > > *status =3D info.pr_err; > > > > @@ -280,27 +280,32 @@ static int fme_pr(struct ifpga_hw *hw, u32 > > port_id, void *buffer, u32 size, > > return ret; > > } > > > > -int do_pr(struct ifpga_hw *hw, u32 port_id, void *buffer, u32 size, > > u64 > > *status) > > +int do_pr(struct ifpga_hw *hw, u32 port_id, const char *buffer, > > + u32 size, u64 *status) > > { > > - struct bts_header *bts_hdr; > > - void *buf; > > + const struct bts_header *bts_hdr; > > + const char *buf; > > struct ifpga_port_hw *port; > > int ret; > > + u32 header_size; > > > > if (!buffer || size =3D=3D 0) { > > dev_err(hw, "invalid parameter\n"); > > return -EINVAL; > > } > > > > - bts_hdr =3D (struct bts_header *)buffer; > > + bts_hdr =3D (const struct bts_header *)buffer; > > > > if (is_valid_bts(bts_hdr)) { > > dev_info(hw, "this is a valid bitsteam..\n"); > > - size -=3D (sizeof(struct bts_header) + > > - bts_hdr->metadata_len); > > - buf =3D (u8 *)buffer + sizeof(struct bts_header) + > > - bts_hdr->metadata_len; > > + header_size =3D sizeof(struct bts_header) + > > + bts_hdr->metadata_len; > > + if (size < header_size) > > + return -EINVAL; > > + size -=3D header_size; > > + buf =3D buffer + header_size; > > } else { > > + dev_err(hw, "this is an invalid bitstream..\n"); > > return -EINVAL; > > } > > > > diff --git a/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > > b/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > > index 0e117d05e..8964e7984 100644 > > --- a/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > > +++ b/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > > @@ -244,8 +244,8 @@ opae_manager_alloc(const char *name, struct > > opae_manager_ops *ops, > > * > > * Return: 0 on success, otherwise error code. > > */ > > -int opae_manager_flash(struct opae_manager *mgr, int id, void *buf, > > u32 size, > > - u64 *status) > > +int opae_manager_flash(struct opae_manager *mgr, int id, const char > *buf, > > + u32 size, u64 *status) > > { > > if (!mgr) > > return -EINVAL; > > diff --git a/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > > b/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > > index 383e751cb..63405a471 100644 > > --- a/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > > +++ b/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > > @@ -44,7 +44,7 @@ struct opae_manager { > > > > /* FIXME: add more management ops, e.g power/thermal and etc */ > > struct opae_manager_ops { > > - int (*flash)(struct opae_manager *mgr, int id, void *buffer, > > + int (*flash)(struct opae_manager *mgr, int id, const char *buffer, > > u32 size, u64 *status); > > int (*get_eth_group_region_info)(struct opae_manager *mgr, > > struct opae_eth_group_region_info *info); @@ -74,7 > > +74,7 @@ struct opae_manager * opae_manager_alloc(const char > *name, > > struct opae_manager_ops *ops, > > struct opae_manager_networking_ops *network_ops, void *data); > > #define opae_manager_free(mgr) opae_free(mgr) -int > > opae_manager_flash(struct opae_manager *mgr, int acc_id, void *buf, > > +int opae_manager_flash(struct opae_manager *mgr, int acc_id, const > > +char *buf, > > u32 size, u64 *status); > > int opae_manager_get_eth_group_region_info(struct opae_manager > *mgr, > > u8 group_id, struct opae_eth_group_region_info *info); diff - -git > > a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > > b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > > index 41be1a205..01aa917de 100644 > > --- a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > > +++ b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > > @@ -225,7 +225,7 @@ ifpga_rawdev_reset(struct rte_rawdev *dev) } > > > > static int > > -fpga_pr(struct rte_rawdev *raw_dev, u32 port_id, u64 *buffer, u32 > > size, > > +fpga_pr(struct rte_rawdev *raw_dev, u32 port_id, const char *buffer, > > +u32 size, > > u64 *status) > > { > > > > @@ -296,6 +296,11 @@ rte_fpga_do_pr(struct rte_rawdev *rawdev, int > > port_id, > > goto close_fd; > > } > > buffer_size =3D file_stat.st_size; > > + if (buffer_size <=3D 0) { > > + ret =3D -EINVAL; > > + goto close_fd; > > + } > > + > > IFPGA_RAWDEV_PMD_INFO("bitstream file size: %zu\n", buffer_size); > > buffer =3D rte_malloc(NULL, buffer_size, 0); > > if (!buffer) { > > -- > > 2.17.1