net/bonding: fix buf corruption in merging un-transmitted packets
Subsystems (2)
PATCH v2 1/1 net/bonding: fix buf corruption in merging un-transmitted packets
When bond slave devices cannot transmit all packets in bufs array, tx_burst callback shall merge the un-transmitted packets back to bufs array. Recent merge logic introduced a bug which causes invalid mbuf addresses being written to bufs array.
When caller frees the un-transmitted packets, due to invalid addresses, application will crash.
The fix is avoid shifting mbufs, and directly write un-transmitted packets back to bufs array.
Fixes 09150784a776 ("net/bonding: burst mode hash calculation")
Signed-off-by Jia Yu <[email protected]>
--- drivers/net/bonding/rte_eth_bond_pmd.c | 116 +++++++-------------------------- 1 file changed, 23 insertions(+), 93 deletions(-)
drivers/net/bonding/rte_eth_bond_pmd.c +23 -93
@@ -300,10 +300,10 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs,/* Mapping array generated by hash function to map mbufs to slaves */ uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS ] = {0 }; -uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS ] = {0 }; +uint16_t slave_tx_count;uint16_t total_tx_count =0 , total_tx_fail_count =0 ; -uint16_t i, j; +uint16_t i;if (unlikely (nb_bufs ==0 ))return 0 ;
@@ -358,34 +358,12 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs,/* If tx burst fails move packets to end of bufs */ if (unlikely (slave_tx_count < slave_nb_bufs[i])) { - slave_tx_fail_count[i] = slave_nb_bufs[i] - +int slave_tx_fail_count = slave_nb_bufs[i] - slave_tx_count; - total_tx_fail_count += slave_tx_fail_count[i]; - -/* -* Shift bufs to beginning of array to allow reordering -* later -*/ -for (j =0 ; j < slave_tx_fail_count[i]; j++) { - slave_bufs[i][j] = - slave_bufs[i][(slave_tx_count -1 ) + j]; - } - } - } - -/* -* If there are tx burst failures we move packets to end of bufs to -* preserve expected PMD behaviour of all failed transmitted being -* at the end of the input mbuf array -*/ -if (unlikely (total_tx_fail_count >0 )) { -int bufs_idx = nb_bufs - total_tx_fail_count -1 ; - -for (i =0 ; i < slave_count; i++) { -if (slave_tx_fail_count[i] >0 ) { -for (j =0 ; j < slave_tx_fail_count[i]; j++) - bufs[bufs_idx++] = slave_bufs[i][j]; - } + total_tx_fail_count += slave_tx_fail_count; +memcpy (&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count *sizeof (bufs[0 ])); } }
@@ -715,8 +693,8 @@ bond_ethdev_tx_burst_round_robin(void *queue, struct rte_mbuf **bufs, tx_fail_total += tx_fail_slave;memcpy (&bufs[nb_pkts - tx_fail_total], - &slave_bufs[i][num_tx_slave], - tx_fail_slave *sizeof (bufs[0 ])); + &slave_bufs[i][num_tx_slave], + tx_fail_slave *sizeof (bufs[0 ])); } num_tx_total += num_tx_slave; }
@@ -1221,10 +1199,10 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs,/* Mapping array generated by hash function to map mbufs to slaves */ uint16_t bufs_slave_port_idxs[nb_bufs]; -uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS ] = {0 }; +uint16_t slave_tx_count;uint16_t total_tx_count =0 , total_tx_fail_count =0 ; -uint16_t i, j; +uint16_t i;if (unlikely (nb_bufs ==0 ))return 0 ;
@@ -1265,34 +1243,12 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs,/* If tx burst fails move packets to end of bufs */ if (unlikely (slave_tx_count < slave_nb_bufs[i])) { - slave_tx_fail_count[i] = slave_nb_bufs[i] - +int slave_tx_fail_count = slave_nb_bufs[i] - slave_tx_count; - total_tx_fail_count += slave_tx_fail_count[i]; - -/* -* Shift bufs to beginning of array to allow reordering -* later -*/ -for (j =0 ; j < slave_tx_fail_count[i]; j++) { - slave_bufs[i][j] = - slave_bufs[i][(slave_tx_count -1 ) + j]; - } - } - } - -/* -* If there are tx burst failures we move packets to end of bufs to -* preserve expected PMD behaviour of all failed transmitted being -* at the end of the input mbuf array -*/ -if (unlikely (total_tx_fail_count >0 )) { -int bufs_idx = nb_bufs - total_tx_fail_count -1 ; - -for (i =0 ; i < slave_count; i++) { -if (slave_tx_fail_count[i] >0 ) { -for (j =0 ; j < slave_tx_fail_count[i]; j++) - bufs[bufs_idx++] = slave_bufs[i][j]; - } + total_tx_fail_count += slave_tx_fail_count; +memcpy (&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count *sizeof (bufs[0 ])); } }
@@ -1319,10 +1275,10 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,/* Mapping array generated by hash function to map mbufs to slaves */ uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS ] = {0 }; -uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS ] = {0 }; +uint16_t slave_tx_count;uint16_t total_tx_count =0 , total_tx_fail_count =0 ; -uint16_t i, j; +uint16_t i;if (unlikely (nb_bufs ==0 ))return 0 ;
@@ -1380,39 +1336,13 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,/* If tx burst fails move packets to end of bufs */ if (unlikely (slave_tx_count < slave_nb_bufs[i])) { - slave_tx_fail_count[i] = slave_nb_bufs[i] - +int slave_tx_fail_count = slave_nb_bufs[i] - slave_tx_count; - total_tx_fail_count += slave_tx_fail_count[i]; - -/* -* Shift bufs to beginning of array to allow -* reordering later -*/ -for (j =0 ; j < slave_tx_fail_count[i]; j++) - slave_bufs[i][j] = - slave_bufs[i] - [(slave_tx_count -1 ) - + j]; - } - } + total_tx_fail_count += slave_tx_fail_count; -/* -* If there are tx burst failures we move packets to end of -* bufs to preserve expected PMD behaviour of all failed -* transmitted being at the end of the input mbuf array -*/ -if (unlikely (total_tx_fail_count >0 )) { -int bufs_idx = nb_bufs - total_tx_fail_count -1 ; - -for (i =0 ; i < slave_count; i++) { -if (slave_tx_fail_count[i] >0 ) { -for (j =0 ; - j < slave_tx_fail_count[i]; - j++) { - bufs[bufs_idx++] = - slave_bufs[i][j]; - } - } +memcpy (&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count *sizeof (bufs[0 ])); } } }
Signature · 1 line
2.7.4
Quoted 7 lines · Jia Yu
When bond slave devices cannot transmit all packets in bufs array,
tx_burstcallback shall merge the un-transmitted packets back to bufs array. Recent merge logic introduced a bug which causes invalid mbuf addresses being written to bufs array.
When caller frees the un-transmitted packets, due to invalid addresses, application will crash.
Quoted 7 lines
The fix is avoid shifting mbufs, and directly write un-transmitted packets back to bufs array.
Fixes 09150784a776 ("net/bonding: burst mode hash calculation")Signed-off-by Jia Yu <[email protected]>
Acked-by Chas Williams <[email protected]>
Quoted 219 lines
--- drivers/net/bonding/rte_eth_bond_pmd.c | 116 +++++++-------------------------- 1 file changed, 23 insertions(+), 93 deletions(-)drivers/net/bonding/rte_eth_bond_pmd.c +2
-2@@ -300,10 +300,10 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue,struct rte_mbuf **bufs,/* Mapping array generated by hash function to map mbufs to slaves */ uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 }; -uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS ] = { 0 }; +uint16_t slave_tx_count;uint16_t total_tx_count =0 , total_tx_fail_count =0 ; -uint16_t i, j; +uint16_t i;if (unlikely(nb_bufs == 0)) return 0;drivers/net/bonding/rte_eth_bond_pmd.c +1
-5@@ -358,34 +358,12 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue,struct rte_mbuf **bufs,/* If tx burst fails move packets to end of bufs */ if (unlikely(slave_tx_count < slave_nb_bufs[i])) { - slave_tx_fail_count[i] = slave_nb_bufs[i] - + int slave_tx_fail_count = slave_nb_bufs[i] - slave_tx_count; - total_tx_fail_count += slave_tx_fail_count[i]; - -/* -* Shift bufs to beginning of array to allow reordering
drivers/net/bonding/rte_eth_bond_pmd.c +1
-20-* later -*/ -for (j =0 ; j < slave_tx_fail_count[i]; j++) { - slave_bufs[i][j] = - slave_bufs[i][(slave_tx_count - 1) + j]; - } - } - } - -/* -* If there are tx burst failures we move packets to end of bufs to -* preserve expected PMD behaviour of all failed transmitted being -* at the end of the input mbuf array -*/ -if (unlikely (total_tx_fail_count >0 )) { -int bufs_idx = nb_bufs - total_tx_fail_count -1 ; - -for (i =0 ; i < slave_count; i++) { -if (slave_tx_fail_count[i] >0 ) { - for (j = 0; j < slave_tx_fail_count[i];j++)
- bufs[bufs_idx++] =
slave_bufs[i][j];drivers/net/bonding/rte_eth_bond_pmd.c +6
-3- } + total_tx_fail_count += slave_tx_fail_count; +memcpy (&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count *sizeof (bufs[0 ])); } }@@ -715,8 +693,8 @@ bond_ethdev_tx_burst_round_robin(void *queue, struct rte_mbuf **bufs, tx_fail_total += tx_fail_slave;memcpy (&bufs[nb_pkts - tx_fail_total], - &slave_bufs[i][num_tx_slave], - tx_fail_slave *sizeof (bufs[0 ])); + &slave_bufs[i][num_tx_slave], + tx_fail_slave * sizeof(bufs[0]));} num_tx_total += num_tx_slave; }drivers/net/bonding/rte_eth_bond_pmd.c +2
-2@@ -1221,10 +1199,10 @@ bond_ethdev_tx_burst_balance(void *queue, structrte_mbuf **bufs,/* Mapping array generated by hash function to map mbufs to slaves */ uint16_t bufs_slave_port_idxs[nb_bufs]; -uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS ] = { 0 }; +uint16_t slave_tx_count;uint16_t total_tx_count =0 , total_tx_fail_count =0 ; -uint16_t i, j; +uint16_t i;if (unlikely(nb_bufs == 0)) return 0;drivers/net/bonding/rte_eth_bond_pmd.c +1
-5@@ -1265,34 +1243,12 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs,/* If tx burst fails move packets to end of bufs */ if (unlikely(slave_tx_count < slave_nb_bufs[i])) { - slave_tx_fail_count[i] = slave_nb_bufs[i] - +int slave_tx_fail_count = slave_nb_bufs[i] - slave_tx_count; - total_tx_fail_count += slave_tx_fail_count[i]; - -/* -* Shift bufs to beginning of array to allow reordering
drivers/net/bonding/rte_eth_bond_pmd.c +1
-20-* later -*/ -for (j =0 ; j < slave_tx_fail_count[i]; j++) { - slave_bufs[i][j] = - slave_bufs[i][(slave_tx_count - 1) + j]; - } - } - } - -/* -* If there are tx burst failures we move packets to end of bufs to -* preserve expected PMD behaviour of all failed transmitted being -* at the end of the input mbuf array -*/ -if (unlikely (total_tx_fail_count >0 )) { -int bufs_idx = nb_bufs - total_tx_fail_count -1 ; - -for (i =0 ; i < slave_count; i++) { -if (slave_tx_fail_count[i] >0 ) { - for (j = 0; j < slave_tx_fail_count[i];j++)
- bufs[bufs_idx++] =
slave_bufs[i][j];drivers/net/bonding/rte_eth_bond_pmd.c +6
-3- } + total_tx_fail_count += slave_tx_fail_count; +memcpy (&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count *sizeof (bufs[0 ])); } }@@ -1319,10 +1275,10 @@ bond_ethdev_tx_burst_8023ad(void *queue, structrte_mbuf **bufs,/* Mapping array generated by hash function to map mbufs to slaves */ uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 }; -uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS ] = { 0 }; +uint16_t slave_tx_count;uint16_t total_tx_count =0 , total_tx_fail_count =0 ; -uint16_t i, j; +uint16_t i;if (unlikely(nb_bufs == 0)) return 0;drivers/net/bonding/rte_eth_bond_pmd.c +1
-6@@ -1380,39 +1336,13 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,/* If tx burst fails move packets to end of bufs */ if (unlikely(slave_tx_count < slave_nb_bufs[i])) { - slave_tx_fail_count[i] = slave_nb_bufs[i] - + int slave_tx_fail_count = slave_nb_bufs[i] - slave_tx_count; - total_tx_fail_count += slave_tx_fail_count[i]; - -/* -* Shift bufs to beginning of array to allow
drivers/net/bonding/rte_eth_bond_pmd.c +0
-3-* reordering later -*/ - for (j = 0; j < slave_tx_fail_count[i];j++)
drivers/net/bonding/rte_eth_bond_pmd.c +0
-3- slave_bufs[i][j] = - slave_bufs[i] - [(slave_tx_count -1)
drivers/net/bonding/rte_eth_bond_pmd.c +2
-20- + j]; - } - } + total_tx_fail_count += slave_tx_fail_count; -/* -* If there are tx burst failures we move packets to end of -* bufs to preserve expected PMD behaviour of all failed -* transmitted being at the end of the input mbuf array -*/ -if (unlikely (total_tx_fail_count >0 )) { -int bufs_idx = nb_bufs - total_tx_fail_count -1 ; - -for (i =0 ; i < slave_count; i++) { -if (slave_tx_fail_count[i] >0 ) { -for (j =0 ; - j < slave_tx_fail_count[i]; - j++) { - bufs[bufs_idx++] = - slave_bufs[i][j]; - } - } + memcpy(&bufs[nb_bufs -
total_tx_fail_count],drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ &slave_bufs[i][slave_tx_count], + slave_tx_fail_count *sizeof(bufs[0])); } } }--
2.7.4
Ferruh Yigit
AppliedQuoted 12 lines · Chas Williams
Quoted 10 lines · Jia Yu
When bond slave devices cannot transmit all packets in bufs array,
tx_burstcallback shall merge the un-transmitted packets back to bufs array. Recent merge logic introduced a bug which causes invalid mbuf addresses being written to bufs array.
When caller frees the un-transmitted packets, due to invalid addresses, application will crash.The fix is avoid shifting mbufs, and directly write un-transmitted packets back to bufs array.
Quoted 6 lines
Quoted 4 lines
Fixes 09150784a776 ("net/bonding: burst mode hash calculation")Signed-off-by Jia Yu <[email protected]>Acked-by Chas Williams <[email protected]>
Applied to dpdk-next-net/master, thanks.