diff mbox series

[v1,1/4] raw/ifpga/base: use trusted buffer to free

Message ID 1615969296-17021-2-git-send-email-wei.huang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series Fix coverity issues reported in DPDK-26380 |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Wei Huang March 17, 2021, 8:21 a.m. UTC 
  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(-)

Comments

Zhang, Tianfei April 1, 2021, 7:46 a.m. UTC | #1 
> -----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
Xu, Rosen April 1, 2021, 8:47 a.m. UTC | #2 
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>
Ferruh Yigit April 7, 2021, 1:59 p.m. UTC | #3 
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;
>   }
>
diff mbox series

Patch

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;
 }