All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.