Skip to content

Commit 77da712

Browse files
committed
Merge branch 'team-fix-reference-count-leak-when-changing-port-netns'
Ido Schimmel says: ==================== team: Fix reference count leak when changing port netns Patch #1 fixes a reference count leak that was reported by syzkaller. The leak happens when a net device that is member in a team is changing netns. The fix is to align the team driver with the bond driver and have it suppress NETDEV_CHANGEMTU events for a net device that is being unregistered. Without this change, the NETDEV_CHANGEMTU event causes inetdev_event() to recreate an inet device for this net device in its original netns, after it was previously destroyed upon NETDEV_UNREGISTER. Later on, when inetdev_event() receives a NETDEV_REGISTER event for this net device in the new nents, it simply leaks the reference: case NETDEV_REGISTER: pr_debug("%s: bug\n", __func__); RCU_INIT_POINTER(dev->ip_ptr, NULL); break; addrconf_notify() handles this differently and reuses the existing inet6 device if one exists when a NETDEV_REGISTER event is received. This creates a different problem where it is possible for a net device to reference an inet6 device that was created in a previous netns. A more generic fix that we can try in net-next is to revert the changes in the bond and team drivers and instead have IPv4 and IPv6 destroy and recreate an inet device if one already exists upon NETDEV_REGISTER. Patch #2 adds a selftest that passes with the fix and hangs without it. ==================== Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
2 parents f975a09 + 58f8ef6 commit 77da712

3 files changed

Lines changed: 39 additions & 5 deletions

File tree

drivers/net/team/team_core.c

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,7 +1292,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
12921292

12931293
static void __team_port_change_port_removed(struct team_port *port);
12941294

1295-
static int team_port_del(struct team *team, struct net_device *port_dev)
1295+
static int team_port_del(struct team *team, struct net_device *port_dev, bool unregister)
12961296
{
12971297
struct net_device *dev = team->dev;
12981298
struct team_port *port;
@@ -1330,7 +1330,13 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
13301330
__team_port_change_port_removed(port);
13311331

13321332
team_port_set_orig_dev_addr(port);
1333-
dev_set_mtu(port_dev, port->orig.mtu);
1333+
if (unregister) {
1334+
netdev_lock_ops(port_dev);
1335+
__netif_set_mtu(port_dev, port->orig.mtu);
1336+
netdev_unlock_ops(port_dev);
1337+
} else {
1338+
dev_set_mtu(port_dev, port->orig.mtu);
1339+
}
13341340
kfree_rcu(port, rcu);
13351341
netdev_info(dev, "Port device %s removed\n", portname);
13361342
netdev_compute_master_upper_features(team->dev, true);
@@ -1634,7 +1640,7 @@ static void team_uninit(struct net_device *dev)
16341640
ASSERT_RTNL();
16351641

16361642
list_for_each_entry_safe(port, tmp, &team->port_list, list)
1637-
team_port_del(team, port->dev);
1643+
team_port_del(team, port->dev, false);
16381644

16391645
__team_change_mode(team, NULL); /* cleanup */
16401646
__team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
@@ -1933,7 +1939,16 @@ static int team_del_slave(struct net_device *dev, struct net_device *port_dev)
19331939

19341940
ASSERT_RTNL();
19351941

1936-
return team_port_del(team, port_dev);
1942+
return team_port_del(team, port_dev, false);
1943+
}
1944+
1945+
static int team_del_slave_on_unregister(struct net_device *dev, struct net_device *port_dev)
1946+
{
1947+
struct team *team = netdev_priv(dev);
1948+
1949+
ASSERT_RTNL();
1950+
1951+
return team_port_del(team, port_dev, true);
19371952
}
19381953

19391954
static netdev_features_t team_fix_features(struct net_device *dev,
@@ -2926,7 +2941,7 @@ static int team_device_event(struct notifier_block *unused,
29262941
!!netif_oper_up(port->dev));
29272942
break;
29282943
case NETDEV_UNREGISTER:
2929-
team_del_slave(port->team->dev, dev);
2944+
team_del_slave_on_unregister(port->team->dev, dev);
29302945
break;
29312946
case NETDEV_FEAT_CHANGE:
29322947
if (!port->team->notifier_ctx) {
@@ -2999,3 +3014,4 @@ MODULE_LICENSE("GPL v2");
29993014
MODULE_AUTHOR("Jiri Pirko <[email protected]>");
30003015
MODULE_DESCRIPTION("Ethernet team device driver");
30013016
MODULE_ALIAS_RTNL_LINK(DRV_NAME);
3017+
MODULE_IMPORT_NS("NETDEV_INTERNAL");

tools/testing/selftests/drivers/net/team/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ TEST_PROGS := \
55
dev_addr_lists.sh \
66
options.sh \
77
propagation.sh \
8+
refleak.sh \
89
# end of TEST_PROGS
910

1011
TEST_INCLUDES := \
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#!/bin/bash
2+
# SPDX-License-Identifier: GPL-2.0
3+
# shellcheck disable=SC2154
4+
5+
lib_dir=$(dirname "$0")
6+
source "$lib_dir"/../../../net/lib.sh
7+
8+
trap cleanup_all_ns EXIT
9+
10+
# Test that there is no reference count leak and that dummy1 can be deleted.
11+
# https://lore.kernel.org/netdev/[email protected]/
12+
setup_ns ns1 ns2
13+
ip -n "$ns1" link add name team1 type team
14+
ip -n "$ns1" link add name dummy1 mtu 1499 type dummy
15+
ip -n "$ns1" link set dev dummy1 master team1
16+
ip -n "$ns1" link set dev dummy1 netns "$ns2"
17+
ip -n "$ns2" link del dev dummy1

0 commit comments

Comments
 (0)