more Coverity related bug fixes

RFC In review Stephen Hemminger 6 participants 20 messages Nov 6, 2018 – Nov 25, 2018

lore

Share & tools

lore

Stephen Hemminger

author

2018-11-06 21:49 UTC #0 lore raw

cover more Coverity related bug fixes

There are some real bugs in here, not just noise. Labeling this version as RFC since compile tested only.

Stephen Hemminger (5):
bus/pci: fix allocation of pci device path
bus/pci: fix TOCTOU issue
bpf: fix validation of eal_divmod
eal/memory: avoid double munmap in error path
pipeline: remove dead code

 drivers/bus/pci/linux/pci_uio.c          | 40 +++++++++++-------------
 lib/librte_bpf/bpf_validate.c            |  2 +-
 lib/librte_eal/linuxapp/eal/eal_memory.c |  1 +
 lib/librte_pipeline/rte_table_action.c   |  2 --
 4 files changed, 21 insertions(+), 24 deletions(-)

Signature · 1 line

2.17.1

Stephen Hemminger

author

2018-11-06 21:49 UTC #1 lore raw

RFC 1/5 bus/pci: fix allocation of pci device path

The pci_resource_by_index called strlen() on uninitialized memory which would lead to the wrong size of memory allocated for the path portion of the resource map. This would either cause excessively large allocation, or worse memory corruption.

Coverity Issue: 300868

Fixes ea9d56226e72 ("pci: introduce function to map uio resource by index")

Signed-off-by Stephen Hemminger <[email protected]>

---
 drivers/bus/pci/linux/pci_uio.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

drivers/bus/pci/linux/pci_uio.c +14 ~~-15~~

index a7c14421aa79..112ac51dddcc 100644
@@ -295,14 +295,6 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 	loc = &dev->addr;
 	maps = uio_res->maps;
 
-	/* allocate memory to keep path */
-	maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
-	if (maps[map_idx].path == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot allocate memory for path: %s\n",
-				strerror(errno));
-		return -1;
-	}
-
 	/*
 	 * open resource file, to mmap it
 	 */

@@ -335,10 +327,19 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 		if (fd < 0) {
 			RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
 				devname, strerror(errno));
-			goto error;
+			return -1;
 		}
 	}
 
+	/* allocate memory to keep path */
+	maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+	if (maps[map_idx].path == NULL) {
+		RTE_LOG(ERR, EAL, "Cannot allocate memory for path: %s\n",
+				strerror(errno));
+		close(fd);
+		return -1;
+	}
+
 	/* try mapping somewhere close to the end of hugepages */
 	if (pci_map_addr == NULL)
 		pci_map_addr = pci_find_max_end_va();

@@ -346,8 +347,10 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 	mapaddr = pci_map_resource(pci_map_addr, fd, 0,
 			(size_t)dev->mem_resource[res_idx].len, 0);
 	close(fd);
-	if (mapaddr == MAP_FAILED)
-		goto error;
+	if (mapaddr == MAP_FAILED) {
+		rte_free(maps[map_idx].path);
+		return -1;
+	}
 
 	pci_map_addr = RTE_PTR_ADD(mapaddr,
 			(size_t)dev->mem_resource[res_idx].len);

@@ -360,10 +363,6 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 	dev->mem_resource[res_idx].addr = mapaddr;
 
 	return 0;
-
-error:
-	rte_free(maps[map_idx].path);
-	return -1;
 }
 
 #if defined(RTE_ARCH_X86)

Signature · 1 line

2.17.1

Thomas Monjalon

2018-11-18 15:03 UTC #2 lore raw

Re: [RFC 1/5] bus/pci: fix allocation of pci device path

06/11/2018 22:48, Stephen Hemminger:

Quoted 8 lines

The pci_resource_by_index called strlen() on uninitialized memory which would lead to the wrong size of memory allocated for the path portion of the resource map. This would either cause excessively large allocation, or worse memory corruption.
Coverity Issue: 300868
Fixes ea9d56226e72 ("pci: introduce function to map uio resource by index")
Signed-off-by Stephen Hemminger <[email protected]>

This patch is RFC but Ferruh (UIO maintainer) was not Cc'ed. I feel this bug is critical. Please advise.

Ferruh Yigit

2018-11-22 23:52 UTC #3 lore raw

Re: [RFC 1/5] bus/pci: fix allocation of pci device path

In reply to Stephen Hemminger

Quoted 4 lines · Stephen Hemminger

The pci_resource_by_index called strlen() on uninitialized memory which would lead to the wrong size of memory allocated for the path portion of the resource map. This would either cause excessively large allocation, or worse memory corruption.

Yes this may corrupt memory, I wonder how nobody hit this. I am for including the fix for the release.

Quoted 13 lines

Coverity Issue: 300868
Fixes ea9d56226e72 ("pci: introduce function to map uio resource by index")
Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/bus/pci/linux/pci_uio.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)
drivers/bus/pci/linux/pci_uio.c +0 -0
index a7c14421aa79..112ac51dddcc 100644

drivers/bus/pci/linux/pci_uio.c pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx, · line 304

Quoted 10 lines

drivers/bus/pci/linux/pci_uio.c +0 -7

@@ -295,14 +295,6 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 	loc = &dev->addr;
 	maps = uio_res->maps;
 
-	/* allocate memory to keep path */
-	maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
-	if (maps[map_idx].path == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot allocate memory for path: %s\n",
-				strerror(errno));
-		return -1;
-	}

What about simply:

- maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+ maps[map_idx].path = rte_malloc(NULL, sizeof(devname) + 1, 0);

Ferruh Yigit

2018-11-23 00:29 UTC #4 lore raw

PATCH 1/1 bus/pci: fix allocation of PCI device path

In reply to Stephen Hemminger

Coverity Issue: 300868

Fixes ea9d56226e72 ("pci: introduce function to map uio resource by index")

Cc [email protected]

Signed-off-by Stephen Hemminger <[email protected]>

Signed-off-by Ferruh Yigit <[email protected]>

---
 drivers/bus/pci/linux/pci_uio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/bus/pci/linux/pci_uio.c +1 -1

index a7c14421a..09ecbb7aa 100644
@@ -296,7 +296,7 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 	maps = uio_res->maps;
 
 	/* allocate memory to keep path */
-	maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+	maps[map_idx].path = rte_malloc(NULL, sizeof(devname), 0);
 	if (maps[map_idx].path == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot allocate memory for path: %s\n",
 				strerror(errno));

Signature · 1 line

2.17.2

Thomas Monjalon

2018-11-23 10:45 UTC #5 lore raw

Re: [PATCH] bus/pci: fix allocation of PCI device path

Please, anyone for a review and a test?

23/11/2018 01:29, Ferruh Yigit:

Quoted 29 lines

The pci_resource_by_index called strlen() on uninitialized memory which would lead to the wrong size of memory allocated for the path portion of the resource map. This would either cause excessively large allocation, or worse memory corruption.
Coverity Issue: 300868
Fixes ea9d56226e72 ("pci: introduce function to map uio resource by index")
Cc [email protected]
Signed-off-by Stephen Hemminger <[email protected]>
Signed-off-by Ferruh Yigit <[email protected]>
---
 drivers/bus/pci/linux/pci_uio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/pci/linux/pci_uio.c +1 -1
index a7c14421a..09ecbb7aa 100644
@@ -296,7 +296,7 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 	maps = uio_res->maps;
 
 	/* allocate memory to keep path */
-	maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+	maps[map_idx].path = rte_malloc(NULL, sizeof(devname), 0);
 	if (maps[map_idx].path == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot allocate memory for path: %s\n",
 				strerror(errno));

Andrew Rybchenko

Reviewed-by

2018-11-23 10:55 UTC #6 lore raw

Re: [PATCH] bus/pci: fix allocation of PCI device path

Quoted 14 lines · Thomas Monjalon

Please, anyone for a review and a test?
23/11/2018 01:29, Ferruh Yigit:
Quoted 11 lines
The pci_resource_by_index called strlen() on uninitialized memory which would lead to the wrong size of memory allocated for the path portion of the resource map. This would either cause excessively large allocation, or worse memory corruption.
Coverity Issue: 300868
Fixes ea9d56226e72 ("pci: introduce function to map uio resource by index")
Cc [email protected]
Signed-off-by Stephen Hemminger <[email protected]>
Signed-off-by Ferruh Yigit <[email protected]>

Reviewed-by Andrew Rybchenko <[email protected]>

Maxime Coquelin

Reviewed-by on v1 1/1

2018-11-23 11:01 UTC #7 lore raw

Re: [PATCH] bus/pci: fix allocation of PCI device path

In reply to Ferruh Yigit

Quoted 21 lines · Ferruh Yigit

The pci_resource_by_index called strlen() on uninitialized memory which would lead to the wrong size of memory allocated for the path portion of the resource map. This would either cause excessively large allocation, or worse memory corruption.
Coverity Issue: 300868
Fixes ea9d56226e72 ("pci: introduce function to map uio resource by index")
Cc [email protected]
Signed-off-by Stephen Hemminger <[email protected]>
Signed-off-by Ferruh Yigit <[email protected]>
---
  drivers/bus/pci/linux/pci_uio.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/pci/linux/pci_uio.c +0 -0
index a7c14421a..09ecbb7aa 100644

drivers/bus/pci/linux/pci_uio.c pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx, · line 302

Quoted 8 lines

drivers/bus/pci/linux/pci_uio.c +1 -1

@@ -296,7 +296,7 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
  	maps = uio_res->maps;
 
  	/* allocate memory to keep path */
-	maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+	maps[map_idx].path = rte_malloc(NULL, sizeof(devname), 0);
  	if (maps[map_idx].path == NULL) {
  		RTE_LOG(ERR, EAL, "Cannot allocate memory for path: %s\n",
  				strerror(errno));

Reviewed-by Maxime Coquelin <[email protected]>

Thanks,
Maxime

Thomas Monjalon

Applied

2018-11-25 10:53 UTC #8 lore raw

Re: [PATCH] bus/pci: fix allocation of PCI device path

23/11/2018 12:01, Maxime Coquelin:

Quoted 6 lines · Ferruh Yigit

Quoted 5 lines
The pci_resource_by_index called strlen() on uninitialized memory which would lead to the wrong size of memory allocated for the path portion of the resource map. This would either cause excessively large allocation, or worse memory corruption.

Quoted 8 lines

Quoted 6 lines
Coverity Issue: 300868
Fixes ea9d56226e72 ("pci: introduce function to map uio resource by index")
Cc [email protected]
Signed-off-by Stephen Hemminger <[email protected]>
Signed-off-by Ferruh Yigit <[email protected]>
Reviewed-by Maxime Coquelin <[email protected]>

Applied, thanks

Stephen Hemminger

author

2018-11-06 21:49 UTC #9 lore raw

RFC 2/5 bus/pci: fix TOCTOU issue

In reply to Stephen Hemminger

Using access followed by open causes a static analysis warning about Time of check versus Time of use. Also, access() and open() have different UID permission checks.

This is not a serious problem; but easy to fix by using errno instead.

Coverity issue: 300870

Fixes 4a928ef9f611 ("bus/pci: enable write combining during mapping")

Signed-off-by Stephen Hemminger <[email protected]>

---
 drivers/bus/pci/linux/pci_uio.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

drivers/bus/pci/linux/pci_uio.c +5 -6

index 112ac51dddcc..8521fe63b0ae 100644
@@ -306,12 +306,11 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 			loc->domain, loc->bus, loc->devid,
 			loc->function, res_idx);
 
-		if (access(devname, R_OK|W_OK) != -1) {
-			fd = open(devname, O_RDWR);
-			if (fd < 0)
-				RTE_LOG(INFO, EAL, "%s cannot be mapped. "
-					"Fall-back to non prefetchable mode.\n",
-					devname);
+		fd = open(devname, O_RDWR);
+		if (fd < 0 && errno != ENOENT) {
+			RTE_LOG(INFO, EAL, "%s cannot be mapped. "
+				"Fall-back to non prefetchable mode.\n",
+				devname);
 		}
 	}

Signature · 1 line

2.17.1

Thomas Monjalon

2018-11-18 15:04 UTC #10 lore raw

Re: [RFC 2/5] bus/pci: fix TOCTOU issue

+Cc Ferruh

06/11/2018 22:48, Stephen Hemminger:

Quoted 34 lines

Using access followed by open causes a static analysis warning about Time of check versus Time of use. Also, access() and open() have different UID permission checks.
This is not a serious problem; but easy to fix by using errno instead.
Coverity issue: 300870
Fixes 4a928ef9f611 ("bus/pci: enable write combining during mapping")
Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/bus/pci/linux/pci_uio.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
drivers/bus/pci/linux/pci_uio.c +5 -6
index 112ac51dddcc..8521fe63b0ae 100644
@@ -306,12 +306,11 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 			loc->domain, loc->bus, loc->devid,
 			loc->function, res_idx);
 
-		if (access(devname, R_OK|W_OK) != -1) {
-			fd = open(devname, O_RDWR);
-			if (fd < 0)
-				RTE_LOG(INFO, EAL, "%s cannot be mapped. "
-					"Fall-back to non prefetchable mode.\n",
-					devname);
+		fd = open(devname, O_RDWR);
+		if (fd < 0 && errno != ENOENT) {
+			RTE_LOG(INFO, EAL, "%s cannot be mapped. "
+				"Fall-back to non prefetchable mode.\n",
+				devname);
 		}
 	}

Stephen Hemminger

author

2018-11-06 21:49 UTC #11 lore raw

RFC 3/5 bpf: fix validation of eal_divmod

In reply to Stephen Hemminger

Coverity spotted self assignment in BPF eval_divmod. This looks like a bug where the incoming source register should have been used instead.

Coverity issue: 302850

Fixes 8021917293d0 ("bpf: add extra validation for input BPF program")

Signed-off-by Stephen Hemminger <[email protected]>

---
 lib/librte_bpf/bpf_validate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/librte_bpf/bpf_validate.c +1 -1

index 83983efc4e5c..b768f72c4c02 100644
@@ -512,7 +512,7 @@ eval_divmod(uint32_t op, struct bpf_reg_val *rd, struct bpf_reg_val *rs,
 		if (op == BPF_MOD)
 			rd->u.max = RTE_MIN(rd->u.max, rs->u.max - 1);
 		else
-			rd->u.max = rd->u.max;
+			rd->u.max = rs->u.max;
 		rd->u.min = 0;
 	}

Signature · 1 line

2.17.1

Ananyev, Konstantin

2018-11-07 12:54 UTC #12 lore raw

Re: [RFC 3/5] bpf: fix validation of eal_divmod

Quoted 10 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Stephen Hemminger
Sent: Tuesday, November 6, 2018 9:49 PM
To: [email protected]
Cc: Stephen Hemminger <[email protected]>
Subject: [dpdk-dev] [RFC 3/5] bpf: fix validation of eal_divmod
Coverity spotted self assignment in BPF eval_divmod.

Yep, there is one.
As I remember I have to add it because one of old versions
of compiler (clang???) complained about 'variable being used uninitialized'.

Quoted 2 lines

This looks like a bug where the incoming source register should have been used instead.

Nope, that's a wrong guess.
We shouldn't do it here.
Konstantin

Quoted 23 lines

Coverity issue: 302850

Fixes 8021917293d0 ("bpf: add extra validation for input BPF program")

Signed-off-by Stephen Hemminger <[email protected]>

---
 lib/librte_bpf/bpf_validate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/librte_bpf/bpf_validate.c +1 -2

index 83983efc4e5c..b768f72c4c02 100644
@@ -512,7 +512,7 @@ eval_divmod(uint32_t op, struct bpf_reg_val *rd, struct bpf_reg_val *rs,
 		if (op == BPF_MOD)
 			rd->u.max = RTE_MIN(rd->u.max, rs->u.max - 1);
 		else
-			rd->u.max = rd->u.max;
+			rd->u.max = rs->u.max;
 		rd->u.min = 0;
 	}
 
--

2.17.1

Stephen Hemminger

author

2018-11-07 19:51 UTC #13 lore raw

Re: [RFC 3/5] bpf: fix validation of eal_divmod

Quoted 41 lines · Konstantin

Quoted 8 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Stephen Hemminger
Sent: Tuesday, November 6, 2018 9:49 PM
To: [email protected]
Cc: Stephen Hemminger <[email protected]>
Subject: [dpdk-dev] [RFC 3/5] bpf: fix validation of eal_divmod
Coverity spotted self assignment in BPF eval_divmod.
Yep, there is one.
As I remember I have to add it because one of old versions
of compiler (clang???) complained about 'variable being used uninitialized'.
Quoted 2 lines
This looks like a bug where the incoming source register should have been used instead.
Nope, that's a wrong guess.
We shouldn't do it here.
Konstantin
Quoted 21 lines
Coverity issue: 302850
Fixes 8021917293d0 ("bpf: add extra validation for input BPF program")
Signed-off-by Stephen Hemminger <[email protected]>
---
 lib/librte_bpf/bpf_validate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_bpf/bpf_validate.c +1 -1
index 83983efc4e5c..b768f72c4c02 100644
@@ -512,7 +512,7 @@ eval_divmod(uint32_t op, struct bpf_reg_val *rd, struct bpf_reg_val *rs,
 		if (op == BPF_MOD)
 			rd->u.max = RTE_MIN(rd->u.max, rs->u.max - 1);
 		else
-			rd->u.max = rd->u.max;
+			rd->u.max = rs->u.max;
 		rd->u.min = 0;
 	}
 

Quoted 3 lines

Quoted 2 lines
--
2.17.1

Well it was being used unintialized, your trick of self assignment fooled clang but did not fool Coverity. What does the other BPF validator do?

Ananyev, Konstantin

2018-11-07 20:07 UTC #14 lore raw

Re: [RFC 3/5] bpf: fix validation of eal_divmod

Quoted 53 lines

-----Original Message-----
From: Stephen Hemminger [mailto:[email protected]]
Sent: Wednesday, November 7, 2018 7:52 PM
To: Ananyev, Konstantin <[email protected]>
Cc: [email protected]
Subject: Re: [dpdk-dev] [RFC 3/5] bpf: fix validation of eal_divmod
Quoted 41 lines · Konstantin
Quoted 8 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Stephen Hemminger
Sent: Tuesday, November 6, 2018 9:49 PM
To: [email protected]
Cc: Stephen Hemminger <[email protected]>
Subject: [dpdk-dev] [RFC 3/5] bpf: fix validation of eal_divmod
Coverity spotted self assignment in BPF eval_divmod.
Yep, there is one.
As I remember I have to add it because one of old versions
of compiler (clang???) complained about 'variable being used uninitialized'.
Quoted 2 lines
This looks like a bug where the incoming source register should have been used instead.
Nope, that's a wrong guess.
We shouldn't do it here.
Konstantin
Quoted 21 lines
Coverity issue: 302850
Fixes 8021917293d0 ("bpf: add extra validation for input BPF program")
Signed-off-by Stephen Hemminger <[email protected]>
---
 lib/librte_bpf/bpf_validate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_bpf/bpf_validate.c +1 -1
index 83983efc4e5c..b768f72c4c02 100644
@@ -512,7 +512,7 @@ eval_divmod(uint32_t op, struct bpf_reg_val *rd, struct bpf_reg_val *rs,
 		if (op == BPF_MOD)
 			rd->u.max = RTE_MIN(rd->u.max, rs->u.max - 1);
 		else
-			rd->u.max = rd->u.max;
+			rd->u.max = rs->u.max;
 		rd->u.min = 0;
 	}
 

Quoted 5 lines

Quoted 3 lines
Quoted 2 lines
--
2.17.1
Well it was being used unintialized,

I don't think so, but if you can point to me where exactly it is used uninitialized, we can discuss it further.

Quoted 1 line

your trick of self assignment fooled clang

It was one particular and pretty old version of clang (if my memory serves me right).
With latest versions (let say 6.0) it doesn't complain, if I remove that self-assignment.
gcc also doesn't see any problem here.
That makes me think it was a false-positive with old version of the compiler.
Konstantin

Quoted 1 line

but did not fool Coverity. What does the other BPF validator do?

Ananyev, Konstantin

2018-11-07 23:04 UTC #15 lore raw

Re: [RFC 3/5] bpf: fix validation of eal_divmod

In reply to Stephen Hemminger

Quoted 46 lines

Quoted 35 lines
Quoted 35 lines
Quoted 2 lines
Coverity spotted self assignment in BPF eval_divmod.
Yep, there is one.
As I remember I have to add it because one of old versions
of compiler (clang???) complained about 'variable being used uninitialized'.
Quoted 2 lines
This looks like a bug where the incoming source register should have been used instead.
Nope, that's a wrong guess.
We shouldn't do it here.
Konstantin
Quoted 21 lines
Coverity issue: 302850
Fixes 8021917293d0 ("bpf: add extra validation for input BPF program")
Signed-off-by Stephen Hemminger <[email protected]>
---
 lib/librte_bpf/bpf_validate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_bpf/bpf_validate.c +1 -1
index 83983efc4e5c..b768f72c4c02 100644
@@ -512,7 +512,7 @@ eval_divmod(uint32_t op, struct bpf_reg_val *rd, struct bpf_reg_val *rs,
 		if (op == BPF_MOD)
 			rd->u.max = RTE_MIN(rd->u.max, rs->u.max - 1);
 		else
-			rd->u.max = rd->u.max;
+			rd->u.max = rs->u.max;
 		rd->u.min = 0;
 	}
 
Quoted 5 lines
Quoted 3 lines
Quoted 2 lines
--
2.17.1
Well it was being used unintialized,
I don't think so, but if you can point to me where exactly it is used uninitialized, we can discuss it further.
Quoted 1 line
your trick of self assignment fooled clang

Quoted 8 lines

It was one particular and pretty old version of clang (if my memory serves me right).
With latest versions (let say 6.0) it doesn't complain, if I remove that self-assignment.
gcc also doesn't see any problem here.
That makes me think it was a false-positive with old version of the compiler.
Konstantin

As a another thought - it wouldn't take much effort to send a patch with NOP self-assignment removed.
If it will pass our build-harness test, then it is probably ok to integrate.
Konstantin

Quoted 2 lines

Quoted 2 lines
but did not fool Coverity. What does the other BPF validator do?

Stephen Hemminger

author

2018-11-06 21:49 UTC #16 lore raw

RFC 4/5 eal/memory: avoid double munmap in error path

In reply to Stephen Hemminger

Don't call munmap of hugepage memory twice in the error path.

Coverity issue: 325730

Fixes 66cc45e293ed ("mem: replace memseg with memseg lists")

Signed-off-by Stephen Hemminger <[email protected]>

---
 lib/librte_eal/linuxapp/eal/eal_memory.c | 1 +
 1 file changed, 1 insertion(+)

lib/librte_eal/linuxapp/eal/eal_memory.c +1 -0

index c1b5e079117a..48b23ce19ad3 100644
@@ -1617,6 +1617,7 @@ eal_legacy_hugepage_init(void)
 	tmp_hp = NULL;
 
 	munmap(hugepage, nr_hugefiles * sizeof(struct hugepage_file));
+	hugepage = NULL;
 
 	/* we're not going to allocate more pages, so release VA space for
 	 * unused memseg lists

Signature · 1 line

2.17.1

Thomas Monjalon

2018-11-06 23:10 UTC #17 lore raw

Re: [RFC 4/5] eal/memory: avoid double munmap in error path

06/11/2018 22:49, Stephen Hemminger:

Quoted 5 lines

Don't call munmap of hugepage memory twice in the error path.
Coverity issue: 325730
Fixes 66cc45e293ed ("mem: replace memseg with memseg lists")
Signed-off-by Stephen Hemminger <[email protected]>

Thank you
Anatoly submitted the same fix:

	https://patches.dpdk.org/patch/47898/

Stephen Hemminger

author

2018-11-06 21:49 UTC #18 lore raw

RFC 5/5 pipeline: remove dead code

In reply to Stephen Hemminger

Coverity detected dead code since pointer into a global array can never be NULL.

Coverity issue: 323523

Fixes 96303217a606 ("pipeline: add symmetric crypto table action")

Signed-off-by Stephen Hemminger <[email protected]>

---
 lib/librte_pipeline/rte_table_action.c | 2 --
 1 file changed, 2 deletions(-)

lib/librte_pipeline/rte_table_action.c +0 -2

index 537e6593e4a0..69ab41a7ab2d 100644
@@ -1696,8 +1696,6 @@ get_block_size(const struct rte_crypto_sym_xform *xform, uint8_t cdev_id)
 
 	for (i = 0;; i++) {
 		cap = &dev_info.capabilities[i];
-		if (!cap)
-			break;
 
 		if (cap->sym.xform_type != xform->type)
 			continue;

Signature · 1 line

2.17.1

Thomas Monjalon

2018-11-18 15:07 UTC #19 lore raw

Re: [RFC 5/5] pipeline: remove dead code

+Cc Cristian

06/11/2018 22:49, Stephen Hemminger:

Quoted 6 lines

Coverity detected dead code since pointer into a global array can never be NULL.
Coverity issue: 323523
Fixes 96303217a606 ("pipeline: add symmetric crypto table action")
Signed-off-by Stephen Hemminger <[email protected]>