Bonding PMD: Invalid array dimension in TX burst for 802.3ad mode with fast queue leads to SEGFAULT

https://bugs.dpdk.org/show_bug.cgi?id=248

Bug ID: 248
Summary: Bonding PMD: Invalid array dimension in TX burst for
802.3ad mode with fast queue leads to SEGFAULT
Product: DPDK
Version: 18.11
Hardware: All
OS: All
Status: CONFIRMED
Severity: major
Priority: Normal
Component: ethdev
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---

DPDK 18.11.1

In drivers/net/bonding/rte_eth_bond_pmd.c::bond_ethdev_tx_burst_8023ad, bufs_slave_port_idxs array is defined as follows (lines 1293-1294):

    /* Mapping array generated by hash function to map mbufs to slaves */
    uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 };

Array dimension should be equal to number of packets being transmitted (nb_pkts) - as correctly implemented in rte_eth_bond_pmd.c::bond_ethdev_tx_burst_balance.

Invalid array dimension causes overflow when number of transmitted packets is
greater than RTE_MAX_ETHPORTS. Some areas of memory end up overwritten (in my
particular case, slave_nb_bufs array), which leads to SIGSEGV and crash.

To work around the issue, ensure that number of packets transmitted in a single
burst is no greater than RTE_MAX_ETHPORTS.

To fix it, it should be sufficient to define bufs_slave_port_idxs as a variable-length array, as in bond_ethdev_tx_burst_balance:

    /* Mapping array generated by hash function to map mbufs to slaves */
    uint16_t bufs_slave_port_idxs[nb_bufs];

Quoted 39 lines · On Tue, Apr 9, 2019 at 4:45 PM <[email protected]> wrote:

https://bugs.dpdk.org/show_bug.cgi?id=248

Bug ID: 248
Summary: Bonding PMD: Invalid array dimension in TX burst for
802.3ad mode with fast queue leads to SEGFAULT
Product: DPDK
Version: 18.11
Hardware: All
OS: All
Status: CONFIRMED
Severity: major
Priority: Normal
Component: ethdev
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---

DPDK 18.11.1

In drivers/net/bonding/rte_eth_bond_pmd.c::bond_ethdev_tx_burst_8023ad, bufs_slave_port_idxs array is defined as follows (lines 1293-1294):

    /* Mapping array generated by hash function to map mbufs to slaves */
    uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 };

Array dimension should be equal to number of packets being transmitted (nb_pkts) - as correctly implemented in rte_eth_bond_pmd.c::bond_ethdev_tx_burst_balance.

Invalid array dimension causes overflow when number of transmitted packets is
greater than RTE_MAX_ETHPORTS. Some areas of memory end up overwritten (in my
particular case, slave_nb_bufs array), which leads to SIGSEGV and crash.

To work around the issue, ensure that number of packets transmitted in a single
burst is no greater than RTE_MAX_ETHPORTS.

Quoted 6 lines

To fix it, it should be sufficient to define bufs_slave_port_idxs as a variable-length array, as in bond_ethdev_tx_burst_balance:

    /* Mapping array generated by hash function to map mbufs to slaves */
    uint16_t bufs_slave_port_idxs[nb_bufs];

I have a series of fixes for this, and on the rx parts as well but did not have time to properly check them.
Would you have some time to test it if I send it ?

--
David Marchand

Hello David,

I can merge these fixes and run our system using patched DPDK version. Then report back any problems / improvements I observe. I can also walk through source code changes and check for anything questionable.

Unfortunately I lack deep enough understanding of DPDK source code to test these fixes comprehensively and I have been short on time recently as well, so that's the best I can do right now.

Quoted 10 lines · Przemysław Ołtarzewski

Hello David,

I can merge these fixes and run our system using patched DPDK version. Then report back any problems / improvements I observe. I can also walk through source code changes and check for anything questionable.

Unfortunately I lack deep enough understanding of DPDK source code to test these fixes comprehensively and I have been short on time recently as well, so that's the best I can do right now.

Please, be aware that other fixes have been already merged in master since 18.11.
https://git.dpdk.org/dpdk/log/drivers/net/bonding

I just sent the patchset that should fix your issue:
http://patchwork.dpdk.org/project/dpdk/list/?series=4240

--
David Marchand

Hello,

Last week I attempted to migrate our system from DPDK 18.11.1 to current master with David's patches applied. I haven't been able to make it work correctly - probably due to other changes in master since 18.11.1 - so unfortunately I cannot confirm that David's patchset solves the issue (although from source code analysis it seems that array dimension has been fixed).

I have already spent considerable time migrating from 17.11.5 to 18.11.1 and for the time being I need to focus on diagnosing the original issue in our system that led to discovery of invalid array dimension. I will try again when I have some more time.

Best Regards,
Przemysław Ołtarzewski