Fix coverity issues reported in DPDK-26381

PATCH Applied Wei Huang 5 participants 12 messages Mar 17, 2021 – Apr 20, 2021

lore

Share & tools

lore

Wei Huang

author

2021-03-17 08:23 UTC #0 lore raw

cover Fix coverity issues reported in DPDK-26381

Below coverity issues are fixed in this patch set. 367480, 367482

Wei Huang (2):
raw/ifpga/base: use untained variable as argument raw/ifpga/base: add argument to use untained variable

 drivers/raw/ifpga/base/ifpga_fme.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Signature · 1 line

2.29.2

Wei Huang

author

2021-03-17 08:23 UTC #1 lore raw

PATCH 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 <[email protected]>

---
 drivers/raw/ifpga/base/ifpga_fme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/raw/ifpga/base/ifpga_fme.c +1 -1

index 43c7b9c3dc..a63f90331f 100644
@@ -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;

Signature · 1 line

2.29.2

Zhang, Tianfei

Acked-by on v1 1/2

2021-04-01 07:48 UTC #2 lore raw

Re: [dpdk-dev] [PATCH v1 1/2] raw/ifpga/base: use untained variable as argument

Quoted 37 lines

-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: 2021年3月17日 16:24
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Huang, Wei <[email protected]>
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 <[email protected]>
---
 drivers/raw/ifpga/base/ifpga_fme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/raw/ifpga/base/ifpga_fme.c +1 -1
index 43c7b9c3dc..a63f90331f 100644
@@ -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);

Quoted 2 lines

return ret;
--

Acked-by Tianfei zhang <[email protected]>

Xu, Rosen

Acked-by on v1 1/2

2021-04-01 08:48 UTC #3 lore raw

Re: [dpdk-dev] [PATCH v1 1/2] raw/ifpga/base: use untained variable as argument

In reply to Wei Huang

Hi,

-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: Wednesday, March 17, 2021 4:24 PM
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Huang, Wei <[email protected]>
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 <[email protected]>

---
 drivers/raw/ifpga/base/ifpga_fme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/raw/ifpga/base/ifpga_fme.c +1 -1

index 43c7b9c3dc..a63f90331f 100644
@@ -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 <[email protected]>

Ferruh Yigit

2021-04-07 14:57 UTC #4 lore raw

Re: [dpdk-dev] [PATCH v1 1/2] raw/ifpga/base: use untained variable as argument

In reply to Wei Huang

Quoted 7 lines · Wei Huang

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?

Quoted 19 lines

Signed-off-by Wei Huang <[email protected]>

---
  drivers/raw/ifpga/base/ifpga_fme.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/raw/ifpga/base/ifpga_fme.c +1 -1

index 43c7b9c3dc..a63f90331f 100644
@@ -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;

Wei Huang

author

2021-03-17 08:23 UTC #5 lore raw

PATCH 2/2 raw/ifpga/base: add argument to use untained variable

In reply to Wei Huang

In fme_nios_spi_init(), passing tainted expression "fme->max10_dev" to function "fme_get_board_interface" has risk. Add new arguemnt in function "fme_get_board_interface" to accept untainted variable.

Coverity issue: 367482

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

Signed-off-by Wei Huang <[email protected]>

---
 drivers/raw/ifpga/base/ifpga_fme.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

drivers/raw/ifpga/base/ifpga_fme.c +7 -6

index a63f90331f..4edb9e09e1 100644
@@ -857,7 +857,8 @@ static int board_type_to_info(u32 type,
 	return 0;
 }
 
-static int fme_get_board_interface(struct ifpga_fme_hw *fme)
+static int fme_get_board_interface(struct ifpga_fme_hw *fme,
+	struct intel_max10_device *max10_dev)
 {
 	struct fme_bitstream_id id;
 	struct ifpga_hw *hw;

@@ -911,15 +912,15 @@ static int fme_get_board_interface(struct ifpga_fme_hw *fme)
 			fme->board_info.nums_of_fvl,
 			fme->board_info.ports_per_fvl);
 
-	if (max10_sys_read(fme->max10_dev, FPGA_PAGE_INFO, &val))
+	if (max10_sys_read(max10_dev, FPGA_PAGE_INFO, &val))
 		return -EINVAL;
 	fme->board_info.boot_page = val & 0x7;
 
-	if (max10_sys_read(fme->max10_dev, MAX10_BUILD_VER, &val))
+	if (max10_sys_read(max10_dev, MAX10_BUILD_VER, &val))
 		return -EINVAL;
 	fme->board_info.max10_version = val;
 
-	if (max10_sys_read(fme->max10_dev, NIOS2_FW_VERSION, &val))
+	if (max10_sys_read(max10_dev, NIOS2_FW_VERSION, &val))
 		return -EINVAL;
 	fme->board_info.nios_fw_version = val;

@@ -1169,7 +1170,7 @@ static int fme_nios_spi_init(struct ifpga_feature *feature)
 
 	max10->bus = hw->pci_data->bus;
 
-	fme_get_board_interface(fme);
+	fme_get_board_interface(fme, max10);
 
 	mgr->sensor_list = &max10->opae_sensor_list;

@@ -1186,7 +1187,7 @@ static int fme_nios_spi_init(struct ifpga_feature *feature)
 	return ret;
 
 spi_fail:
-	intel_max10_device_remove(fme->max10_dev);
+	intel_max10_device_remove(max10);
 release_dev:
 	altera_spi_release(spi_master);
 	return -ENODEV;

Signature · 1 line

2.29.2

Zhang, Tianfei

Acked-by on v1 2/2

2021-04-01 07:48 UTC #6 lore raw

Re: [dpdk-dev] [PATCH v1 2/2] raw/ifpga/base: add argument to use untained variable

Quoted 78 lines

-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: 2021年3月17日 16:24
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Huang, Wei <[email protected]>
Subject: [PATCH v1 2/2] raw/ifpga/base: add argument to use untained variable

Coverity issue: 367482

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

Signed-off-by Wei Huang <[email protected]>

---
 drivers/raw/ifpga/base/ifpga_fme.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

drivers/raw/ifpga/base/ifpga_fme.c +7 -6

index a63f90331f..4edb9e09e1 100644
@@ -857,7 +857,8 @@ static int board_type_to_info(u32 type,
 	return 0;
 }
 
-static int fme_get_board_interface(struct ifpga_fme_hw *fme)
+static int fme_get_board_interface(struct ifpga_fme_hw *fme,
+	struct intel_max10_device *max10_dev)
 {
 	struct fme_bitstream_id id;
 	struct ifpga_hw *hw;

@@ -911,15 +912,15 @@ static int fme_get_board_interface(struct
 ifpga_fme_hw *fme)
 			fme->board_info.nums_of_fvl,
 			fme->board_info.ports_per_fvl);
 
-	if (max10_sys_read(fme->max10_dev, FPGA_PAGE_INFO, &val))
+	if (max10_sys_read(max10_dev, FPGA_PAGE_INFO, &val))
 		return -EINVAL;
 	fme->board_info.boot_page = val & 0x7;
 
-	if (max10_sys_read(fme->max10_dev, MAX10_BUILD_VER, &val))
+	if (max10_sys_read(max10_dev, MAX10_BUILD_VER, &val))
 		return -EINVAL;
 	fme->board_info.max10_version = val;
 
-	if (max10_sys_read(fme->max10_dev, NIOS2_FW_VERSION, &val))
+	if (max10_sys_read(max10_dev, NIOS2_FW_VERSION, &val))
 		return -EINVAL;
 	fme->board_info.nios_fw_version = val;

@@ -1169,7 +1170,7 @@ static int fme_nios_spi_init(struct ifpga_feature
 *feature)
 
 	max10->bus = hw->pci_data->bus;
 
-	fme_get_board_interface(fme);
+	fme_get_board_interface(fme, max10);
 
 	mgr->sensor_list = &max10->opae_sensor_list;

@@ -1186,7 +1187,7 @@ static int fme_nios_spi_init(struct ifpga_feature
 *feature)
 	return ret;
 
 spi_fail:
-	intel_max10_device_remove(fme->max10_dev);
+	intel_max10_device_remove(max10);
 release_dev:
 	altera_spi_release(spi_master);

Quoted 2 lines

return -ENODEV;
--

Acked-by Tianfei zhang <[email protected]>

Xu, Rosen

Acked-by on v1 2/2

2021-04-01 08:49 UTC #7 lore raw

Re: [dpdk-dev] [PATCH v1 2/2] raw/ifpga/base: add argument to use untained variable

In reply to Wei Huang

Hi,

-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: Wednesday, March 17, 2021 4:24 PM
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Huang, Wei <[email protected]>
Subject: [PATCH v1 2/2] raw/ifpga/base: add argument to use untained variable

In fme_nios_spi_init(), passing tainted expression "fme->max10_dev"
to function "fme_get_board_interface" has risk. Add new arguemnt in function "fme_get_board_interface" to accept untainted variable.

Coverity issue: 367482

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

Signed-off-by Wei Huang <[email protected]>

---
 drivers/raw/ifpga/base/ifpga_fme.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

drivers/raw/ifpga/base/ifpga_fme.c +7 -6

index a63f90331f..4edb9e09e1 100644
@@ -857,7 +857,8 @@ static int board_type_to_info(u32 type,
 	return 0;
 }
 
-static int fme_get_board_interface(struct ifpga_fme_hw *fme)
+static int fme_get_board_interface(struct ifpga_fme_hw *fme,
+	struct intel_max10_device *max10_dev)
 {
 	struct fme_bitstream_id id;
 	struct ifpga_hw *hw;

@@ -911,15 +912,15 @@ static int fme_get_board_interface(struct ifpga_fme_hw *fme)
 			fme->board_info.nums_of_fvl,
 			fme->board_info.ports_per_fvl);
 
-	if (max10_sys_read(fme->max10_dev, FPGA_PAGE_INFO, &val))
+	if (max10_sys_read(max10_dev, FPGA_PAGE_INFO, &val))
 		return -EINVAL;
 	fme->board_info.boot_page = val & 0x7;
 
-	if (max10_sys_read(fme->max10_dev, MAX10_BUILD_VER, &val))
+	if (max10_sys_read(max10_dev, MAX10_BUILD_VER, &val))
 		return -EINVAL;
 	fme->board_info.max10_version = val;
 
-	if (max10_sys_read(fme->max10_dev, NIOS2_FW_VERSION, &val))
+	if (max10_sys_read(max10_dev, NIOS2_FW_VERSION, &val))
 		return -EINVAL;
 	fme->board_info.nios_fw_version = val;

@@ -1169,7 +1170,7 @@ static int fme_nios_spi_init(struct ifpga_feature *feature)
 
 	max10->bus = hw->pci_data->bus;
 
-	fme_get_board_interface(fme);
+	fme_get_board_interface(fme, max10);
 
 	mgr->sensor_list = &max10->opae_sensor_list;

@@ -1186,7 +1187,7 @@ static int fme_nios_spi_init(struct ifpga_feature *feature)
 	return ret;
 
 spi_fail:
-	intel_max10_device_remove(fme->max10_dev);
+	intel_max10_device_remove(max10);
 release_dev:
 	altera_spi_release(spi_master);
 	return -ENODEV;

--
2.29.2

Acked-by Rosen Xu <[email protected]>

Zhang, Qi Z

Applied

2021-04-01 12:17 UTC #8 lore raw

In reply to Wei Huang

Quoted 18 lines

-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: Wednesday, March 17, 2021 4:24 PM
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Huang, Wei <[email protected]>
Subject: [PATCH v1 0/2] Fix coverity issues reported in DPDK-26381
Below coverity issues are fixed in this patch set. 367480, 367482
Wei Huang (2):
raw/ifpga/base: use untained variable as argument raw/ifpga/base: add argument to use untained variable

Quoted 5 lines

 drivers/raw/ifpga/base/ifpga_fme.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

--
2.29.2

Applied to dpdk-next-net-intel.

Thanks
Qi

Ferruh Yigit

2021-04-20 08:10 UTC #9 lore raw

Quoted 18 lines · Qi Z

Quoted 18 lines
-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: Wednesday, March 17, 2021 4:24 PM
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Huang, Wei <[email protected]>
Subject: [PATCH v1 0/2] Fix coverity issues reported in DPDK-26381
Below coverity issues are fixed in this patch set. 367480, 367482
Wei Huang (2):
raw/ifpga/base: use untained variable as argument raw/ifpga/base: add argument to use untained variable

Quoted 8 lines

Quoted 5 lines
  drivers/raw/ifpga/base/ifpga_fme.c | 15 ++++++++-------
  1 file changed, 8 insertions(+), 7 deletions(-)
--
2.29.2
Applied to dpdk-next-net-intel.

Hi Qi,

What is the status of this set?
It is not merged to your tree, and its patchwork status is 'Rejected'.

Zhang, Qi Z

2021-04-20 09:12 UTC #10 lore raw

Quoted 36 lines

-----Original Message-----
From: Yigit, Ferruh <[email protected]>
Sent: Tuesday, April 20, 2021 4:10 PM
To: Zhang, Qi Z <[email protected]>; Huang, Wei <[email protected]>; [email protected]; Xu, Rosen <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>
Subject: Re: [dpdk-dev] [PATCH v1 0/2] Fix coverity issues reported in DPDK-26381
Quoted 24 lines · Qi Z
Quoted 23 lines
-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: Wednesday, March 17, 2021 4:24 PM
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Huang, Wei <[email protected]>
Subject: [PATCH v1 0/2] Fix coverity issues reported in DPDK-26381
Below coverity issues are fixed in this patch set. 367480, 367482
Wei Huang (2):
raw/ifpga/base: use untained variable as argument raw/ifpga/base: add argument to use untained variable
  drivers/raw/ifpga/base/ifpga_fme.c | 15 ++++++++-------
  1 file changed, 8 insertions(+), 7 deletions(-)
--
2.29.2

Quoted 7 lines

Quoted 2 lines
Applied to dpdk-next-net-intel.
Hi Qi,
What is the status of this set?
It is not merged to your tree, and its patchwork status is 'Rejected'.

I think the status is wrong updated, and the patches are just dropped after re-sync with next-net, thanks for capture this.

Zhang, Qi Z

2021-04-20 09:29 UTC #11 lore raw

Quoted 50 lines

-----Original Message-----
From: Zhang, Qi Z
Sent: Tuesday, April 20, 2021 5:13 PM
To: Yigit, Ferruh <[email protected]>; Huang, Wei <[email protected]>; [email protected]; Xu, Rosen <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>
Subject: RE: [dpdk-dev] [PATCH v1 0/2] Fix coverity issues reported in DPDK-26381
Quoted 37 lines
-----Original Message-----
From: Yigit, Ferruh <[email protected]>
Sent: Tuesday, April 20, 2021 4:10 PM
To: Zhang, Qi Z <[email protected]>; Huang, Wei <[email protected]>; [email protected]; Xu, Rosen <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>
Subject: Re: [dpdk-dev] [PATCH v1 0/2] Fix coverity issues reported in DPDK-26381
Quoted 26 lines · Qi Z
Quoted 18 lines
-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: Wednesday, March 17, 2021 4:24 PM
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Huang, Wei <[email protected]>
Subject: [PATCH v1 0/2] Fix coverity issues reported in DPDK-26381
Below coverity issues are fixed in this patch set. 367480, 367482
Wei Huang (2):
raw/ifpga/base: use untained variable as argument raw/ifpga/base: add argument to use untained variable
Quoted 5 lines
  drivers/raw/ifpga/base/ifpga_fme.c | 15 ++++++++-------
  1 file changed, 8 insertions(+), 7 deletions(-)
--
2.29.2
Applied to dpdk-next-net-intel.

Quoted 7 lines

Quoted 4 lines
Hi Qi,
What is the status of this set?
It is not merged to your tree, and its patchwork status is 'Rejected'.
I think the status is wrong updated, and the patches are just dropped after re-sync with next-net, thanks for capture this.

OK, the patch set should be rejected, as they already be covered by below patch.
http://patchwork.dpdk.org/project/dpdk/patch/[email protected]/
So there is no miss operation in path work, just can't remember what happened :)

We are OK now.