* [PATCH] bonding: 3ad: fix a crash in agg_device_up()
@ 2021-06-02 12:44 zhudi
2021-06-03 22:12 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: zhudi @ 2021-06-02 12:44 UTC (permalink / raw)
To: j.vosburgh, vfalico, kuba, davem; +Cc: netdev, zhudi21, rose.chen
From: Di Zhu <zhudi21@huawei.com>
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 <zhudi21@huawei.com>
---
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] bonding: 3ad: fix a crash in agg_device_up()
2021-06-02 12:44 [PATCH] bonding: 3ad: fix a crash in agg_device_up() zhudi
@ 2021-06-03 22:12 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2021-06-03 22:12 UTC (permalink / raw)
To: zhudi21; +Cc: j.vosburgh, vfalico, kuba, netdev, rose.chen
Please prep[ost with an appropriate Fixes: tag, thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-06-03 22:12 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02 12:44 [PATCH] bonding: 3ad: fix a crash in agg_device_up() zhudi
2021-06-03 22:12 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.