[dpdk-dev,v2,2/8] net/bnxt: fix to avoid a segfault
Checks
Commit Message
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>
--
v1->v2: incorporate review feedback.
---
drivers/net/bnxt/rte_pmd_bnxt.c | 101 +++++++++++++++++++---------------------
1 file changed, 49 insertions(+), 52 deletions(-)
Comments
So the only change was replacing a nesting level with a continue? Yeah,
looks good.
On Thu, Jul 20, 2017 at 8:22 PM, Ajit Khaparde <ajit.khaparde@broadcom.com>
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>
>
> --
> v1->v2: incorporate review feedback.
> ---
> drivers/net/bnxt/rte_pmd_bnxt.c | 101 +++++++++++++++++++-----------
> ----------
> 1 file changed, 49 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/net/bnxt/rte_pmd_bnxt.c b/drivers/net/bnxt/rte_pmd_
> bnxt.c
> index 0a8fb1e..ec5855d 100644
> --- a/drivers/net/bnxt/rte_pmd_bnxt.c
> +++ b/drivers/net/bnxt/rte_pmd_bnxt.c
> @@ -473,62 +473,59 @@ int rte_pmd_bnxt_set_vf_vlan_filter(uint8_t port,
> uint16_t vlan,
>
> for (i = 0; vf_mask; i++, vf_mask >>= 1) {
> cnt = bp->pf.vf_info[i].vlan_count;
> - if (vf_mask & 1) {
> - if (bp->pf.vf_info[i].vlan_table == NULL) {
> - rc = -1;
> - continue;
> + if ((vf_mask & 1) == 0)
> + continue;
> +
> + if (bp->pf.vf_info[i].vlan_table == NULL) {
> + rc = -1;
> + continue;
> + }
> + if (vlan_on) {
> + /* First, search for a duplicate... */
> + for (j = 0; j < cnt; j++) {
> + if (rte_be_to_cpu_16(
> + bp->pf.vf_info[i].vlan_table[j].vid)
> == vlan)
> + break;
> }
> - if (vlan_on) {
> - /* First, search for a duplicate... */
> - for (j = 0; j < cnt; j++) {
> - if (rte_be_to_cpu_16(
> - bp->pf.vf_info[i].vlan_table[j].vid)
> ==
> - vlan)
> - break;
> - }
> - if (j == cnt) {
> - /* Now check that there's space */
> - if (cnt == getpagesize() /
> - sizeof(struct
> bnxt_vlan_table_entry)) {
> - RTE_LOG(ERR, PMD,
> - "VF %d VLAN table is
> full\n",
> - i);
> - RTE_LOG(ERR, PMD,
> - "cannot add VLAN
> %u\n",
> - vlan);
> - rc = -1;
> - continue;
> - }
> -
> - cnt =
> bp->pf.vf_info[i].vlan_count++;
> - /*
> - * And finally, add to the
> - * end of the table
> - */
> - ve = &bp->pf.vf_info[i].vlan_table[
> cnt];
> - /* TODO: Hardcoded TPID */
> - ve->tpid =
> rte_cpu_to_be_16(0x8100);
> - ve->vid = rte_cpu_to_be_16(vlan);
> - }
> - } else {
> - for (j = 0; cnt; j++) {
> - if (rte_be_to_cpu_16(
> - 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) *
> - sizeof(struct
> bnxt_vlan_table_entry)));
> - j--;
> - cnt =
> bp->pf.vf_info[i].vlan_count--;
> + if (j == cnt) {
> + /* Now check that there's space */
> + if (cnt == getpagesize() /
> + sizeof(struct bnxt_vlan_table_entry)) {
> + RTE_LOG(ERR, PMD,
> + "VF %d VLAN table is
> full\n",
> + i);
> + RTE_LOG(ERR, PMD,
> + "cannot add VLAN %u\n",
> vlan);
> + rc = -1;
> + continue;
> }
> +
> + /* cnt is one less than vlan_count */
> + cnt = bp->pf.vf_info[i].vlan_count++;
> + /*
> + * And finally, add to the
> + * end of the table
> + */
> + ve = &bp->pf.vf_info[i].vlan_table[cnt];
> + /* TODO: Hardcoded TPID */
> + ve->tpid = rte_cpu_to_be_16(0x8100);
> + ve->vid = rte_cpu_to_be_16(vlan);
> + }
> + } else {
> + for (j = 0; j < cnt; j++) {
> + if (rte_be_to_cpu_16(
> + 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) *
> + sizeof(struct
> bnxt_vlan_table_entry)));
> + j--;
> + cnt = --bp->pf.vf_info[i].vlan_count;
> }
> - rte_pmd_bnxt_set_vf_vlan_anti_
> spoof(dev->data->port_id,
> - i, bp->pf.vf_info[i].vlan_spoof_
> en);
> }
> + rte_pmd_bnxt_set_vf_vlan_anti_spoof(dev->data->port_id, i,
> + bp->pf.vf_info[i].vlan_spoof_en);
> }
>
> return rc;
> --
> 2.10.1 (Apple Git-78)
>
>
@@ -473,62 +473,59 @@ int rte_pmd_bnxt_set_vf_vlan_filter(uint8_t port, uint16_t vlan,
for (i = 0; vf_mask; i++, vf_mask >>= 1) {
cnt = bp->pf.vf_info[i].vlan_count;
- if (vf_mask & 1) {
- if (bp->pf.vf_info[i].vlan_table == NULL) {
- rc = -1;
- continue;
+ if ((vf_mask & 1) == 0)
+ continue;
+
+ if (bp->pf.vf_info[i].vlan_table == NULL) {
+ rc = -1;
+ continue;
+ }
+ if (vlan_on) {
+ /* First, search for a duplicate... */
+ for (j = 0; j < cnt; j++) {
+ if (rte_be_to_cpu_16(
+ bp->pf.vf_info[i].vlan_table[j].vid) == vlan)
+ break;
}
- if (vlan_on) {
- /* First, search for a duplicate... */
- for (j = 0; j < cnt; j++) {
- if (rte_be_to_cpu_16(
- bp->pf.vf_info[i].vlan_table[j].vid) ==
- vlan)
- break;
- }
- if (j == cnt) {
- /* Now check that there's space */
- if (cnt == getpagesize() /
- sizeof(struct bnxt_vlan_table_entry)) {
- RTE_LOG(ERR, PMD,
- "VF %d VLAN table is full\n",
- i);
- RTE_LOG(ERR, PMD,
- "cannot add VLAN %u\n",
- vlan);
- rc = -1;
- continue;
- }
-
- cnt = bp->pf.vf_info[i].vlan_count++;
- /*
- * And finally, add to the
- * end of the table
- */
- ve = &bp->pf.vf_info[i].vlan_table[cnt];
- /* TODO: Hardcoded TPID */
- ve->tpid = rte_cpu_to_be_16(0x8100);
- ve->vid = rte_cpu_to_be_16(vlan);
- }
- } else {
- for (j = 0; cnt; j++) {
- if (rte_be_to_cpu_16(
- 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) *
- sizeof(struct bnxt_vlan_table_entry)));
- j--;
- cnt = bp->pf.vf_info[i].vlan_count--;
+ if (j == cnt) {
+ /* Now check that there's space */
+ if (cnt == getpagesize() /
+ sizeof(struct bnxt_vlan_table_entry)) {
+ RTE_LOG(ERR, PMD,
+ "VF %d VLAN table is full\n",
+ i);
+ RTE_LOG(ERR, PMD,
+ "cannot add VLAN %u\n", vlan);
+ rc = -1;
+ continue;
}
+
+ /* cnt is one less than vlan_count */
+ cnt = bp->pf.vf_info[i].vlan_count++;
+ /*
+ * And finally, add to the
+ * end of the table
+ */
+ ve = &bp->pf.vf_info[i].vlan_table[cnt];
+ /* TODO: Hardcoded TPID */
+ ve->tpid = rte_cpu_to_be_16(0x8100);
+ ve->vid = rte_cpu_to_be_16(vlan);
+ }
+ } else {
+ for (j = 0; j < cnt; j++) {
+ if (rte_be_to_cpu_16(
+ 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) *
+ sizeof(struct bnxt_vlan_table_entry)));
+ j--;
+ cnt = --bp->pf.vf_info[i].vlan_count;
}
- rte_pmd_bnxt_set_vf_vlan_anti_spoof(dev->data->port_id,
- i, bp->pf.vf_info[i].vlan_spoof_en);
}
+ rte_pmd_bnxt_set_vf_vlan_anti_spoof(dev->data->port_id, i,
+ bp->pf.vf_info[i].vlan_spoof_en);
}
return rc;