This is a note to let you know that I've just added the patch titled
bonding: move dev_mc_sync after master_upper_dev_link in bond_enslave
to the 4.14-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: bonding-move-dev_mc_sync-after-master_upper_dev_link-in-bond_enslave.patch and it can be found in the queue-4.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Tue Apr 10 23:20:08 CEST 2018
From: Xin Long lucien.xin@gmail.com Date: Mon, 26 Mar 2018 01:16:46 +0800 Subject: bonding: move dev_mc_sync after master_upper_dev_link in bond_enslave
From: Xin Long lucien.xin@gmail.com
[ Upstream commit ae42cc62a9f07f1f6979054ed92606b9c30f4a2e ]
Beniamino found a crash when adding vlan as slave of bond which is also the parent link:
ip link add bond1 type bond ip link set bond1 up ip link add link bond1 vlan1 type vlan id 80 ip link set vlan1 master bond1
The call trace is as below:
[<ffffffffa850842a>] queued_spin_lock_slowpath+0xb/0xf [<ffffffffa8515680>] _raw_spin_lock+0x20/0x30 [<ffffffffa83f6f07>] dev_mc_sync+0x37/0x80 [<ffffffffc08687dc>] vlan_dev_set_rx_mode+0x1c/0x30 [8021q] [<ffffffffa83efd2a>] __dev_set_rx_mode+0x5a/0xa0 [<ffffffffa83f7138>] dev_mc_sync_multiple+0x78/0x80 [<ffffffffc084127c>] bond_enslave+0x67c/0x1190 [bonding] [<ffffffffa8401909>] do_setlink+0x9c9/0xe50 [<ffffffffa8403bf2>] rtnl_newlink+0x522/0x880 [<ffffffffa8403ff7>] rtnetlink_rcv_msg+0xa7/0x260 [<ffffffffa8424ecb>] netlink_rcv_skb+0xab/0xc0 [<ffffffffa83fe498>] rtnetlink_rcv+0x28/0x30 [<ffffffffa8424850>] netlink_unicast+0x170/0x210 [<ffffffffa8424bf8>] netlink_sendmsg+0x308/0x420 [<ffffffffa83cc396>] sock_sendmsg+0xb6/0xf0
This is actually a dead lock caused by sync slave hwaddr from master when the master is the slave's 'slave'. This dead loop check is actually done by netdev_master_upper_dev_link. However, Commit 1f718f0f4f97 ("bonding: populate neighbour's private on enslave") moved it after dev_mc_sync.
This patch is to fix it by moving dev_mc_sync after master_upper_dev_link, so that this loop check would be earlier than dev_mc_sync. It also moves if (mode == BOND_MODE_8023AD) into if (!bond_uses_primary) clause as an improvement.
Note team driver also has this issue, I will fix it in another patch.
Fixes: 1f718f0f4f97 ("bonding: populate neighbour's private on enslave") Reported-by: Beniamino Galvani bgalvani@redhat.com Signed-off-by: Xin Long lucien.xin@gmail.com Acked-by: Andy Gospodarek andy@greyhouse.net Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/net/bonding/bond_main.c | 73 +++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 38 deletions(-)
--- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1524,44 +1524,11 @@ int bond_enslave(struct net_device *bond goto err_close; }
- /* If the mode uses primary, then the following is handled by - * bond_change_active_slave(). - */ - if (!bond_uses_primary(bond)) { - /* set promiscuity level to new slave */ - if (bond_dev->flags & IFF_PROMISC) { - res = dev_set_promiscuity(slave_dev, 1); - if (res) - goto err_close; - } - - /* set allmulti level to new slave */ - if (bond_dev->flags & IFF_ALLMULTI) { - res = dev_set_allmulti(slave_dev, 1); - if (res) - goto err_close; - } - - netif_addr_lock_bh(bond_dev); - - dev_mc_sync_multiple(slave_dev, bond_dev); - dev_uc_sync_multiple(slave_dev, bond_dev); - - netif_addr_unlock_bh(bond_dev); - } - - if (BOND_MODE(bond) == BOND_MODE_8023AD) { - /* add lacpdu mc addr to mc list */ - u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR; - - dev_mc_add(slave_dev, lacpdu_multicast); - } - res = vlan_vids_add_by_dev(slave_dev, bond_dev); if (res) { netdev_err(bond_dev, "Couldn't add bond vlan ids to %s\n", slave_dev->name); - goto err_hwaddr_unsync; + goto err_close; }
prev_slave = bond_last_slave(bond); @@ -1721,6 +1688,37 @@ int bond_enslave(struct net_device *bond goto err_upper_unlink; }
+ /* If the mode uses primary, then the following is handled by + * bond_change_active_slave(). + */ + if (!bond_uses_primary(bond)) { + /* set promiscuity level to new slave */ + if (bond_dev->flags & IFF_PROMISC) { + res = dev_set_promiscuity(slave_dev, 1); + if (res) + goto err_sysfs_del; + } + + /* set allmulti level to new slave */ + if (bond_dev->flags & IFF_ALLMULTI) { + res = dev_set_allmulti(slave_dev, 1); + if (res) + goto err_sysfs_del; + } + + netif_addr_lock_bh(bond_dev); + dev_mc_sync_multiple(slave_dev, bond_dev); + dev_uc_sync_multiple(slave_dev, bond_dev); + netif_addr_unlock_bh(bond_dev); + + if (BOND_MODE(bond) == BOND_MODE_8023AD) { + /* add lacpdu mc addr to mc list */ + u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR; + + dev_mc_add(slave_dev, lacpdu_multicast); + } + } + bond->slave_cnt++; bond_compute_features(bond); bond_set_carrier(bond); @@ -1744,6 +1742,9 @@ int bond_enslave(struct net_device *bond return 0;
/* Undo stages on error */ +err_sysfs_del: + bond_sysfs_slave_del(new_slave); + err_upper_unlink: bond_upper_dev_unlink(bond, new_slave);
@@ -1764,10 +1765,6 @@ err_detach: synchronize_rcu(); slave_disable_netpoll(new_slave);
-err_hwaddr_unsync: - if (!bond_uses_primary(bond)) - bond_hw_addr_flush(bond_dev, slave_dev); - err_close: slave_dev->priv_flags &= ~IFF_BONDING; dev_close(slave_dev);
Patches currently in stable-queue which might be from lucien.xin@gmail.com are
queue-4.14/team-move-dev_mc_sync-after-master_upper_dev_link-in-team_port_add.patch queue-4.14/bonding-process-the-err-returned-by-dev_set_allmulti-properly-in-bond_enslave.patch queue-4.14/bonding-fix-the-err-path-for-dev-hwaddr-sync-in-bond_enslave.patch queue-4.14/bonding-move-dev_mc_sync-after-master_upper_dev_link-in-bond_enslave.patch queue-4.14/route-check-sysctl_fib_multipath_use_neigh-earlier-than-hash.patch