patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380
@ 2021-03-17  8:21 Wei Huang
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free Wei Huang
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Wei Huang @ 2021-03-17  8:21 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, Wei Huang

Below coverity issues are fixed in this patch set.
367477, 367478, 367481, 367483

Wei Huang (4):
  raw/ifpga/base: use trusted buffer to free
  raw/ifpga/base: check return value of lseek
  raw/ifpga/base: assign unsigned value to length
  raw/ifpga/base: check pointer before dereferencing

 drivers/raw/ifpga/base/ifpga_fme_rsu.c | 26 +++++++++++++++++++++++---
 drivers/raw/ifpga/base/ifpga_sec_mgr.c |  2 +-
 2 files changed, 24 insertions(+), 4 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free
  2021-03-17  8:21 [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380 Wei Huang
@ 2021-03-17  8:21 ` Wei Huang
  2021-04-01  7:46   ` Zhang, Tianfei
                     ` (2 more replies)
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 2/4] raw/ifpga/base: check return value of lseek Wei Huang
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 15+ messages in thread
From: Wei Huang @ 2021-03-17  8:21 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, Wei Huang

In write_flash_image(), calling function "read" may taints variable
"buf" which turn to an untrusted value as argument of "rte_free".

Coverity issue: 367477
Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_fme_rsu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
index 28198abd78..d32f1eccb1 100644
--- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
+++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
@@ -92,6 +92,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
 	uint32_t offset)
 {
 	void *buf = NULL;
+	void *buf_to_free = NULL;
 	int retry = 0;
 	uint32_t length = 0;
 	uint32_t to_transfer = 0;
@@ -122,6 +123,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
 		close(fd);
 		return -ENOMEM;
 	}
+	buf_to_free = buf;
 
 	length = smgr->rsu_length;
 	one_percent = length / 100;
@@ -177,7 +179,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
 	printf("\n");
 
 end:
-	free(buf);
+	free(buf_to_free);
 	close(fd);
 	return ret;
 }
-- 
2.29.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-stable] [PATCH v1 2/4] raw/ifpga/base: check return value of lseek
  2021-03-17  8:21 [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380 Wei Huang
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free Wei Huang
@ 2021-03-17  8:21 ` Wei Huang
  2021-04-01  7:46   ` Zhang, Tianfei
  2021-04-01  8:47   ` Xu, Rosen
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length Wei Huang
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Wei Huang @ 2021-03-17  8:21 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, Wei Huang

In write_flash_image(), calling function "lseek" without checking
return value has risk. Negative return value should be handled as
an error condition.

Coverity issue: 367478
Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_fme_rsu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
index d32f1eccb1..a4cb2f54ba 100644
--- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
+++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
@@ -130,7 +130,12 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
 	do {
 		to_transfer = (length > IFPGA_RSU_DATA_BLK_SIZE) ?
 			IFPGA_RSU_DATA_BLK_SIZE : length;
-		lseek(fd, offset, SEEK_SET);
+		if (lseek(fd, offset, SEEK_SET) < 0) {
+			dev_err(smgr, "Failed to seek in \'%s\' [e:%s]\n",
+				image, strerror(errno));
+			ret = -EIO;
+			goto end;
+		}
 		read_size = read(fd, buf, to_transfer);
 		if (read_size < 0) {
 			dev_err(smgr, "Failed to read from \'%s\' [e:%s]\n",
-- 
2.29.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-stable] [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length
  2021-03-17  8:21 [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380 Wei Huang
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free Wei Huang
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 2/4] raw/ifpga/base: check return value of lseek Wei Huang
@ 2021-03-17  8:21 ` Wei Huang
  2021-04-01  7:47   ` Zhang, Tianfei
  2021-04-01  8:47   ` Xu, Rosen
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing Wei Huang
  2021-04-01 11:50 ` [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380 Zhang, Qi Z
  4 siblings, 2 replies; 15+ messages in thread
From: Wei Huang @ 2021-03-17  8:21 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, Wei Huang

In fpga_update_flash(), "smgr->rsu_length" is passed to a
parameter that cannot be negative. So return value of
function "lseek" should be checked before being assigned
to "smgr->rsu_length".

Coverity issue: 367481
Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_fme_rsu.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
index a4cb2f54ba..79ee37c282 100644
--- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
+++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
@@ -277,6 +277,7 @@ int fpga_update_flash(struct ifpga_fme_hw *fme, const char *image,
 	struct ifpga_sec_mgr *smgr = NULL;
 	uint32_t rsu_stat = 0;
 	int fd = -1;
+	off_t len = 0;
 	struct sigaction old_sigint_action;
 	struct sigaction sa;
 	time_t start;
@@ -320,9 +321,21 @@ int fpga_update_flash(struct ifpga_fme_hw *fme, const char *image,
 			image, strerror(errno));
 		return -EIO;
 	}
-	smgr->rsu_length = lseek(fd, 0, SEEK_END);
+	len = lseek(fd, 0, SEEK_END);
 	close(fd);
 
+	if (len < 0) {
+		dev_err(smgr,
+			"Failed to get file length of \'%s\' [e:%s]\n",
+			image, strerror(errno));
+		return -EIO;
+	}
+	if (len == 0) {
+		dev_err(smgr, "Length of file \'%s\' is invalid\n", image);
+		return -EINVAL;
+	}
+	smgr->rsu_length = len;
+
 	if (smgr->max10_dev->staging_area_size < smgr->rsu_length) {
 		dev_err(dev, "Size of staging area is small than image length "
 			"[%u<%u]\n", smgr->max10_dev->staging_area_size,
-- 
2.29.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-stable] [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing
  2021-03-17  8:21 [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380 Wei Huang
                   ` (2 preceding siblings ...)
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length Wei Huang
@ 2021-03-17  8:21 ` Wei Huang
  2021-04-01  7:47   ` Zhang, Tianfei
  2021-04-01  8:48   ` Xu, Rosen
  2021-04-01 11:50 ` [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380 Zhang, Qi Z
  4 siblings, 2 replies; 15+ messages in thread
From: Wei Huang @ 2021-03-17  8:21 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang, Wei Huang

In init_sec_mgr(), pointer "hw" may be NULL, so "hw" should
be checked before dereferencing.

Coverity issue: 367483
Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_sec_mgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/base/ifpga_sec_mgr.c b/drivers/raw/ifpga/base/ifpga_sec_mgr.c
index 4cf1db3049..15fb5b6d4d 100644
--- a/drivers/raw/ifpga/base/ifpga_sec_mgr.c
+++ b/drivers/raw/ifpga/base/ifpga_sec_mgr.c
@@ -610,7 +610,7 @@ int init_sec_mgr(struct ifpga_fme_hw *fme)
 		smgr->rsu_status = NULL;
 	}
 
-	if ((hw->pci_data->device_id == IFPGA_N3000_DID) &&
+	if (hw && (hw->pci_data->device_id == IFPGA_N3000_DID) &&
 		(hw->pci_data->vendor_id == IFPGA_N3000_VID)) {
 		smgr->ops = &n3000_sec_ops;
 		smgr->copy_speed = IFPGA_N3000_COPY_SPEED;
-- 
2.29.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free Wei Huang
@ 2021-04-01  7:46   ` Zhang, Tianfei
  2021-04-01  8:47   ` Xu, Rosen
  2021-04-07 13:59   ` Ferruh Yigit
  2 siblings, 0 replies; 15+ messages in thread
From: Zhang, Tianfei @ 2021-04-01  7:46 UTC (permalink / raw)
  To: Huang, Wei, dev, Xu, Rosen, Zhang, Qi Z; +Cc: stable



> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: 2021年3月17日 16:22
> To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang, Wei
> <wei.huang@intel.com>
> Subject: [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free
> 
> In write_flash_image(), calling function "read" may taints variable "buf" which
> turn to an untrusted value as argument of "rte_free".
> 
> Coverity issue: 367477
> Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
>  drivers/raw/ifpga/base/ifpga_fme_rsu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> index 28198abd78..d32f1eccb1 100644
> --- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> +++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> @@ -92,6 +92,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr,
> const char *image,
>  	uint32_t offset)
>  {
>  	void *buf = NULL;
> +	void *buf_to_free = NULL;
>  	int retry = 0;
>  	uint32_t length = 0;
>  	uint32_t to_transfer = 0;
> @@ -122,6 +123,7 @@ static int write_flash_image(struct ifpga_sec_mgr
> *smgr, const char *image,
>  		close(fd);
>  		return -ENOMEM;
>  	}
> +	buf_to_free = buf;
> 
>  	length = smgr->rsu_length;
>  	one_percent = length / 100;
> @@ -177,7 +179,7 @@ static int write_flash_image(struct ifpga_sec_mgr
> *smgr, const char *image,
>  	printf("\n");
> 
>  end:
> -	free(buf);
> +	free(buf_to_free);
>  	close(fd);
>  	return ret;
>  }

Acked-by: Tianfei zhang <Tianfei.zhang@intel.com>

> --
> 2.29.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 2/4] raw/ifpga/base: check return value of lseek
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 2/4] raw/ifpga/base: check return value of lseek Wei Huang
@ 2021-04-01  7:46   ` Zhang, Tianfei
  2021-04-01  8:47   ` Xu, Rosen
  1 sibling, 0 replies; 15+ messages in thread
From: Zhang, Tianfei @ 2021-04-01  7:46 UTC (permalink / raw)
  To: Huang, Wei, dev, Xu, Rosen, Zhang, Qi Z; +Cc: stable



> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: 2021年3月17日 16:22
> To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang, Wei
> <wei.huang@intel.com>
> Subject: [PATCH v1 2/4] raw/ifpga/base: check return value of lseek
> 
> In write_flash_image(), calling function "lseek" without checking return value
> has risk. Negative return value should be handled as an error condition.
> 
> Coverity issue: 367478
> Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
>  drivers/raw/ifpga/base/ifpga_fme_rsu.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> index d32f1eccb1..a4cb2f54ba 100644
> --- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> +++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> @@ -130,7 +130,12 @@ static int write_flash_image(struct ifpga_sec_mgr
> *smgr, const char *image,
>  	do {
>  		to_transfer = (length > IFPGA_RSU_DATA_BLK_SIZE) ?
>  			IFPGA_RSU_DATA_BLK_SIZE : length;
> -		lseek(fd, offset, SEEK_SET);
> +		if (lseek(fd, offset, SEEK_SET) < 0) {
> +			dev_err(smgr, "Failed to seek in \'%s\' [e:%s]\n",
> +				image, strerror(errno));
> +			ret = -EIO;
> +			goto end;
> +		}
>  		read_size = read(fd, buf, to_transfer);
>  		if (read_size < 0) {
>  			dev_err(smgr, "Failed to read from \'%s\' [e:%s]\n",
> --

Acked-by: Tianfei zhang <Tianfei.zhang@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length Wei Huang
@ 2021-04-01  7:47   ` Zhang, Tianfei
  2021-04-01  8:47   ` Xu, Rosen
  1 sibling, 0 replies; 15+ messages in thread
From: Zhang, Tianfei @ 2021-04-01  7:47 UTC (permalink / raw)
  To: Huang, Wei, dev, Xu, Rosen, Zhang, Qi Z; +Cc: stable



> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: 2021年3月17日 16:22
> To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang, Wei
> <wei.huang@intel.com>
> Subject: [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length
> 
> In fpga_update_flash(), "smgr->rsu_length" is passed to a parameter that
> cannot be negative. So return value of function "lseek" should be checked
> before being assigned to "smgr->rsu_length".
> 
> Coverity issue: 367481
> Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
>  drivers/raw/ifpga/base/ifpga_fme_rsu.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> index a4cb2f54ba..79ee37c282 100644
> --- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> +++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> @@ -277,6 +277,7 @@ int fpga_update_flash(struct ifpga_fme_hw *fme,
> const char *image,
>  	struct ifpga_sec_mgr *smgr = NULL;
>  	uint32_t rsu_stat = 0;
>  	int fd = -1;
> +	off_t len = 0;
>  	struct sigaction old_sigint_action;
>  	struct sigaction sa;
>  	time_t start;
> @@ -320,9 +321,21 @@ int fpga_update_flash(struct ifpga_fme_hw *fme,
> const char *image,
>  			image, strerror(errno));
>  		return -EIO;
>  	}
> -	smgr->rsu_length = lseek(fd, 0, SEEK_END);
> +	len = lseek(fd, 0, SEEK_END);
>  	close(fd);
> 
> +	if (len < 0) {
> +		dev_err(smgr,
> +			"Failed to get file length of \'%s\' [e:%s]\n",
> +			image, strerror(errno));
> +		return -EIO;
> +	}
> +	if (len == 0) {
> +		dev_err(smgr, "Length of file \'%s\' is invalid\n", image);
> +		return -EINVAL;
> +	}
> +	smgr->rsu_length = len;
> +
>  	if (smgr->max10_dev->staging_area_size < smgr->rsu_length) {
>  		dev_err(dev, "Size of staging area is small than image length "
>  			"[%u<%u]\n", smgr->max10_dev->staging_area_size,
> --

Acked-by: Tianfei zhang <Tianfei.zhang@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing Wei Huang
@ 2021-04-01  7:47   ` Zhang, Tianfei
  2021-04-01  8:48   ` Xu, Rosen
  1 sibling, 0 replies; 15+ messages in thread
From: Zhang, Tianfei @ 2021-04-01  7:47 UTC (permalink / raw)
  To: Huang, Wei, dev, Xu, Rosen, Zhang, Qi Z; +Cc: stable



> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: 2021年3月17日 16:22
> To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang, Wei
> <wei.huang@intel.com>
> Subject: [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing
> 
> In init_sec_mgr(), pointer "hw" may be NULL, so "hw" should be checked
> before dereferencing.
> 
> Coverity issue: 367483
> Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
>  drivers/raw/ifpga/base/ifpga_sec_mgr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/raw/ifpga/base/ifpga_sec_mgr.c
> b/drivers/raw/ifpga/base/ifpga_sec_mgr.c
> index 4cf1db3049..15fb5b6d4d 100644
> --- a/drivers/raw/ifpga/base/ifpga_sec_mgr.c
> +++ b/drivers/raw/ifpga/base/ifpga_sec_mgr.c
> @@ -610,7 +610,7 @@ int init_sec_mgr(struct ifpga_fme_hw *fme)
>  		smgr->rsu_status = NULL;
>  	}
> 
> -	if ((hw->pci_data->device_id == IFPGA_N3000_DID) &&
> +	if (hw && (hw->pci_data->device_id == IFPGA_N3000_DID) &&
>  		(hw->pci_data->vendor_id == IFPGA_N3000_VID)) {
>  		smgr->ops = &n3000_sec_ops;
>  		smgr->copy_speed = IFPGA_N3000_COPY_SPEED;
> --

Acked-by: Tianfei zhang <Tianfei.zhang@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free Wei Huang
  2021-04-01  7:46   ` Zhang, Tianfei
@ 2021-04-01  8:47   ` Xu, Rosen
  2021-04-07 13:59   ` Ferruh Yigit
  2 siblings, 0 replies; 15+ messages in thread
From: Xu, Rosen @ 2021-04-01  8:47 UTC (permalink / raw)
  To: Huang, Wei, dev, Zhang,  Qi Z; +Cc: stable, Zhang, Tianfei

Hi,

-----Original Message-----
From: Huang, Wei <wei.huang@intel.com> 
Sent: Wednesday, March 17, 2021 4:22 PM
To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang, Wei <wei.huang@intel.com>
Subject: [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free

In write_flash_image(), calling function "read" may taints variable "buf" which turn to an untrusted value as argument of "rte_free".

Coverity issue: 367477
Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_fme_rsu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
index 28198abd78..d32f1eccb1 100644
--- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
+++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
@@ -92,6 +92,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
 	uint32_t offset)
 {
 	void *buf = NULL;
+	void *buf_to_free = NULL;
 	int retry = 0;
 	uint32_t length = 0;
 	uint32_t to_transfer = 0;
@@ -122,6 +123,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
 		close(fd);
 		return -ENOMEM;
 	}
+	buf_to_free = buf;
 
 	length = smgr->rsu_length;
 	one_percent = length / 100;
@@ -177,7 +179,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
 	printf("\n");
 
 end:
-	free(buf);
+	free(buf_to_free);
 	close(fd);
 	return ret;
 }
--
2.29.2

Acked-by: Rosen Xu <rosen.xu@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 2/4] raw/ifpga/base: check return value of lseek
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 2/4] raw/ifpga/base: check return value of lseek Wei Huang
  2021-04-01  7:46   ` Zhang, Tianfei
@ 2021-04-01  8:47   ` Xu, Rosen
  1 sibling, 0 replies; 15+ messages in thread
From: Xu, Rosen @ 2021-04-01  8:47 UTC (permalink / raw)
  To: Huang, Wei, dev, Zhang,  Qi Z; +Cc: stable, Zhang, Tianfei

Hi,

-----Original Message-----
From: Huang, Wei <wei.huang@intel.com> 
Sent: Wednesday, March 17, 2021 4:22 PM
To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang, Wei <wei.huang@intel.com>
Subject: [PATCH v1 2/4] raw/ifpga/base: check return value of lseek

In write_flash_image(), calling function "lseek" without checking return value has risk. Negative return value should be handled as an error condition.

Coverity issue: 367478
Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_fme_rsu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
index d32f1eccb1..a4cb2f54ba 100644
--- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
+++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
@@ -130,7 +130,12 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
 	do {
 		to_transfer = (length > IFPGA_RSU_DATA_BLK_SIZE) ?
 			IFPGA_RSU_DATA_BLK_SIZE : length;
-		lseek(fd, offset, SEEK_SET);
+		if (lseek(fd, offset, SEEK_SET) < 0) {
+			dev_err(smgr, "Failed to seek in \'%s\' [e:%s]\n",
+				image, strerror(errno));
+			ret = -EIO;
+			goto end;
+		}
 		read_size = read(fd, buf, to_transfer);
 		if (read_size < 0) {
 			dev_err(smgr, "Failed to read from \'%s\' [e:%s]\n",
--
2.29.2

Acked-by: Rosen Xu <rosen.xu@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length Wei Huang
  2021-04-01  7:47   ` Zhang, Tianfei
@ 2021-04-01  8:47   ` Xu, Rosen
  1 sibling, 0 replies; 15+ messages in thread
From: Xu, Rosen @ 2021-04-01  8:47 UTC (permalink / raw)
  To: Huang, Wei, dev, Zhang,  Qi Z; +Cc: stable, Zhang, Tianfei

Hi,

-----Original Message-----
From: Huang, Wei <wei.huang@intel.com> 
Sent: Wednesday, March 17, 2021 4:22 PM
To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang, Wei <wei.huang@intel.com>
Subject: [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length

In fpga_update_flash(), "smgr->rsu_length" is passed to a parameter that cannot be negative. So return value of function "lseek" should be checked before being assigned to "smgr->rsu_length".

Coverity issue: 367481
Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_fme_rsu.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
index a4cb2f54ba..79ee37c282 100644
--- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
+++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
@@ -277,6 +277,7 @@ int fpga_update_flash(struct ifpga_fme_hw *fme, const char *image,
 	struct ifpga_sec_mgr *smgr = NULL;
 	uint32_t rsu_stat = 0;
 	int fd = -1;
+	off_t len = 0;
 	struct sigaction old_sigint_action;
 	struct sigaction sa;
 	time_t start;
@@ -320,9 +321,21 @@ int fpga_update_flash(struct ifpga_fme_hw *fme, const char *image,
 			image, strerror(errno));
 		return -EIO;
 	}
-	smgr->rsu_length = lseek(fd, 0, SEEK_END);
+	len = lseek(fd, 0, SEEK_END);
 	close(fd);
 
+	if (len < 0) {
+		dev_err(smgr,
+			"Failed to get file length of \'%s\' [e:%s]\n",
+			image, strerror(errno));
+		return -EIO;
+	}
+	if (len == 0) {
+		dev_err(smgr, "Length of file \'%s\' is invalid\n", image);
+		return -EINVAL;
+	}
+	smgr->rsu_length = len;
+
 	if (smgr->max10_dev->staging_area_size < smgr->rsu_length) {
 		dev_err(dev, "Size of staging area is small than image length "
 			"[%u<%u]\n", smgr->max10_dev->staging_area_size,
--
2.29.2

Acked-by: Rosen Xu <rosen.xu@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing Wei Huang
  2021-04-01  7:47   ` Zhang, Tianfei
@ 2021-04-01  8:48   ` Xu, Rosen
  1 sibling, 0 replies; 15+ messages in thread
From: Xu, Rosen @ 2021-04-01  8:48 UTC (permalink / raw)
  To: Huang, Wei, dev, Zhang,  Qi Z; +Cc: stable, Zhang, Tianfei

Hi,

-----Original Message-----
From: Huang, Wei <wei.huang@intel.com> 
Sent: Wednesday, March 17, 2021 4:22 PM
To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang, Wei <wei.huang@intel.com>
Subject: [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing

In init_sec_mgr(), pointer "hw" may be NULL, so "hw" should be checked before dereferencing.

Coverity issue: 367483
Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")

Signed-off-by: Wei Huang <wei.huang@intel.com>
---
 drivers/raw/ifpga/base/ifpga_sec_mgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/base/ifpga_sec_mgr.c b/drivers/raw/ifpga/base/ifpga_sec_mgr.c
index 4cf1db3049..15fb5b6d4d 100644
--- a/drivers/raw/ifpga/base/ifpga_sec_mgr.c
+++ b/drivers/raw/ifpga/base/ifpga_sec_mgr.c
@@ -610,7 +610,7 @@ int init_sec_mgr(struct ifpga_fme_hw *fme)
 		smgr->rsu_status = NULL;
 	}
 
-	if ((hw->pci_data->device_id == IFPGA_N3000_DID) &&
+	if (hw && (hw->pci_data->device_id == IFPGA_N3000_DID) &&
 		(hw->pci_data->vendor_id == IFPGA_N3000_VID)) {
 		smgr->ops = &n3000_sec_ops;
 		smgr->copy_speed = IFPGA_N3000_COPY_SPEED;
--
2.29.2

Acked-by: Rosen Xu <rosen.xu@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380
  2021-03-17  8:21 [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380 Wei Huang
                   ` (3 preceding siblings ...)
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing Wei Huang
@ 2021-04-01 11:50 ` Zhang, Qi Z
  4 siblings, 0 replies; 15+ messages in thread
From: Zhang, Qi Z @ 2021-04-01 11:50 UTC (permalink / raw)
  To: Huang, Wei, dev, Xu, Rosen; +Cc: stable, Zhang, Tianfei



> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: Wednesday, March 17, 2021 4:22 PM
> To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang, Wei
> <wei.huang@intel.com>
> Subject: [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380
> 
> Below coverity issues are fixed in this patch set.
> 367477, 367478, 367481, 367483
> 
> Wei Huang (4):
>   raw/ifpga/base: use trusted buffer to free
>   raw/ifpga/base: check return value of lseek
>   raw/ifpga/base: assign unsigned value to length
>   raw/ifpga/base: check pointer before dereferencing
> 
>  drivers/raw/ifpga/base/ifpga_fme_rsu.c | 26 +++++++++++++++++++++++---
> drivers/raw/ifpga/base/ifpga_sec_mgr.c |  2 +-
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> --
> 2.29.2

Applied to dpdk-next-net-intel.

Thanks
Qi


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free
  2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free Wei Huang
  2021-04-01  7:46   ` Zhang, Tianfei
  2021-04-01  8:47   ` Xu, Rosen
@ 2021-04-07 13:59   ` Ferruh Yigit
  2 siblings, 0 replies; 15+ messages in thread
From: Ferruh Yigit @ 2021-04-07 13:59 UTC (permalink / raw)
  To: Wei Huang, dev, rosen.xu, qi.z.zhang; +Cc: stable, tianfei.zhang

On 3/17/2021 8:21 AM, Wei Huang wrote:
> In write_flash_image(), calling function "read" may taints variable
> "buf" which turn to an untrusted value as argument of "rte_free".
> 
> Coverity issue: 367477
> Fixes: 7a4f3993f269 ("raw/ifpga: add FPGA RSU APIs")
> 

Hi Huang, Rosen,

I checked the coverity issue but still not clear about the problem. What does 
'read' taints 'buf' mean?
The 'buf' passed as an argument to read, so all 'read' can do is change the 
memory that 'buf' points, so why it should affect the 'free' at all?
If the memory is overflow etc, your change is just hiding the error not fixing it.

And the error message mentions from 'rte_free', not 'free', not sure how 
'rte_free' is involved in the problem, any idea?

> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
>   drivers/raw/ifpga/base/ifpga_fme_rsu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/raw/ifpga/base/ifpga_fme_rsu.c b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> index 28198abd78..d32f1eccb1 100644
> --- a/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> +++ b/drivers/raw/ifpga/base/ifpga_fme_rsu.c
> @@ -92,6 +92,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
>   	uint32_t offset)
>   {
>   	void *buf = NULL;
> +	void *buf_to_free = NULL;
>   	int retry = 0;
>   	uint32_t length = 0;
>   	uint32_t to_transfer = 0;
> @@ -122,6 +123,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
>   		close(fd);
>   		return -ENOMEM;
>   	}
> +	buf_to_free = buf;
>   
>   	length = smgr->rsu_length;
>   	one_percent = length / 100;
> @@ -177,7 +179,7 @@ static int write_flash_image(struct ifpga_sec_mgr *smgr, const char *image,
>   	printf("\n");
>   
>   end:
> -	free(buf);
> +	free(buf_to_free);
>   	close(fd);
>   	return ret;
>   }
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-04-07 13:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-17  8:21 [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380 Wei Huang
2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 1/4] raw/ifpga/base: use trusted buffer to free Wei Huang
2021-04-01  7:46   ` Zhang, Tianfei
2021-04-01  8:47   ` Xu, Rosen
2021-04-07 13:59   ` Ferruh Yigit
2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 2/4] raw/ifpga/base: check return value of lseek Wei Huang
2021-04-01  7:46   ` Zhang, Tianfei
2021-04-01  8:47   ` Xu, Rosen
2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 3/4] raw/ifpga/base: assign unsigned value to length Wei Huang
2021-04-01  7:47   ` Zhang, Tianfei
2021-04-01  8:47   ` Xu, Rosen
2021-03-17  8:21 ` [dpdk-stable] [PATCH v1 4/4] raw/ifpga/base: check pointer before dereferencing Wei Huang
2021-04-01  7:47   ` Zhang, Tianfei
2021-04-01  8:48   ` Xu, Rosen
2021-04-01 11:50 ` [dpdk-stable] [PATCH v1 0/4] Fix coverity issues reported in DPDK-26380 Zhang, Qi Z

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).