[dpdk-dev] [PATCH 0/4] random fixes for bugs identified by Coverity

[dpdk-dev] [PATCH 0/4] random fixes for bugs identified by Coverity

[John W. Linville]

Today I attempted to exercise my brain between meetings by handling
a few bugs identified by Coverity.

All of the fixes are simple and (hopefully) obvious. They are
compile-tested only.

John
--
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

[dpdk-dev] [PATCH 1/4] ena_config_host_info: improve safety of string handling

[John W. Linville]

Use sizeof dest rather than sizeof src for limiting copy length,
and replace strncpy with snprintf to ensure NULL termination.

Coverity: 127795

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/ena/ena_ethdev.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index d106f5f39dce..649cb17d4a06 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -342,11 +342,13 @@ static void ena_config_host_info(struct ena_com_dev *ena_dev)
 
 	host_info->os_type = ENA_ADMIN_OS_DPDK;
 	host_info->kernel_ver = RTE_VERSION;
-	strncpy((char *)host_info->kernel_ver_str, rte_version(),
-		strlen(rte_version()));
+	snprintf((char *)host_info->kernel_ver_str,
+		 sizeof(host_info->kernel_ver_str),
+		 "%s", rte_version());
 	host_info->os_dist = RTE_VERSION;
-	strncpy((char *)host_info->os_dist_str, rte_version(),
-		strlen(rte_version()));
+	snprintf((char *)host_info->os_dist_str,
+		 sizeof(host_info->os_dist_str),
+		 "%s", rte_version());
 	host_info->driver_version =
 		(DRV_MODULE_VER_MAJOR) |
 		(DRV_MODULE_VER_MINOR << ENA_ADMIN_HOST_INFO_MINOR_SHIFT) |
-- 
2.7.4

[dpdk-dev] [PATCH 2/4] bnxt_alloc_vnic_attributes: ensure entry_length is unsigned

[John W. Linville]

Otherwise, the inherent cast when multiplying entry_length by max_vnics
in the call to rte_memzone_reserve could promote max_vnics to a signed
value, causing hilarity to ensue...

Coverity: 127557

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/bnxt/bnxt_vnic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
index c04c4c740645..1b5f54c4d1ae 100644
--- a/drivers/net/bnxt/bnxt_vnic.c
+++ b/drivers/net/bnxt/bnxt_vnic.c
@@ -175,7 +175,7 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
 	struct rte_pci_device *pdev = bp->pdev;
 	const struct rte_memzone *mz;
 	char mz_name[RTE_MEMZONE_NAMESIZE];
-	int entry_length = RTE_CACHE_LINE_ROUNDUP(
+	uint16_t entry_length = RTE_CACHE_LINE_ROUNDUP(
 				HW_HASH_INDEX_SIZE * sizeof(*vnic->rss_table) +
 				HW_HASH_KEY_SIZE);
 	uint16_t max_vnics;
-- 
2.7.4

[dpdk-dev] [PATCH 3/4] i40e_vsi_release: do not use vsi until after NULL check

[John W. Linville]

Coverity: 127556

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/i40e/i40e_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index b04c833b2e04..d1f0f762b04c 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -4097,11 +4097,13 @@ i40e_vsi_release(struct i40e_vsi *vsi)
 	void *temp;
 	int ret;
 	struct i40e_mac_filter *f;
-	uint16_t user_param = vsi->user_param;
+	uint16_t user_param;
 
 	if (!vsi)
 		return I40E_SUCCESS;
 
+	user_param = vsi->user_param;
+
 	pf = I40E_VSI_TO_PF(vsi);
 	hw = I40E_VSI_TO_HW(vsi);
 
-- 
2.7.4

[dpdk-dev] [PATCH 4/4] bnxt_mac_addr_remove_op: add ULL suffix to constant 1 in bit shift

[John W. Linville]

Some(?) compilers will treat the unmarked constant 1 as a 32-bit
integer, but the shift operation is in a loop that could run up to
63 times -- undefined behavior!

Coverity: 127546

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index f4eedfd812bb..d7447b15983b 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -463,7 +463,7 @@ static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
 	 * remove the corresponding MAC addr filter
 	 */
 	for (i = 0; i < MAX_FF_POOLS; i++) {
-		if (!(pool_mask & (1 << i)))
+		if (!(pool_mask & (1ULL << i)))
 			continue;
 
 		STAILQ_FOREACH(vnic, &bp->ff_pool[i], next) {
-- 
2.7.4

Re: [dpdk-dev] [PATCH 3/4] i40e_vsi_release: do not use vsi until after NULL check

[Ferruh Yigit]

On 9/28/2016 8:20 PM, John W. Linville wrote:
> Coverity: 127556
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>


a few minor nits:
- patch subject tag/label should be "net/<driver>: ", so patch subject:
"net/i40e: do not use VSI until after NULL check"

- These are fixes, requires fixes line, for this patch:
Fixes: 440499cf5376 ("net/i40e: support floating VEB")

- For coverity fixes, defined usage is: "Coverity issue: XXXXX"

- Can be good to CC stable tree for these fixes.


Overall patch comment becomes something like:
"
net/i40e: do not use VSI until after NULL check

Coverity issue: 127556
Fixes: 440499cf5376 ("net/i40e: support floating VEB")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
"

[dpdk-dev] [PATCH v2 0/4] random fixes for bugs identified by Coverity

[John W. Linville]

Today I attempted to exercise my brain between meetings by handling
a few bugs identified by Coverity.

All of the fixes are simple and (hopefully) obvious. They are
compile-tested only.

v2: fix-up changelog entries based-on comments from Ferruh Yigit

John
--
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

[dpdk-dev] [PATCH v2 1/4] net/ena: improve safety of string handling in ena_config_host_info

[John W. Linville]

Use sizeof dest rather than sizeof src for limiting copy length,
and replace strncpy with snprintf to ensure NULL termination.

Coverity issue: 127795
Fixes: 372c1af5ed8f ("net/ena: add dedicated memory area for extra device info")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
v2: fix-up changelog entries based-on comments from Ferruh Yigit

 drivers/net/ena/ena_ethdev.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index d106f5f39dce..649cb17d4a06 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -342,11 +342,13 @@ static void ena_config_host_info(struct ena_com_dev *ena_dev)
 
 	host_info->os_type = ENA_ADMIN_OS_DPDK;
 	host_info->kernel_ver = RTE_VERSION;
-	strncpy((char *)host_info->kernel_ver_str, rte_version(),
-		strlen(rte_version()));
+	snprintf((char *)host_info->kernel_ver_str,
+		 sizeof(host_info->kernel_ver_str),
+		 "%s", rte_version());
 	host_info->os_dist = RTE_VERSION;
-	strncpy((char *)host_info->os_dist_str, rte_version(),
-		strlen(rte_version()));
+	snprintf((char *)host_info->os_dist_str,
+		 sizeof(host_info->os_dist_str),
+		 "%s", rte_version());
 	host_info->driver_version =
 		(DRV_MODULE_VER_MAJOR) |
 		(DRV_MODULE_VER_MINOR << ENA_ADMIN_HOST_INFO_MINOR_SHIFT) |
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/4] net/bnxt: ensure entry_length is unsigned in bnxt_alloc_vnic_attributes

[John W. Linville]

Otherwise, the inherent cast when multiplying entry_length by max_vnics
in the call to rte_memzone_reserve could promote max_vnics to a signed
value, causing hilarity to ensue...

Coverity issue: 127557
Fixes: 9738793f28ec ("net/bnxt: add VNIC functions and structs")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
v2: fix-up changelog entries based-on comments from Ferruh Yigit

 drivers/net/bnxt/bnxt_vnic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
index c04c4c740645..1b5f54c4d1ae 100644
--- a/drivers/net/bnxt/bnxt_vnic.c
+++ b/drivers/net/bnxt/bnxt_vnic.c
@@ -175,7 +175,7 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
 	struct rte_pci_device *pdev = bp->pdev;
 	const struct rte_memzone *mz;
 	char mz_name[RTE_MEMZONE_NAMESIZE];
-	int entry_length = RTE_CACHE_LINE_ROUNDUP(
+	uint16_t entry_length = RTE_CACHE_LINE_ROUNDUP(
 				HW_HASH_INDEX_SIZE * sizeof(*vnic->rss_table) +
 				HW_HASH_KEY_SIZE);
 	uint16_t max_vnics;
-- 
2.7.4

[dpdk-dev] [PATCH v2 3/4] net/i40e: do not use vsi until after NULL check in i40e_vsi_release

[John W. Linville]

Coverity issue: 127556
Fixes: 440499cf5376 ("net/i40e: support floating VEB")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
v2: fix-up changelog entries based-on comments from Ferruh Yigit

 drivers/net/i40e/i40e_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index b04c833b2e04..d1f0f762b04c 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -4097,11 +4097,13 @@ i40e_vsi_release(struct i40e_vsi *vsi)
 	void *temp;
 	int ret;
 	struct i40e_mac_filter *f;
-	uint16_t user_param = vsi->user_param;
+	uint16_t user_param;
 
 	if (!vsi)
 		return I40E_SUCCESS;
 
+	user_param = vsi->user_param;
+
 	pf = I40E_VSI_TO_PF(vsi);
 	hw = I40E_VSI_TO_HW(vsi);
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 4/4] net/bnxt: add ULL suffix to constant 1 for bit shift in bnxt_mac_addr_remove_op

[John W. Linville]

Some(?) compilers will treat the unmarked constant 1 as a 32-bit
integer, but the shift operation is in a loop that could run up to
63 times -- undefined behavior!

Coverity issue: 127546
Fixes: 778b759ba10e ("net/bnxt: add MAC address")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
v2: fix-up changelog entries based-on comments from Ferruh Yigit

 drivers/net/bnxt/bnxt_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index f4eedfd812bb..d7447b15983b 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -463,7 +463,7 @@ static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
 	 * remove the corresponding MAC addr filter
 	 */
 	for (i = 0; i < MAX_FF_POOLS; i++) {
-		if (!(pool_mask & (1 << i)))
+		if (!(pool_mask & (1ULL << i)))
 			continue;
 
 		STAILQ_FOREACH(vnic, &bp->ff_pool[i], next) {
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 2/4] net/bnxt: ensure entry_length is unsigned in bnxt_alloc_vnic_attributes

[Ajit Khaparde]

On Thu, Sep 29, 2016 at 12:39 PM, John W. Linville <linville@tuxdriver.com>
wrote:

> Otherwise, the inherent cast when multiplying entry_length by max_vnics
> in the call to rte_memzone_reserve could promote max_vnics to a signed
> value, causing hilarity to ensue...
>
> Coverity issue: 127557
> Fixes: 9738793f28ec ("net/bnxt: add VNIC functions and structs")
>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
>
​Acked
-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

---
> v2: fix-up changelog entries based-on comments from Ferruh Yigit
>
>  drivers/net/bnxt/bnxt_vnic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
> index c04c4c740645..1b5f54c4d1ae 100644
> --- a/drivers/net/bnxt/bnxt_vnic.c
> +++ b/drivers/net/bnxt/bnxt_vnic.c
> @@ -175,7 +175,7 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
>         struct rte_pci_device *pdev = bp->pdev;
>         const struct rte_memzone *mz;
>         char mz_name[RTE_MEMZONE_NAMESIZE];
> -       int entry_length = RTE_CACHE_LINE_ROUNDUP(
> +       uint16_t entry_length = RTE_CACHE_LINE_ROUNDUP(
>                                 HW_HASH_INDEX_SIZE *
> sizeof(*vnic->rss_table) +
>                                 HW_HASH_KEY_SIZE);
>         uint16_t max_vnics;
> --
> 2.7.4
>
>

Re: [dpdk-dev] [PATCH v2 4/4] net/bnxt: add ULL suffix to constant 1 for bit shift in bnxt_mac_addr_remove_op

[Ajit Khaparde]

On Thu, Sep 29, 2016 at 12:39 PM, John W. Linville <linville@tuxdriver.com>
wrote:

> Some(?) compilers will treat the unmarked constant 1 as a 32-bit
> integer, but the shift operation is in a loop that could run up to
> 63 times -- undefined behavior!
>
> Coverity issue: 127546
> Fixes: 778b759ba10e ("net/bnxt: add MAC address")
>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
>
​Acked
-by: Ajit Khaparde <ajit.khaparde@broadcom.com>


> ---
> v2: fix-up changelog entries based-on comments from Ferruh Yigit
>
>  drivers/net/bnxt/bnxt_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev
> .c
> index f4eedfd812bb..d7447b15983b 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -463,7 +463,7 @@ static void bnxt_mac_addr_remove_op(struct rte_eth_dev
> *eth_dev,
>          * remove the corresponding MAC addr filter
>          */
>         for (i = 0; i < MAX_FF_POOLS; i++) {
> -               if (!(pool_mask & (1 << i)))
> +               if (!(pool_mask & (1ULL << i)))
>                         continue;
>
>                 STAILQ_FOREACH(vnic, &bp->ff_pool[i], next) {
> --
> 2.7.4
>
>

Re: [dpdk-dev] [dpdk-stable] [PATCH v2 1/4] net/ena: improve safety of string handling in ena_config_host_info

[Ferruh Yigit]

On 9/29/2016 6:39 PM, John W. Linville wrote:
> Use sizeof dest rather than sizeof src for limiting copy length,
> and replace strncpy with snprintf to ensure NULL termination.
> 
> Coverity issue: 127795
> Fixes: 372c1af5ed8f ("net/ena: add dedicated memory area for extra device info")
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v2 0/4] random fixes for bugs identified by Coverity

[Bruce Richardson]

On Thu, Sep 29, 2016 at 01:39:32PM -0400, John W. Linville wrote:
> Today I attempted to exercise my brain between meetings by handling
> a few bugs identified by Coverity.
> 
> All of the fixes are simple and (hopefully) obvious. They are
> compile-tested only.
> 
> v2: fix-up changelog entries based-on comments from Ferruh Yigit
> 
Series applied to dpdk-next-net/rel_16_11

/Bruce