]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
net: bonding: don't recurse on the slave's netdev ops lock
authorJakub Kicinski <kuba@kernel.org>
Wed, 3 Jun 2026 01:28:34 +0000 (18:28 -0700)
committerJakub Kicinski <kuba@kernel.org>
Thu, 4 Jun 2026 21:04:55 +0000 (14:04 -0700)
bond_update_speed_duplex() calls __ethtool_get_link_ksettings() on
the slave, which will soon take the slave's ops lock. One of its
callers already holds it and the other three don't, so the function
would either deadlock or run unprotected depending on the path.

Make the helper expect the slave's ops lock held and switch to
netif_get_link_ksettings(). Wrap the three call sites that don't
already hold it:

  * bond_enslave() (rtnl held; core drops the lower's ops lock
    around ->ndo_add_slave).
  * bond_miimon_commit() (rtnl_trylock'd from the mii workqueue).
  * bond_ethtool_get_link_ksettings() (rtnl held via ethtool layer,
    bond device itself is not ops locked).

The call site which does already hold the ops lock is
bond_slave_netdev_event() via NETDEV_UP / NETDEV_CHANGE notifiers,
so it stays as-is.

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Link: https://patch.msgid.link/20260603012840.2254293-6-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/bonding/bond_main.c

index 82e779f7916b56db4349905e7171d140d1f68862..0bcab797e468d2f23af3d87f624e977d4171f299 100644 (file)
@@ -787,6 +787,10 @@ down:
  * values are invalid, set speed and duplex to -1,
  * and return. Return 1 if speed or duplex settings are
  * UNKNOWN; 0 otherwise.
+ *
+ * Caller must hold the slave's netdev ops lock. The notifier path
+ * (bond_netdev_event NETDEV_CHANGE/UP) reaches us with the slave's ops
+ * lock held; other call sites take it explicitly.
  */
 static int bond_update_speed_duplex(struct slave *slave)
 {
@@ -794,7 +798,7 @@ static int bond_update_speed_duplex(struct slave *slave)
        struct ethtool_link_ksettings ecmd;
        int res;
 
-       res = __ethtool_get_link_ksettings(slave_dev, &ecmd);
+       res = netif_get_link_ksettings(slave_dev, &ecmd);
        if (res < 0)
                goto speed_duplex_unknown;
        if (ecmd.base.speed == 0 || ecmd.base.speed == ((__u32)-1))
@@ -2112,8 +2116,10 @@ skip_mac_set:
        new_slave->delay = 0;
        new_slave->link_failure_count = 0;
 
-       if (bond_update_speed_duplex(new_slave) &&
-           bond_needs_speed_duplex(bond))
+       netdev_lock_ops(slave_dev);
+       res = bond_update_speed_duplex(new_slave);
+       netdev_unlock_ops(slave_dev);
+       if (res && bond_needs_speed_duplex(bond))
                new_slave->link = BOND_LINK_DOWN;
 
        new_slave->last_rx = jiffies -
@@ -2780,6 +2786,7 @@ static void bond_miimon_commit(struct bonding *bond)
        struct slave *slave, *primary, *active;
        bool do_failover = false;
        struct list_head *iter;
+       int err;
 
        ASSERT_RTNL();
 
@@ -2798,8 +2805,10 @@ static void bond_miimon_commit(struct bonding *bond)
                        continue;
 
                case BOND_LINK_UP:
-                       if (bond_update_speed_duplex(slave) &&
-                           bond_needs_speed_duplex(bond)) {
+                       netdev_lock_ops(slave->dev);
+                       err = bond_update_speed_duplex(slave);
+                       netdev_unlock_ops(slave->dev);
+                       if (err && bond_needs_speed_duplex(bond)) {
                                slave->link = BOND_LINK_DOWN;
                                if (net_ratelimit())
                                        slave_warn(bond->dev, slave->dev,
@@ -5861,7 +5870,9 @@ static int bond_ethtool_get_link_ksettings(struct net_device *bond_dev,
         */
        bond_for_each_slave(bond, slave, iter) {
                if (bond_slave_can_tx(slave)) {
+                       netdev_lock_ops(slave->dev);
                        bond_update_speed_duplex(slave);
+                       netdev_unlock_ops(slave->dev);
                        if (slave->speed != SPEED_UNKNOWN) {
                                if (BOND_MODE(bond) == BOND_MODE_BROADCAST)
                                        speed = bond_mode_bcast_speed(slave,