[dpdk-dev,2/8] net/bnxt: fix to avoid a segfault

Message ID 20170720044826.44103-4-ajit.khaparde@broadcom.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ajit Khaparde July 20, 2017, 4:48 a.m. UTC
  Fix use of local variable to avoid segfault.
cnt was incorrectly tested and decremented in the loop that removes
a VLAN from the table.

Fixes: 36735a932ca7 ("support set VF QOS and MAC anti spoof")

Signed-off-by: Stephen Hurd <stephen.hurd@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/rte_pmd_bnxt.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)
  

Comments

Ferruh Yigit July 20, 2017, 9:56 a.m. UTC | #1
On 7/20/2017 5:48 AM, Ajit Khaparde wrote:
> Fix use of local variable to avoid segfault.
> cnt was incorrectly tested and decremented in the loop that removes
> a VLAN from the table.
> 
> Fixes: 36735a932ca7 ("support set VF QOS and MAC anti spoof")
> 
> Signed-off-by: Stephen Hurd <stephen.hurd@broadcom.com>
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>  drivers/net/bnxt/rte_pmd_bnxt.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/bnxt/rte_pmd_bnxt.c b/drivers/net/bnxt/rte_pmd_bnxt.c
> index 0a8fb1e..0d48873 100644
> --- a/drivers/net/bnxt/rte_pmd_bnxt.c
> +++ b/drivers/net/bnxt/rte_pmd_bnxt.c
> @@ -500,6 +500,7 @@ int rte_pmd_bnxt_set_vf_vlan_filter(uint8_t port, uint16_t vlan,
>  						continue;
>  					}
>  
> +					/* cnt is one less than vlan_count */
>  					cnt = bp->pf.vf_info[i].vlan_count++;
>  					/*
>  					 * And finally, add to the
> @@ -511,19 +512,19 @@ int rte_pmd_bnxt_set_vf_vlan_filter(uint8_t port, uint16_t vlan,
>  					ve->vid = rte_cpu_to_be_16(vlan);
>  				}
>  			} else {
> -				for (j = 0; cnt; j++) {
> +				for (j = 0; j < cnt; j++) {
>  					if (rte_be_to_cpu_16(
> -					bp->pf.vf_info[i].vlan_table[j].vid) !=
> -					    vlan)
> +					 bp->pf.vf_info[i].vlan_table[j].vid) !=
> +					 vlan)
>  						continue;
>  					memmove(
> -					 &bp->pf.vf_info[i].vlan_table[j],
> -					 &bp->pf.vf_info[i].vlan_table[j + 1],

I may be missing something but if &bp->pf.vf_info[i].vlan_table[j +
1].vid == vlan, this may be endless loop.

because each time vlan_table[j + 1] copied into vlan_table[j] and j--,
in next iteration same thing will happen.


> -					 getpagesize() -
> -					 ((j + 1) *
> +					    &bp->pf.vf_info[i].vlan_table[j],
> +					   &bp->pf.vf_info[i].vlan_table[j + 1],

I guess line realigned to fit into 80 characters limitation, one of the
good thing with 80 chars limitation is it forces to reduce indention.

It is up to you, but if you do following, it can be possible to reduce
the indention one level and it will be easier to fit into limit:

if.((vf_mask.&.1) == 0)
	continue;

> +					    getpagesize() -
> +					    ((j + 1) *	
>  					 sizeof(struct bnxt_vlan_table_entry)));
>  					j--;
> -					cnt = bp->pf.vf_info[i].vlan_count--;
> +					cnt = --bp->pf.vf_info[i].vlan_count;
>  				}
>  			}
>  			rte_pmd_bnxt_set_vf_vlan_anti_spoof(dev->data->port_id,
>
  

Patch

diff --git a/drivers/net/bnxt/rte_pmd_bnxt.c b/drivers/net/bnxt/rte_pmd_bnxt.c
index 0a8fb1e..0d48873 100644
--- a/drivers/net/bnxt/rte_pmd_bnxt.c
+++ b/drivers/net/bnxt/rte_pmd_bnxt.c
@@ -500,6 +500,7 @@  int rte_pmd_bnxt_set_vf_vlan_filter(uint8_t port, uint16_t vlan,
 						continue;
 					}
 
+					/* cnt is one less than vlan_count */
 					cnt = bp->pf.vf_info[i].vlan_count++;
 					/*
 					 * And finally, add to the
@@ -511,19 +512,19 @@  int rte_pmd_bnxt_set_vf_vlan_filter(uint8_t port, uint16_t vlan,
 					ve->vid = rte_cpu_to_be_16(vlan);
 				}
 			} else {
-				for (j = 0; cnt; j++) {
+				for (j = 0; j < cnt; j++) {
 					if (rte_be_to_cpu_16(
-					bp->pf.vf_info[i].vlan_table[j].vid) !=
-					    vlan)
+					 bp->pf.vf_info[i].vlan_table[j].vid) !=
+					 vlan)
 						continue;
 					memmove(
-					 &bp->pf.vf_info[i].vlan_table[j],
-					 &bp->pf.vf_info[i].vlan_table[j + 1],
-					 getpagesize() -
-					 ((j + 1) *
+					    &bp->pf.vf_info[i].vlan_table[j],
+					   &bp->pf.vf_info[i].vlan_table[j + 1],
+					    getpagesize() -
+					    ((j + 1) *
 					 sizeof(struct bnxt_vlan_table_entry)));
 					j--;
-					cnt = bp->pf.vf_info[i].vlan_count--;
+					cnt = --bp->pf.vf_info[i].vlan_count;
 				}
 			}
 			rte_pmd_bnxt_set_vf_vlan_anti_spoof(dev->data->port_id,