diff mbox series

[v1,1/2] raw/ifpga/base: use untained variable as argument

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

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Wei Huang March 17, 2021, 8:23 a.m. UTC 
  In fme_spi_init(), passing tainted expression "fme->max10_dev"
to function "intel_max10_device_remove" has risk. Untainted
variable "max10" should be used.

Coverity issue: 367480
Fixes: 96ebfcf8125c ("raw/ifpga/base: add SPI and MAX10 device driver")

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

Comments

Zhang, Tianfei April 1, 2021, 7:48 a.m. UTC | #1 
> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: 2021年3月17日 16:24
> 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/2] raw/ifpga/base: use untained variable as argument
> 
> In fme_spi_init(), passing tainted expression "fme->max10_dev"
> to function "intel_max10_device_remove" has risk. Untainted variable
> "max10" should be used.
> 
> Coverity issue: 367480
> Fixes: 96ebfcf8125c ("raw/ifpga/base: add SPI and MAX10 device driver")
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
>  drivers/raw/ifpga/base/ifpga_fme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/raw/ifpga/base/ifpga_fme.c
> b/drivers/raw/ifpga/base/ifpga_fme.c
> index 43c7b9c3dc..a63f90331f 100644
> --- a/drivers/raw/ifpga/base/ifpga_fme.c
> +++ b/drivers/raw/ifpga/base/ifpga_fme.c
> @@ -1001,7 +1001,7 @@ static int fme_spi_init(struct ifpga_feature
> *feature)
>  	return ret;
> 
>  max10_fail:
> -	intel_max10_device_remove(fme->max10_dev);
> +	intel_max10_device_remove(max10);
>  spi_fail:
>  	altera_spi_release(spi_master);
>  	return ret;
> --
Acked-by: Tianfei zhang <Tianfei.zhang@intel.com>
Xu, Rosen April 1, 2021, 8:48 a.m. UTC | #2 
Hi,

-----Original Message-----
From: Huang, Wei <wei.huang@intel.com> 
Sent: Wednesday, March 17, 2021 4:24 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/2] raw/ifpga/base: use untained variable as argument

In fme_spi_init(), passing tainted expression "fme->max10_dev"
to function "intel_max10_device_remove" has risk. Untainted variable "max10" should be used.

Coverity issue: 367480
Fixes: 96ebfcf8125c ("raw/ifpga/base: add SPI and MAX10 device driver")

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

diff --git a/drivers/raw/ifpga/base/ifpga_fme.c b/drivers/raw/ifpga/base/ifpga_fme.c
index 43c7b9c3dc..a63f90331f 100644
--- a/drivers/raw/ifpga/base/ifpga_fme.c
+++ b/drivers/raw/ifpga/base/ifpga_fme.c
@@ -1001,7 +1001,7 @@ static int fme_spi_init(struct ifpga_feature *feature)
 	return ret;
 
 max10_fail:
-	intel_max10_device_remove(fme->max10_dev);
+	intel_max10_device_remove(max10);
 spi_fail:
 	altera_spi_release(spi_master);
 	return ret;
--
2.29.2

Acked-by: Rosen Xu <rosen.xu@intel.com>
Ferruh Yigit April 7, 2021, 2:57 p.m. UTC | #3 
On 3/17/2021 8:23 AM, Wei Huang wrote:
> In fme_spi_init(), passing tainted expression "fme->max10_dev"
> to function "intel_max10_device_remove" has risk. Untainted
> variable "max10" should be used.
> 
> Coverity issue: 367480
> Fixes: 96ebfcf8125c ("raw/ifpga/base: add SPI and MAX10 device driver")
> 

Following is from the coverity:
2. tainted_return_value: Function intel_max10_device_probe returns tainted data.
3. var_assign: Assigning: max10 = intel_max10_device_probe(spi_master, 0), which 
taints max10
5. var_assign_var: Assigning: fme->max10_dev = max10. Both are now tainted.


'max10' is tainted at first place, and 'fme->max10_dev' is tainted because of 
'max10'. In this case how replacing 'fme->max10_dev' with 'max10' helps?

Did you verified that change is fixing the coverity issue?


And as the previous one, what is the exact problem, what is tainted data and how 
it is a problem, can you please describe?

> Signed-off-by: Wei Huang <wei.huang@intel.com>
> ---
>   drivers/raw/ifpga/base/ifpga_fme.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/raw/ifpga/base/ifpga_fme.c b/drivers/raw/ifpga/base/ifpga_fme.c
> index 43c7b9c3dc..a63f90331f 100644
> --- a/drivers/raw/ifpga/base/ifpga_fme.c
> +++ b/drivers/raw/ifpga/base/ifpga_fme.c
> @@ -1001,7 +1001,7 @@ static int fme_spi_init(struct ifpga_feature *feature)
>   	return ret;
>   
>   max10_fail:
> -	intel_max10_device_remove(fme->max10_dev);
> +	intel_max10_device_remove(max10);
>   spi_fail:
>   	altera_spi_release(spi_master);
>   	return ret;
>
diff mbox series

Patch

diff --git a/drivers/raw/ifpga/base/ifpga_fme.c b/drivers/raw/ifpga/base/ifpga_fme.c
index 43c7b9c3dc..a63f90331f 100644
--- a/drivers/raw/ifpga/base/ifpga_fme.c
+++ b/drivers/raw/ifpga/base/ifpga_fme.c
@@ -1001,7 +1001,7 @@  static int fme_spi_init(struct ifpga_feature *feature)
 	return ret;
 
 max10_fail:
-	intel_max10_device_remove(fme->max10_dev);
+	intel_max10_device_remove(max10);
 spi_fail:
 	altera_spi_release(spi_master);
 	return ret;