[PATCH] bonding: 3ad: fix a crash in agg_device_up()
From: zhudi <hidden>
Date: 2021-06-02 12:45:08
From: Di Zhu <redacted>
When doing the test of restarting the network card, the system is
broken because the port->slave is null pointer in agg_device_up().
After in-depth investigation, we found the real cause: in
bond_3ad_unbind_slave() if there are no active ports in the
aggregator to be deleted, the ad_clear_agg() will be called to
set "aggregator->lag_ports = NULL", but the ports originally
belonging to the aggregator are still linked together.
Before bond_3ad_unbind_slave():
aggregator4->lag_ports = port1->port2->port3
After bond_3ad_unbind_slave():
aggregator4->lag_ports = NULL
port1->port2->port3
After the port2 is deleted, the port is still remain in the linked
list: because the port does not belong to any agg, so unbind do
nothing for this port.
After a while, bond_3ad_state_machine_handler() will run and
traverse each existing port, trying to bind each port to the
newly selected agg, such as:
if (!found) {
if (free_aggregator) {
...
port->aggregator->lag_ports = port;
...
}
}
After this operation, the link list looks like this:
aggregator1->lag_ports = port1->port2(has been deleted)->port3
After that, just traverse the linked list of agg1 and access the
port2->slave, the crash will happen.
The easiest way to fix it is: if a port does not belong to any agg, delete
it from the list and wait for the state machine to select the agg again
Signed-off-by: Di Zhu <redacted>
---
drivers/net/bonding/bond_3ad.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 6908822d9773..1d6ff4e1ed28 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c@@ -1793,6 +1793,8 @@ static void ad_agg_selection_logic(struct aggregator *agg, static void ad_clear_agg(struct aggregator *aggregator) { if (aggregator) { + struct port *port, *next; + aggregator->is_individual = false; aggregator->actor_admin_aggregator_key = 0; aggregator->actor_oper_aggregator_key = 0;
@@ -1801,6 +1803,11 @@ static void ad_clear_agg(struct aggregator *aggregator) aggregator->partner_oper_aggregator_key = 0; aggregator->receive_state = 0; aggregator->transmit_state = 0; + for (port = aggregator->lag_ports; port; port = next) { + next = port->next_port_in_aggregator; + if (port->aggregator == aggregator) + port->next_port_in_aggregator = NULL; + } aggregator->lag_ports = NULL; aggregator->is_active = 0; aggregator->num_of_ports = 0;
--
2.23.0