net/bonding: fix add bonded device itself as its slave
Checks
Commit Message
From: Chengchang Tang <tangchengchang@huawei.com>
Adding the bond device as its own slave should be forbidden. This
will cause a recursive endless loop in many subsequent operations,
and eventually lead to coredump.
This problem was found in testpmd, the related logs are as follows:
testpmd> create bonded device 1 0
Created new bonded device net_bonding_testpmd_0 on (port 4).
testpmd> add bonding slave 4 4
Segmentation fault (core dumped)
The call stack is as follows:
0x000000000064eb90 in rte_eth_dev_info_get ()
0x00000000006df4b4 in bond_ethdev_info ()
0x000000000064eb90 in rte_eth_dev_info_get ()
0x00000000006df4b4 in bond_ethdev_info ()
0x000000000064eb90 in rte_eth_dev_info_get ()
0x0000000000564e58 in eth_dev_info_get_print_err ()
0x000000000055e8a4 in init_port_config ()
0x000000000052730c in cmd_add_bonding_slave_parsed ()
0x0000000000646f60 in cmdline_parse ()
0x0000000000645e08 in cmdline_valid_buffer ()
0x000000000064956c in rdline_char_in ()
0x0000000000645ee0 in cmdline_in ()
0x00000000006460a4 in cmdline_interact ()
0x0000000000531904 in prompt ()
0x000000000051cca8 in main ()
Fixes: 2efb58cbab6e ("bond: new link bonding library")
Cc: stable@dpdk.org
Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
drivers/net/bonding/eth_bond_private.h | 2 +-
drivers/net/bonding/rte_eth_bond_api.c | 26 +++++++++++++++++---------
2 files changed, 18 insertions(+), 10 deletions(-)
Comments
On 4/15/2021 8:09 AM, Min Hu (Connor) wrote:
> From: Chengchang Tang <tangchengchang@huawei.com>
>
> Adding the bond device as its own slave should be forbidden. This
> will cause a recursive endless loop in many subsequent operations,
> and eventually lead to coredump.
>
> This problem was found in testpmd, the related logs are as follows:
> testpmd> create bonded device 1 0
> Created new bonded device net_bonding_testpmd_0 on (port 4).
> testpmd> add bonding slave 4 4
> Segmentation fault (core dumped)
>
> The call stack is as follows:
> 0x000000000064eb90 in rte_eth_dev_info_get ()
> 0x00000000006df4b4 in bond_ethdev_info ()
> 0x000000000064eb90 in rte_eth_dev_info_get ()
> 0x00000000006df4b4 in bond_ethdev_info ()
> 0x000000000064eb90 in rte_eth_dev_info_get ()
> 0x0000000000564e58 in eth_dev_info_get_print_err ()
> 0x000000000055e8a4 in init_port_config ()
> 0x000000000052730c in cmd_add_bonding_slave_parsed ()
> 0x0000000000646f60 in cmdline_parse ()
> 0x0000000000645e08 in cmdline_valid_buffer ()
> 0x000000000064956c in rdline_char_in ()
> 0x0000000000645ee0 in cmdline_in ()
> 0x00000000006460a4 in cmdline_interact ()
> 0x0000000000531904 in prompt ()
> 0x000000000051cca8 in main ()
>
> Fixes: 2efb58cbab6e ("bond: new link bonding library")
> Cc: stable@dpdk.org
>
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Applied to dpdk-next-net/main, thanks.
@@ -212,7 +212,7 @@ int
valid_bonded_port_id(uint16_t port_id);
int
-valid_slave_port_id(uint16_t port_id, uint8_t mode);
+valid_slave_port_id(struct bond_dev_private *internals, uint16_t port_id);
void
deactivate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id);
@@ -56,19 +56,25 @@ check_for_master_bonded_ethdev(const struct rte_eth_dev *eth_dev)
}
int
-valid_slave_port_id(uint16_t port_id, uint8_t mode)
+valid_slave_port_id(struct bond_dev_private *internals, uint16_t slave_port_id)
{
- RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(slave_port_id, -1);
- /* Verify that port_id refers to a non bonded port */
- if (check_for_bonded_ethdev(&rte_eth_devices[port_id]) == 0 &&
- mode == BONDING_MODE_8023AD) {
+ /* Verify that slave_port_id refers to a non bonded port */
+ if (check_for_bonded_ethdev(&rte_eth_devices[slave_port_id]) == 0 &&
+ internals->mode == BONDING_MODE_8023AD) {
RTE_BOND_LOG(ERR, "Cannot add slave to bonded device in 802.3ad"
" mode as slave is also a bonded device, only "
"physical devices can be support in this mode.");
return -1;
}
+ if (internals->port_id == slave_port_id) {
+ RTE_BOND_LOG(ERR,
+ "Cannot add the bonded device itself as its slave.");
+ return -1;
+ }
+
return 0;
}
@@ -456,7 +462,7 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
bonded_eth_dev = &rte_eth_devices[bonded_port_id];
internals = bonded_eth_dev->data->dev_private;
- if (valid_slave_port_id(slave_port_id, internals->mode) != 0)
+ if (valid_slave_port_id(internals, slave_port_id) != 0)
return -1;
slave_eth_dev = &rte_eth_devices[slave_port_id];
@@ -605,13 +611,15 @@ rte_eth_bond_slave_add(uint16_t bonded_port_id, uint16_t slave_port_id)
int retval;
- /* Verify that port id's are valid bonded and slave ports */
if (valid_bonded_port_id(bonded_port_id) != 0)
return -1;
bonded_eth_dev = &rte_eth_devices[bonded_port_id];
internals = bonded_eth_dev->data->dev_private;
+ if (valid_slave_port_id(internals, slave_port_id) != 0)
+ return -1;
+
rte_spinlock_lock(&internals->lock);
retval = __eth_bond_slave_add_lock_free(bonded_port_id, slave_port_id);
@@ -635,7 +643,7 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
bonded_eth_dev = &rte_eth_devices[bonded_port_id];
internals = bonded_eth_dev->data->dev_private;
- if (valid_slave_port_id(slave_port_id, internals->mode) < 0)
+ if (valid_slave_port_id(internals, slave_port_id) < 0)
return -1;
/* first remove from active slave list */
@@ -783,7 +791,7 @@ rte_eth_bond_primary_set(uint16_t bonded_port_id, uint16_t slave_port_id)
internals = rte_eth_devices[bonded_port_id].data->dev_private;
- if (valid_slave_port_id(slave_port_id, internals->mode) != 0)
+ if (valid_slave_port_id(internals, slave_port_id) != 0)
return -1;
internals->user_defined_primary_port = 1;