[PATCH 0/4] Fixes for coverity issues in fbarray

[PATCH 0/4] Fixes for coverity issues in fbarray

[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].

[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(-)

-- 
2.7.4

[PATCH 1/4] fbarray: use strlcpy instead of snprintf

[Anatoly Burakov]

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_fbarray.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_fbarray.c b/lib/librte_eal/common/eal_common_fbarray.c
index f65875d..8aa9013 100644
--- a/lib/librte_eal/common/eal_common_fbarray.c
+++ b/lib/librte_eal/common/eal_common_fbarray.c
@@ -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;
-- 
2.7.4

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

[Anatoly Burakov]

Coverity issue: 272564

Fixes: c44d09811b40 ("eal: add shared indexed file-backed array")
Cc: anatoly.burakov@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_fbarray.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_fbarray.c b/lib/librte_eal/common/eal_common_fbarray.c
index 8aa9013..95a7c8e 100644
--- a/lib/librte_eal/common/eal_common_fbarray.c
+++ b/lib/librte_eal/common/eal_common_fbarray.c
@@ -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;
-- 
2.7.4

[PATCH 3/4] fbarray: fix potential null-dereference

[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: anatoly.burakov@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_fbarray.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_fbarray.c b/lib/librte_eal/common/eal_common_fbarray.c
index 95a7c8e..c4ed97e 100644
--- a/lib/librte_eal/common/eal_common_fbarray.c
+++ b/lib/librte_eal/common/eal_common_fbarray.c
@@ -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 */
-- 
2.7.4

[PATCH 4/4] fbarray: handle negative return

[Anatoly Burakov]

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

Coverity issue: 272586

Fixes: c44d09811b40 ("eal: add shared indexed file-backed array")
Cc: anatoly.burakov@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_fbarray.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_fbarray.c b/lib/librte_eal/common/eal_common_fbarray.c
index c4ed97e..6486679 100644
--- a/lib/librte_eal/common/eal_common_fbarray.c
+++ b/lib/librte_eal/common/eal_common_fbarray.c
@@ -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.
-- 
2.7.4

Re: [PATCH 0/4] Fixes for coverity issues in fbarray

[Adrien Mazarguil]

On Tue, Apr 17, 2018 at 04:44:04PM +0100, Anatoly Burakov wrote:
> 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

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

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

-- 
Adrien Mazarguil
6WIND

Re: [PATCH 0/4] Fixes for coverity issues in fbarray

[Thomas Monjalon]

18/04/2018 10:51, Adrien Mazarguil:
> On Tue, Apr 17, 2018 at 04:44:04PM +0100, Anatoly Burakov wrote:
> > 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
> 
> Given that I recently addressed a similar issue in fbarray as well:
> 
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Applied, thanks