[dpdk-dev] [PATCH 0/3] (More) Fixes for issues highlighted by static analysis

[dpdk-dev] [PATCH 0/3] (More) Fixes for issues highlighted by static analysis

[Bruce Richardson]

A further three small patches fixing more issues highlighted by static 
analysis scans.

Bruce Richardson (3):
  af_packet: ensure *internals is not null when dereferencing
  ixgbe: prevent array overflow access in vector driver
  eal: for safety, use snprintf instead of sprintf

 lib/librte_eal/common/include/rte_version.h  |  4 ++--
 lib/librte_pmd_af_packet/rte_eth_af_packet.c | 15 ++++++++-------
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c        |  2 +-
 3 files changed, 11 insertions(+), 10 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH 1/3] af_packet: ensure *internals is not null when dereferencing

[Bruce Richardson]

The cleanup code on error checks for *internals being NULL only after
using the pointer to perform other cleanup. Fix this by moving the
clean-up based on the pointer inside the check for NULL.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_pmd_af_packet/rte_eth_af_packet.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
index d0fb3eb..ad7242c 100644
--- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
+++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
@@ -676,14 +676,15 @@ error:
 		rte_free(data);
 	if (pci_dev)
 		rte_free(pci_dev);
-	for (q = 0; q < nb_queues; q++) {
-		if ((*internals)->rx_queue[q].rd)
-			rte_free((*internals)->rx_queue[q].rd);
-		if ((*internals)->tx_queue[q].rd)
-			rte_free((*internals)->tx_queue[q].rd);
-	}
-	if (*internals)
+	if (*internals) {
+		for (q = 0; q < nb_queues; q++) {
+			if ((*internals)->rx_queue[q].rd)
+				rte_free((*internals)->rx_queue[q].rd);
+			if ((*internals)->tx_queue[q].rd)
+				rte_free((*internals)->tx_queue[q].rd);
+		}
 		rte_free(*internals);
+	}
 	return -1;
 }
 
-- 
1.9.3

[dpdk-dev] [PATCH 2/3] ixgbe: prevent array overflow access in vector driver

[Bruce Richardson]

Switch the order of the conditions in a while loop, so we check the
range of "i" against the max, before using it to index into the array.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index 3a30fa7..b54cb19 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -489,7 +489,7 @@ ixgbe_recv_scattered_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	unsigned i = 0;
 	if (rxq->pkt_first_seg == NULL) {
 		/* find the first split flag, and only reassemble then*/
-		while (!split_flags[i] && i < nb_bufs)
+		while (i < nb_bufs && !split_flags[i])
 			i++;
 		if (i == nb_bufs)
 			return nb_bufs;
-- 
1.9.3

[dpdk-dev] [PATCH 3/3] eal: for safety, use snprintf instead of sprintf

[Bruce Richardson]

When printing the version string to a local variable, use snprintf for
safety over sprintf. This is general good practice even if the values
to print are all hard-coded.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/common/include/rte_version.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_version.h b/lib/librte_eal/common/include/rte_version.h
index 54cb9a6..356207c 100644
--- a/lib/librte_eal/common/include/rte_version.h
+++ b/lib/librte_eal/common/include/rte_version.h
@@ -104,13 +104,13 @@ rte_version(void)
 	if (version[0] != 0)
 		return version;
 	if (strlen(RTE_VER_SUFFIX) == 0)
-		sprintf(version, "%s %d.%d.%d",
+		snprintf(version, sizeof(version), "%s %d.%d.%d",
 			RTE_VER_PREFIX,
 			RTE_VER_MAJOR,
 			RTE_VER_MINOR,
 			RTE_VER_PATCH_LEVEL);
 	else
-		sprintf(version, "%s %d.%d.%d%s%d",
+		snprintf(version, sizeof(version), "%s %d.%d.%d%s%d",
 			RTE_VER_PREFIX,
 			RTE_VER_MAJOR,
 			RTE_VER_MINOR,
-- 
1.9.3

Re: [dpdk-dev] [PATCH 0/3] (More) Fixes for issues highlighted by static analysis

[Thomas Monjalon]

> A further three small patches fixing more issues highlighted by static 
> analysis scans.
> 
> Bruce Richardson (3):
>   af_packet: ensure *internals is not null when dereferencing
>   ixgbe: prevent array overflow access in vector driver
>   eal: for safety, use snprintf instead of sprintf

Applied

Thanks
-- 
Thomas