Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Stanislav Fomichev and committed by
Jakub Kicinski
bfb4fb77 ab4eb6a2

+50 -65
+44 -52
drivers/net/team/team_core.c
··· 933 933 * Enable/disable port by adding to enabled port hashlist and setting 934 934 * port->index (Might be racy so reader could see incorrect ifindex when 935 935 * processing a flying packet, but that is not a problem). Write guarded 936 - * by team->lock. 936 + * by RTNL. 937 937 */ 938 938 static void team_port_enable(struct team *team, 939 939 struct team_port *port) ··· 1660 1660 goto err_options_register; 1661 1661 netif_carrier_off(dev); 1662 1662 1663 - lockdep_register_key(&team->team_lock_key); 1664 - __mutex_init(&team->lock, "team->team_lock_key", &team->team_lock_key); 1665 1663 netdev_lockdep_set_classes(dev); 1666 1664 1667 1665 return 0; ··· 1680 1682 struct team_port *port; 1681 1683 struct team_port *tmp; 1682 1684 1683 - mutex_lock(&team->lock); 1685 + ASSERT_RTNL(); 1686 + 1684 1687 list_for_each_entry_safe(port, tmp, &team->port_list, list) 1685 1688 team_port_del(team, port->dev); 1686 1689 ··· 1690 1691 team_mcast_rejoin_fini(team); 1691 1692 team_notify_peers_fini(team); 1692 1693 team_queue_override_fini(team); 1693 - mutex_unlock(&team->lock); 1694 1694 netdev_change_features(dev); 1695 - lockdep_unregister_key(&team->team_lock_key); 1696 1695 } 1697 1696 1698 1697 static void team_destructor(struct net_device *dev) ··· 1775 1778 struct team_port *port; 1776 1779 int inc; 1777 1780 1778 - mutex_lock(&team->lock); 1781 + ASSERT_RTNL(); 1782 + 1779 1783 list_for_each_entry(port, &team->port_list, list) { 1780 1784 if (change & IFF_PROMISC) { 1781 1785 inc = dev->flags & IFF_PROMISC ? 1 : -1; ··· 1787 1789 dev_set_allmulti(port->dev, inc); 1788 1790 } 1789 1791 } 1790 - mutex_unlock(&team->lock); 1791 1792 } 1792 1793 1793 1794 static void team_set_rx_mode(struct net_device *dev) ··· 1808 1811 struct team *team = netdev_priv(dev); 1809 1812 struct team_port *port; 1810 1813 1814 + ASSERT_RTNL(); 1815 + 1811 1816 if (dev->type == ARPHRD_ETHER && !is_valid_ether_addr(addr->sa_data)) 1812 1817 return -EADDRNOTAVAIL; 1813 1818 dev_addr_set(dev, addr->sa_data); 1814 - mutex_lock(&team->lock); 1815 1819 list_for_each_entry(port, &team->port_list, list) 1816 1820 if (team->ops.port_change_dev_addr) 1817 1821 team->ops.port_change_dev_addr(team, port); 1818 - mutex_unlock(&team->lock); 1819 1822 return 0; 1820 1823 } 1821 1824 ··· 1825 1828 struct team_port *port; 1826 1829 int err; 1827 1830 1828 - /* 1829 - * Alhough this is reader, it's guarded by team lock. It's not possible 1830 - * to traverse list in reverse under rcu_read_lock 1831 - */ 1832 - mutex_lock(&team->lock); 1831 + ASSERT_RTNL(); 1832 + 1833 1833 team->port_mtu_change_allowed = true; 1834 1834 list_for_each_entry(port, &team->port_list, list) { 1835 1835 err = dev_set_mtu(port->dev, new_mtu); ··· 1837 1843 } 1838 1844 } 1839 1845 team->port_mtu_change_allowed = false; 1840 - mutex_unlock(&team->lock); 1841 1846 1842 1847 WRITE_ONCE(dev->mtu, new_mtu); 1843 1848 ··· 1846 1853 list_for_each_entry_continue_reverse(port, &team->port_list, list) 1847 1854 dev_set_mtu(port->dev, dev->mtu); 1848 1855 team->port_mtu_change_allowed = false; 1849 - mutex_unlock(&team->lock); 1850 1856 1851 1857 return err; 1852 1858 } ··· 1895 1903 struct team_port *port; 1896 1904 int err; 1897 1905 1898 - /* 1899 - * Alhough this is reader, it's guarded by team lock. It's not possible 1900 - * to traverse list in reverse under rcu_read_lock 1901 - */ 1902 - mutex_lock(&team->lock); 1906 + ASSERT_RTNL(); 1907 + 1903 1908 list_for_each_entry(port, &team->port_list, list) { 1904 1909 err = vlan_vid_add(port->dev, proto, vid); 1905 1910 if (err) 1906 1911 goto unwind; 1907 1912 } 1908 - mutex_unlock(&team->lock); 1909 1913 1910 1914 return 0; 1911 1915 1912 1916 unwind: 1913 1917 list_for_each_entry_continue_reverse(port, &team->port_list, list) 1914 1918 vlan_vid_del(port->dev, proto, vid); 1915 - mutex_unlock(&team->lock); 1916 1919 1917 1920 return err; 1918 1921 } ··· 1917 1930 struct team *team = netdev_priv(dev); 1918 1931 struct team_port *port; 1919 1932 1920 - mutex_lock(&team->lock); 1933 + ASSERT_RTNL(); 1934 + 1921 1935 list_for_each_entry(port, &team->port_list, list) 1922 1936 vlan_vid_del(port->dev, proto, vid); 1923 - mutex_unlock(&team->lock); 1924 1937 1925 1938 return 0; 1926 1939 } ··· 1942 1955 { 1943 1956 struct team *team = netdev_priv(dev); 1944 1957 1945 - mutex_lock(&team->lock); 1958 + ASSERT_RTNL(); 1959 + 1946 1960 __team_netpoll_cleanup(team); 1947 - mutex_unlock(&team->lock); 1948 1961 } 1949 1962 1950 1963 static int team_netpoll_setup(struct net_device *dev) ··· 1953 1966 struct team_port *port; 1954 1967 int err = 0; 1955 1968 1956 - mutex_lock(&team->lock); 1969 + ASSERT_RTNL(); 1970 + 1957 1971 list_for_each_entry(port, &team->port_list, list) { 1958 1972 err = __team_port_enable_netpoll(port); 1959 1973 if (err) { ··· 1962 1974 break; 1963 1975 } 1964 1976 } 1965 - mutex_unlock(&team->lock); 1966 1977 return err; 1967 1978 } 1968 1979 #endif ··· 1972 1985 struct team *team = netdev_priv(dev); 1973 1986 int err; 1974 1987 1975 - mutex_lock(&team->lock); 1988 + ASSERT_RTNL(); 1989 + 1976 1990 err = team_port_add(team, port_dev, extack); 1977 - mutex_unlock(&team->lock); 1978 1991 1979 1992 if (!err) 1980 1993 netdev_change_features(dev); ··· 1987 2000 struct team *team = netdev_priv(dev); 1988 2001 int err; 1989 2002 1990 - mutex_lock(&team->lock); 2003 + ASSERT_RTNL(); 2004 + 1991 2005 err = team_port_del(team, port_dev); 1992 - mutex_unlock(&team->lock); 1993 2006 1994 2007 if (err) 1995 2008 return err; 1996 2009 1997 - if (netif_is_team_master(port_dev)) { 1998 - lockdep_unregister_key(&team->team_lock_key); 1999 - lockdep_register_key(&team->team_lock_key); 2000 - lockdep_set_class(&team->lock, &team->team_lock_key); 2001 - } 2002 2010 netdev_change_features(dev); 2003 2011 2004 2012 return err; ··· 2286 2304 static struct team *team_nl_team_get(struct genl_info *info) 2287 2305 { 2288 2306 struct net *net = genl_info_net(info); 2289 - int ifindex; 2290 2307 struct net_device *dev; 2291 - struct team *team; 2308 + int ifindex; 2309 + 2310 + ASSERT_RTNL(); 2292 2311 2293 2312 if (!info->attrs[TEAM_ATTR_TEAM_IFINDEX]) 2294 2313 return NULL; ··· 2301 2318 return NULL; 2302 2319 } 2303 2320 2304 - team = netdev_priv(dev); 2305 - mutex_lock(&team->lock); 2306 - return team; 2321 + return netdev_priv(dev); 2307 2322 } 2308 2323 2309 2324 static void team_nl_team_put(struct team *team) 2310 2325 { 2311 - mutex_unlock(&team->lock); 2312 2326 dev_put(team->dev); 2313 2327 } 2314 2328 ··· 2495 2515 int err; 2496 2516 LIST_HEAD(sel_opt_inst_list); 2497 2517 2518 + rtnl_lock(); 2519 + 2498 2520 team = team_nl_team_get(info); 2499 - if (!team) 2500 - return -EINVAL; 2521 + if (!team) { 2522 + err = -EINVAL; 2523 + goto rtnl_unlock; 2524 + } 2501 2525 2502 2526 list_for_each_entry(opt_inst, &team->option_inst_list, list) 2503 2527 list_add_tail(&opt_inst->tmp_list, &sel_opt_inst_list); ··· 2510 2526 &sel_opt_inst_list); 2511 2527 2512 2528 team_nl_team_put(team); 2529 + 2530 + rtnl_unlock: 2531 + rtnl_unlock(); 2513 2532 2514 2533 return err; 2515 2534 } ··· 2792 2805 struct team *team; 2793 2806 int err; 2794 2807 2808 + rtnl_lock(); 2809 + 2795 2810 team = team_nl_team_get(info); 2796 - if (!team) 2797 - return -EINVAL; 2811 + if (!team) { 2812 + err = -EINVAL; 2813 + goto rtnl_unlock; 2814 + } 2798 2815 2799 2816 err = team_nl_send_port_list_get(team, info->snd_portid, info->snd_seq, 2800 2817 NLM_F_ACK, team_nl_send_unicast, NULL); 2801 2818 2802 2819 team_nl_team_put(team); 2820 + 2821 + rtnl_unlock: 2822 + rtnl_unlock(); 2803 2823 2804 2824 return err; 2805 2825 } ··· 2955 2961 2956 2962 static void team_port_change_check(struct team_port *port, bool linkup) 2957 2963 { 2958 - struct team *team = port->team; 2964 + ASSERT_RTNL(); 2959 2965 2960 - mutex_lock(&team->lock); 2961 2966 __team_port_change_check(port, linkup); 2962 - mutex_unlock(&team->lock); 2963 2967 } 2964 2968 2965 2969
+1 -2
drivers/net/team/team_mode_activebackup.c
··· 67 67 { 68 68 struct team_port *active_port; 69 69 70 - active_port = rcu_dereference_protected(ab_priv(team)->active_port, 71 - lockdep_is_held(&team->lock)); 70 + active_port = rtnl_dereference(ab_priv(team)->active_port); 72 71 if (active_port) 73 72 ctx->data.u32_val = active_port->dev->ifindex; 74 73 else
+5 -8
drivers/net/team/team_mode_loadbalance.c
··· 301 301 if (lb_priv->ex->orig_fprog) { 302 302 /* Clear old filter data */ 303 303 __fprog_destroy(lb_priv->ex->orig_fprog); 304 - orig_fp = rcu_dereference_protected(lb_priv->fp, 305 - lockdep_is_held(&team->lock)); 304 + orig_fp = rtnl_dereference(lb_priv->fp); 306 305 } 307 306 308 307 rcu_assign_pointer(lb_priv->fp, fp); ··· 323 324 return; 324 325 325 326 __fprog_destroy(lb_priv->ex->orig_fprog); 326 - fp = rcu_dereference_protected(lb_priv->fp, 327 - lockdep_is_held(&team->lock)); 327 + fp = rtnl_dereference(lb_priv->fp); 328 328 bpf_prog_destroy(fp); 329 329 } 330 330 ··· 333 335 lb_select_tx_port_func_t *func; 334 336 char *name; 335 337 336 - func = rcu_dereference_protected(lb_priv->select_tx_port_func, 337 - lockdep_is_held(&team->lock)); 338 + func = rtnl_dereference(lb_priv->select_tx_port_func); 338 339 name = lb_select_tx_port_get_name(func); 339 340 BUG_ON(!name); 340 341 ctx->data.str_val = name; ··· 475 478 team = lb_priv_ex->team; 476 479 lb_priv = get_lb_priv(team); 477 480 478 - if (!mutex_trylock(&team->lock)) { 481 + if (!rtnl_trylock()) { 479 482 schedule_delayed_work(&lb_priv_ex->stats.refresh_dw, 0); 480 483 return; 481 484 } ··· 512 515 schedule_delayed_work(&lb_priv_ex->stats.refresh_dw, 513 516 (lb_priv_ex->stats.refresh_interval * HZ) / 10); 514 517 515 - mutex_unlock(&team->lock); 518 + rtnl_unlock(); 516 519 } 517 520 518 521 static void lb_stats_refresh_interval_get(struct team *team,
-3
include/linux/if_team.h
··· 191 191 192 192 const struct header_ops *header_ops_cache; 193 193 194 - struct mutex lock; /* used for overall locking, e.g. port lists write */ 195 - 196 194 /* 197 195 * List of enabled ports and their count 198 196 */ ··· 221 223 atomic_t count_pending; 222 224 struct delayed_work dw; 223 225 } mcast_rejoin; 224 - struct lock_class_key team_lock_key; 225 226 long mode_priv[TEAM_MODE_PRIV_LONGS]; 226 227 }; 227 228