From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jia Yu Subject: [PATCH] net/bonding: fix buf corruption in merging un-transmitted packets Date: Fri, 17 Aug 2018 17:10:09 -0700 Message-ID: <1534551009-16550-1-git-send-email-jyu@vmware.com> Mime-Version: 1.0 Content-Type: text/plain Cc: declan.doherty@intel.com, chas3@att.com To: dev@dpdk.org Return-path: Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03on0075.outbound.protection.outlook.com [104.47.40.75]) by dpdk.org (Postfix) with ESMTP id 6C6C998 for ; Sat, 18 Aug 2018 03:46:12 +0200 (CEST) List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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. Signed-off-by: Jia Yu --- drivers/net/bonding/rte_eth_bond_pmd.c | 98 +++++----------------------------- 1 file changed, 14 insertions(+), 84 deletions(-) diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index 58f7377..cce973a 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -303,7 +303,7 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs, uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { 0 }; 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; @@ -361,31 +361,9 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs, slave_tx_fail_count[i] = 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]; - } + memcpy(&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count[i] * 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; } @@ -1224,7 +1202,7 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs, uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { 0 }; 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; @@ -1268,31 +1246,9 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs, slave_tx_fail_count[i] = 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]; - } + memcpy(&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count[i] * sizeof(bufs[0])); } } @@ -1322,7 +1278,7 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { 0 }; 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; @@ -1384,35 +1340,9 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, 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]; - } - } + memcpy(&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count[i] * sizeof(bufs[0])); } } } -- 2.7.4