team: replace team lock with rtnl lock

syszbot reports various ordering issues for lower instance locks and
team lock. Switch to using rtnl lock for protecting team device,
similar to bonding. Based on the patch by Tetsuo Handa.

Cc: Jiri Pirko <jiri@resnulli.us>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reported-by: syzbot+705c61d60b091ef42c04@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=705c61d60b091ef42c04
Reported-by: syzbot+71fd22ae4b81631e22fd@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=71fd22ae4b81631e22fd
Fixes: 6b1d3c5f675c ("team: grab team lock during team_change_rx_flags")
Link: https://lkml.kernel.org/r/ZoZ2RH9BcahEB9Sb@nanopsycho.orion
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20250623153147.3413631-1-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index 8bc56186b2a3e9889e4332e359f09e357fc328de..17f07eb0ee52a625c77c485c0543a737f4f39d25 100644 (file)
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -933,7 +933,7 @@ static bool team_port_find(const struct team *team,
  * Enable/disable port by adding to enabled port hashlist and setting
  * port->index (Might be racy so reader could see incorrect ifindex when
  * processing a flying packet, but that is not a problem). Write guarded
- * by team->lock.
+ * by RTNL.
  */
 static void team_port_enable(struct team *team,
                             struct team_port *port)
@@ -1660,8 +1660,6 @@ static int team_init(struct net_device *dev)
                goto err_options_register;
        netif_carrier_off(dev);
 
-       lockdep_register_key(&team->team_lock_key);
-       __mutex_init(&team->lock, "team->team_lock_key", &team->team_lock_key);
        netdev_lockdep_set_classes(dev);
 
        return 0;
@@ -1682,7 +1680,8 @@ static void team_uninit(struct net_device *dev)
        struct team_port *port;
        struct team_port *tmp;
 
-       mutex_lock(&team->lock);
+       ASSERT_RTNL();
+
        list_for_each_entry_safe(port, tmp, &team->port_list, list)
                team_port_del(team, port->dev);
 
@@ -1691,9 +1690,7 @@ static void team_uninit(struct net_device *dev)
        team_mcast_rejoin_fini(team);
        team_notify_peers_fini(team);
        team_queue_override_fini(team);
-       mutex_unlock(&team->lock);
        netdev_change_features(dev);
-       lockdep_unregister_key(&team->team_lock_key);
 }
 
 static void team_destructor(struct net_device *dev)
@@ -1778,7 +1775,8 @@ static void team_change_rx_flags(struct net_device *dev, int change)
        struct team_port *port;
        int inc;
 
-       mutex_lock(&team->lock);
+       ASSERT_RTNL();
+
        list_for_each_entry(port, &team->port_list, list) {
                if (change & IFF_PROMISC) {
                        inc = dev->flags & IFF_PROMISC ? 1 : -1;
@@ -1789,7 +1787,6 @@ static void team_change_rx_flags(struct net_device *dev, int change)
                        dev_set_allmulti(port->dev, inc);
                }
        }
-       mutex_unlock(&team->lock);
 }
 
 static void team_set_rx_mode(struct net_device *dev)
@@ -1811,14 +1808,14 @@ static int team_set_mac_address(struct net_device *dev, void *p)
        struct team *team = netdev_priv(dev);
        struct team_port *port;
 
+       ASSERT_RTNL();
+
        if (dev->type == ARPHRD_ETHER && !is_valid_ether_addr(addr->sa_data))
                return -EADDRNOTAVAIL;
        dev_addr_set(dev, addr->sa_data);
-       mutex_lock(&team->lock);
        list_for_each_entry(port, &team->port_list, list)
                if (team->ops.port_change_dev_addr)
                        team->ops.port_change_dev_addr(team, port);
-       mutex_unlock(&team->lock);
        return 0;
 }
 
@@ -1828,11 +1825,8 @@ static int team_change_mtu(struct net_device *dev, int new_mtu)
        struct team_port *port;
        int err;
 
-       /*
-        * Alhough this is reader, it's guarded by team lock. It's not possible
-        * to traverse list in reverse under rcu_read_lock
-        */
-       mutex_lock(&team->lock);
+       ASSERT_RTNL();
+
        team->port_mtu_change_allowed = true;
        list_for_each_entry(port, &team->port_list, list) {
                err = dev_set_mtu(port->dev, new_mtu);
@@ -1843,7 +1837,6 @@ static int team_change_mtu(struct net_device *dev, int new_mtu)
                }
        }
        team->port_mtu_change_allowed = false;
-       mutex_unlock(&team->lock);
 
        WRITE_ONCE(dev->mtu, new_mtu);
 
@@ -1853,7 +1846,6 @@ unwind:
        list_for_each_entry_continue_reverse(port, &team->port_list, list)
                dev_set_mtu(port->dev, dev->mtu);
        team->port_mtu_change_allowed = false;
-       mutex_unlock(&team->lock);
 
        return err;
 }
@@ -1903,24 +1895,19 @@ static int team_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
        struct team_port *port;
        int err;
 
-       /*
-        * Alhough this is reader, it's guarded by team lock. It's not possible
-        * to traverse list in reverse under rcu_read_lock
-        */
-       mutex_lock(&team->lock);
+       ASSERT_RTNL();
+
        list_for_each_entry(port, &team->port_list, list) {
                err = vlan_vid_add(port->dev, proto, vid);
                if (err)
                        goto unwind;
        }
-       mutex_unlock(&team->lock);
 
        return 0;
 
 unwind:
        list_for_each_entry_continue_reverse(port, &team->port_list, list)
                vlan_vid_del(port->dev, proto, vid);
-       mutex_unlock(&team->lock);
 
        return err;
 }
@@ -1930,10 +1917,10 @@ static int team_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
        struct team *team = netdev_priv(dev);
        struct team_port *port;
 
-       mutex_lock(&team->lock);
+       ASSERT_RTNL();
+
        list_for_each_entry(port, &team->port_list, list)
                vlan_vid_del(port->dev, proto, vid);
-       mutex_unlock(&team->lock);
 
        return 0;
 }
@@ -1955,9 +1942,9 @@ static void team_netpoll_cleanup(struct net_device *dev)
 {
        struct team *team = netdev_priv(dev);
 
-       mutex_lock(&team->lock);
+       ASSERT_RTNL();
+
        __team_netpoll_cleanup(team);
-       mutex_unlock(&team->lock);
 }
 
 static int team_netpoll_setup(struct net_device *dev)
@@ -1966,7 +1953,8 @@ static int team_netpoll_setup(struct net_device *dev)
        struct team_port *port;
        int err = 0;
 
-       mutex_lock(&team->lock);
+       ASSERT_RTNL();
+
        list_for_each_entry(port, &team->port_list, list) {
                err = __team_port_enable_netpoll(port);
                if (err) {
@@ -1974,7 +1962,6 @@ static int team_netpoll_setup(struct net_device *dev)
                        break;
                }
        }
-       mutex_unlock(&team->lock);
        return err;
 }
 #endif
@@ -1985,9 +1972,9 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev,
        struct team *team = netdev_priv(dev);
        int err;
 
-       mutex_lock(&team->lock);
+       ASSERT_RTNL();
+
        err = team_port_add(team, port_dev, extack);
-       mutex_unlock(&team->lock);
 
        if (!err)
                netdev_change_features(dev);
@@ -2000,18 +1987,13 @@ static int team_del_slave(struct net_device *dev, struct net_device *port_dev)
        struct team *team = netdev_priv(dev);
        int err;
 
-       mutex_lock(&team->lock);
+       ASSERT_RTNL();
+
        err = team_port_del(team, port_dev);
-       mutex_unlock(&team->lock);
 
        if (err)
                return err;
 
-       if (netif_is_team_master(port_dev)) {
-               lockdep_unregister_key(&team->team_lock_key);
-               lockdep_register_key(&team->team_lock_key);
-               lockdep_set_class(&team->lock, &team->team_lock_key);
-       }
        netdev_change_features(dev);
 
        return err;
@@ -2304,9 +2286,10 @@ err_msg_put:
 static struct team *team_nl_team_get(struct genl_info *info)
 {
        struct net *net = genl_info_net(info);
-       int ifindex;
        struct net_device *dev;
-       struct team *team;
+       int ifindex;
+
+       ASSERT_RTNL();
 
        if (!info->attrs[TEAM_ATTR_TEAM_IFINDEX])
                return NULL;
@@ -2318,14 +2301,11 @@ static struct team *team_nl_team_get(struct genl_info *info)
                return NULL;
        }
 
-       team = netdev_priv(dev);
-       mutex_lock(&team->lock);
-       return team;
+       return netdev_priv(dev);
 }
 
 static void team_nl_team_put(struct team *team)
 {
-       mutex_unlock(&team->lock);
        dev_put(team->dev);
 }
 
@@ -2515,9 +2495,13 @@ int team_nl_options_get_doit(struct sk_buff *skb, struct genl_info *info)
        int err;
        LIST_HEAD(sel_opt_inst_list);
 
+       rtnl_lock();
+
        team = team_nl_team_get(info);
-       if (!team)
-               return -EINVAL;
+       if (!team) {
+               err = -EINVAL;
+               goto rtnl_unlock;
+       }
 
        list_for_each_entry(opt_inst, &team->option_inst_list, list)
                list_add_tail(&opt_inst->tmp_list, &sel_opt_inst_list);
@@ -2527,6 +2511,9 @@ int team_nl_options_get_doit(struct sk_buff *skb, struct genl_info *info)
 
        team_nl_team_put(team);
 
+rtnl_unlock:
+       rtnl_unlock();
+
        return err;
 }
 
@@ -2805,15 +2792,22 @@ int team_nl_port_list_get_doit(struct sk_buff *skb,
        struct team *team;
        int err;
 
+       rtnl_lock();
+
        team = team_nl_team_get(info);
-       if (!team)
-               return -EINVAL;
+       if (!team) {
+               err = -EINVAL;
+               goto rtnl_unlock;
+       }
 
        err = team_nl_send_port_list_get(team, info->snd_portid, info->snd_seq,
                                         NLM_F_ACK, team_nl_send_unicast, NULL);
 
        team_nl_team_put(team);
 
+rtnl_unlock:
+       rtnl_unlock();
+
        return err;
 }
 
@@ -2961,11 +2955,9 @@ static void __team_port_change_port_removed(struct team_port *port)
 
 static void team_port_change_check(struct team_port *port, bool linkup)
 {
-       struct team *team = port->team;
+       ASSERT_RTNL();
 
-       mutex_lock(&team->lock);
        __team_port_change_check(port, linkup);
-       mutex_unlock(&team->lock);
 }
 
 

diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c
index e0f599e2a51dd6da759c9c0489f74bc3c665dcc4..1c3336c7a1b26e02ba11e3246172b1d48023e25e 100644 (file)
--- a/drivers/net/team/team_mode_activebackup.c
+++ b/drivers/net/team/team_mode_activebackup.c
@@ -67,8 +67,7 @@ static void ab_active_port_get(struct team *team, struct team_gsetter_ctx *ctx)
 {
        struct team_port *active_port;
 
-       active_port = rcu_dereference_protected(ab_priv(team)->active_port,
-                                               lockdep_is_held(&team->lock));
+       active_port = rtnl_dereference(ab_priv(team)->active_port);
        if (active_port)
                ctx->data.u32_val = active_port->dev->ifindex;
        else

diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index 00f8989c29c0ff443f37d584356fad7cdac72094..b14538bde2f824c19edb589987f835753862fd4b 100644 (file)
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c
@@ -301,8 +301,7 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx)
        if (lb_priv->ex->orig_fprog) {
                /* Clear old filter data */
                __fprog_destroy(lb_priv->ex->orig_fprog);
-               orig_fp = rcu_dereference_protected(lb_priv->fp,
-                                               lockdep_is_held(&team->lock));
+               orig_fp = rtnl_dereference(lb_priv->fp);
        }
 
        rcu_assign_pointer(lb_priv->fp, fp);
@@ -324,8 +323,7 @@ static void lb_bpf_func_free(struct team *team)
                return;
 
        __fprog_destroy(lb_priv->ex->orig_fprog);
-       fp = rcu_dereference_protected(lb_priv->fp,
-                                      lockdep_is_held(&team->lock));
+       fp = rtnl_dereference(lb_priv->fp);
        bpf_prog_destroy(fp);
 }
 
@@ -335,8 +333,7 @@ static void lb_tx_method_get(struct team *team, struct team_gsetter_ctx *ctx)
        lb_select_tx_port_func_t *func;
        char *name;
 
-       func = rcu_dereference_protected(lb_priv->select_tx_port_func,
-                                        lockdep_is_held(&team->lock));
+       func = rtnl_dereference(lb_priv->select_tx_port_func);
        name = lb_select_tx_port_get_name(func);
        BUG_ON(!name);
        ctx->data.str_val = name;
@@ -478,7 +475,7 @@ static void lb_stats_refresh(struct work_struct *work)
        team = lb_priv_ex->team;
        lb_priv = get_lb_priv(team);
 
-       if (!mutex_trylock(&team->lock)) {
+       if (!rtnl_trylock()) {
                schedule_delayed_work(&lb_priv_ex->stats.refresh_dw, 0);
                return;
        }
@@ -515,7 +512,7 @@ static void lb_stats_refresh(struct work_struct *work)
        schedule_delayed_work(&lb_priv_ex->stats.refresh_dw,
                              (lb_priv_ex->stats.refresh_interval * HZ) / 10);
 
-       mutex_unlock(&team->lock);
+       rtnl_unlock();
 }
 
 static void lb_stats_refresh_interval_get(struct team *team,

diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index cdc684e04a2fb6fcabda07a3427270af79b7eca8..ce97d891cf720f58b74fc754c864daadbc0474c7 100644 (file)
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -191,8 +191,6 @@ struct team {
 
        const struct header_ops *header_ops_cache;
 
-       struct mutex lock; /* used for overall locking, e.g. port lists write */
-
        /*
         * List of enabled ports and their count
         */
@@ -223,7 +221,6 @@ struct team {
                atomic_t count_pending;
                struct delayed_work dw;
        } mcast_rejoin;
-       struct lock_class_key team_lock_key;
        long mode_priv[TEAM_MODE_PRIV_LONGS];
 };