Fixes for coverity issues in fbarray

PATCH Applied Anatoly Burakov 3 participants 7 messages Apr 17, 2018 – Apr 23, 2018

lore

Share & tools

lore

Anatoly Burakov

author

2018-04-17 15:44 UTC #0 lore raw

cover Fixes for coverity issues in fbarray

This patchset fixes a number of Coverity issues introduced in recent DPDK memory hotplug patchset.

Coverity issues fixed:
- 272564 - error condition not checked
- 272579 - dereference before null check
- 272586 - error condition not checked

There are two additional issues reported by coverity:
- 272598 - error condition not checked
- 272599 - error condition not checked

However, they are fixed by a separate patch [1].

[1] http://dpdk.org/dev/patchwork/patch/38091/

Anatoly Burakov (4):
fbarray: use strlcpy instead of snprintf
fbarray: add check for failed file descriptor open
fbarray: fix potential null-dereference
fbarray: handle negative return

 lib/librte_eal/common/eal_common_fbarray.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-04-17 15:44 UTC #1 lore raw

PATCH 1/4 fbarray: use strlcpy instead of snprintf

Signed-off-by Anatoly Burakov <[email protected]>

---
 lib/librte_eal/common/eal_common_fbarray.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/librte_eal/common/eal_common_fbarray.c +1 -1

index f65875d..8aa9013 100644
@@ -462,7 +462,7 @@ rte_fbarray_init(struct rte_fbarray *arr, const char *name, int len, int elt_sz)
 	memset(data, 0, mmap_len);
 
 	/* populate data structure */
-	snprintf(arr->name, sizeof(arr->name), "%s", name);
+	strlcpy(arr->name, name, sizeof(arr->name));
 	arr->data = data;
 	arr->len = len;
 	arr->elt_sz = elt_sz;

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-04-17 15:44 UTC #2 lore raw

PATCH 2/4 fbarray: add check for failed file descriptor open

In reply to Anatoly Burakov

Coverity issue: 272564

Fixes c44d09811b40 ("eal: add shared indexed file-backed array")

Cc [email protected]

Signed-off-by Anatoly Burakov <[email protected]>

---
 lib/librte_eal/common/eal_common_fbarray.c | 5 +++++
 1 file changed, 5 insertions(+)

lib/librte_eal/common/eal_common_fbarray.c +5 -0

index 8aa9013..95a7c8e 100644
@@ -583,6 +583,11 @@ rte_fbarray_destroy(struct rte_fbarray *arr)
 	eal_get_fbarray_path(path, sizeof(path), arr->name);
 
 	fd = open(path, O_RDONLY);
+	if (fd < 0) {
+		RTE_LOG(ERR, EAL, "Could not open fbarray file: %s\n",
+			strerror(errno));
+		return -1;
+	}
 	if (flock(fd, LOCK_EX | LOCK_NB)) {
 		RTE_LOG(DEBUG, EAL, "Cannot destroy fbarray - another process is using it\n");
 		rte_errno = EBUSY;

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-04-17 15:44 UTC #3 lore raw

PATCH 3/4 fbarray: fix potential null-dereference

In reply to Anatoly Burakov

We get pointer to mask before we check if fbarray is NULL. Fix by moving getting mask pointer to until after NULL check.

Coverity issue: 272579

Fixes c44d09811b40 ("eal: add shared indexed file-backed array")

Cc [email protected]

Signed-off-by Anatoly Burakov <[email protected]>

---
 lib/librte_eal/common/eal_common_fbarray.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

lib/librte_eal/common/eal_common_fbarray.c +2 -1

index 95a7c8e..c4ed97e 100644
@@ -349,7 +349,7 @@ find_contig(const struct rte_fbarray *arr, int start, bool used)
 static int
 set_used(struct rte_fbarray *arr, int idx, bool used)
 {
-	struct used_mask *msk = get_used_mask(arr->data, arr->elt_sz, arr->len);
+	struct used_mask *msk;
 	uint64_t msk_bit = 1ULL << MASK_LEN_TO_MOD(idx);
 	int msk_idx = MASK_LEN_TO_IDX(idx);
 	bool already_used;

@@ -359,6 +359,7 @@ set_used(struct rte_fbarray *arr, int idx, bool used)
 		rte_errno = EINVAL;
 		return -1;
 	}
+	msk = get_used_mask(arr->data, arr->elt_sz, arr->len);
 	ret = 0;
 
 	/* prevent array from changing under us */

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-04-17 15:44 UTC #4 lore raw

PATCH 4/4 fbarray: handle negative return

In reply to Anatoly Burakov

sysconf() may return a negative value, check for it.

Coverity issue: 272586

Fixes c44d09811b40 ("eal: add shared indexed file-backed array")

Cc [email protected]

Signed-off-by Anatoly Burakov <[email protected]>

---
 lib/librte_eal/common/eal_common_fbarray.c | 3 +++
 1 file changed, 3 insertions(+)

lib/librte_eal/common/eal_common_fbarray.c +3 -0

index c4ed97e..6486679 100644
@@ -561,6 +561,9 @@ rte_fbarray_detach(struct rte_fbarray *arr)
 
 	size_t page_sz = sysconf(_SC_PAGESIZE);
 
+	if (page_sz == (size_t)-1)
+		return -1;
+
 	/* this may already be unmapped (e.g. repeated call from previously
 	 * failed destroy(), but this is on user, we can't (easily) know if this
 	 * is still mapped.

Signature · 1 line

2.7.4

Adrien Mazarguil

Acked-by on the v1 series

2018-04-18 08:51 UTC #5 lore raw

In reply to Anatoly Burakov

Quoted 14 lines · Anatoly Burakov

This patchset fixes a number of Coverity issues introduced in recent DPDK memory hotplug patchset.
Coverity issues fixed:
- 272564 - error condition not checked
- 272579 - dereference before null check
- 272586 - error condition not checked
There are two additional issues reported by coverity:
- 272598 - error condition not checked
- 272599 - error condition not checked
However, they are fixed by a separate patch [1].

Quoted 7 lines

[1] http://dpdk.org/dev/patchwork/patch/38091/
Anatoly Burakov (4):
fbarray: use strlcpy instead of snprintf
fbarray: add check for failed file descriptor open
fbarray: fix potential null-dereference
fbarray: handle negative return

Given that I recently addressed a similar issue in fbarray as well:

Acked-by Adrien Mazarguil <[email protected]>

Signature · 2 lines

Adrien Mazarguil
6WIND

Thomas Monjalon

Applied

2018-04-23 20:16 UTC #6 lore raw

18/04/2018 10:51, Adrien Mazarguil:

Quoted 23 lines · Anatoly Burakov

Quoted 14 lines
This patchset fixes a number of Coverity issues introduced in recent DPDK memory hotplug patchset.
Coverity issues fixed:
- 272564 - error condition not checked
- 272579 - dereference before null check
- 272586 - error condition not checked
There are two additional issues reported by coverity:
- 272598 - error condition not checked
- 272599 - error condition not checked
However, they are fixed by a separate patch [1].
Quoted 7 lines
[1] http://dpdk.org/dev/patchwork/patch/38091/
Anatoly Burakov (4):
fbarray: use strlcpy instead of snprintf
fbarray: add check for failed file descriptor open
fbarray: fix potential null-dereference
fbarray: handle negative return

Quoted 3 lines

Given that I recently addressed a similar issue in fbarray as well:
Acked-by Adrien Mazarguil <[email protected]>

Applied, thanks