Skip to content

Commit

Permalink
Merge branch 'net-fix-nested-device-bugs'
Browse files Browse the repository at this point in the history
Taehee Yoo says:

====================
net: fix nested device bugs

This patchset fixes several bugs that are related to nesting
device infrastructure.
Current nesting infrastructure code doesn't limit the depth level of
devices. nested devices could be handled recursively. at that moment,
it needs huge memory and stack overflow could occur.
Below devices type have same bug.
VLAN, BONDING, TEAM, MACSEC, MACVLAN, IPVLAN, and VXLAN.
But I couldn't test all interface types so there could be more device
types, which have similar problems.
Maybe qmi_wwan.c code could have same problem.
So, I would appreciate if someone test qmi_wwan.c and other modules.

Test commands:
    ip link add dummy0 type dummy
    ip link add vlan1 link dummy0 type vlan id 1

    for i in {2..100}
    do
	    let A=$i-1
	    ip link add name vlan$i link vlan$A type vlan id $i
    done
    ip link del dummy0

1st patch actually fixes the root cause.
It adds new common variables {upper/lower}_level that represent
depth level. upper_level variable is depth of upper devices.
lower_level variable is depth of lower devices.

      [U][L]       [U][L]
vlan1  1  5  vlan4  1  4
vlan2  2  4  vlan5  2  3
vlan3  3  3    |
  |            |
  +------------+
  |
vlan6  4  2
dummy0 5  1

After this patch, the nesting infrastructure code uses this variable to
check the depth level.

2nd patch fixes Qdisc lockdep related problem.
Before this patch, devices use static lockdep map.
So, if devices that are same types are nested, lockdep will warn about
recursive situation.
These patches make these devices use dynamic lockdep key instead of
static lock or subclass.

3rd patch fixes unexpected IFF_BONDING bit unset.
When nested bonding interface scenario, bonding interface could lost it's
IFF_BONDING flag. This should not happen.
This patch adds a condition before unsetting IFF_BONDING.

4th patch fixes nested locking problem in bonding interface
Bonding interface has own lock and this uses static lock.
Bonding interface could be nested and it uses same lockdep key.
So that unexisting lockdep warning occurs.

5th patch fixes nested locking problem in team interface
Team interface has own lock and this uses static lock.
Team interface could be nested and it uses same lockdep key.
So that unexisting lockdep warning occurs.

6th patch fixes a refcnt leak in the macsec module.
When the macsec module is unloaded, refcnt leaks occur.
But actually, that holding refcnt is unnecessary.
So this patch just removes these code.

7th patch adds ignore flag to an adjacent structure.
In order to exchange an adjacent node safely, ignore flag is needed.

8th patch makes vxlan add an adjacent link to limit depth level.
Vxlan interface could set it's lower interface and these lower interfaces
are handled recursively.
So, if the depth of lower interfaces is too deep, stack overflow could
happen.

9th patch removes unnecessary variables and callback.
After 1st patch, subclass callback and variables are unnecessary.
This patch just removes these variables and callback.

10th patch fix refcnt leaks in the virt_wifi module
Like every nested interface, the upper interface should be deleted
before the lower interface is deleted.
In order to fix this, the notifier routine is added in this patch.

v4 -> v5 :
 - Update log messages
 - Move variables position, 1st patch
 - Fix iterator routine, 1st patch
 - Add generic lockdep key code, which replaces 2, 4, 5, 6, 7 patches.
 - Log message update, 10th patch
 - Fix wrong error value in error path of __init routine, 10th patch
 - hold module refcnt when interface is created, 10th patch
v3 -> v4 :
 - Add new 12th patch to fix refcnt leaks in the virt_wifi module
 - Fix wrong usage netdev_upper_dev_link() in the vxlan.c
 - Preserve reverse christmas tree variable ordering in the vxlan.c
 - Add missing static keyword in the dev.c
 - Expose netdev_adjacent_change_{prepare/commit/abort} instead of
   netdev_adjacent_dev_{enable/disable}
v2 -> v3 :
 - Modify nesting infrastructure code to use iterator instead of recursive.
v1 -> v2 :
 - Make the 3rd patch do not add a new priv_flag.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Oct 24, 2019
2 parents 82ecff6 + 1962f86 commit 6592137
Show file tree
Hide file tree
Showing 38 changed files with 628 additions and 521 deletions.
2 changes: 1 addition & 1 deletion drivers/net/bonding/bond_alb.c
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ static int alb_upper_dev_walk(struct net_device *upper, void *_data)
struct bond_vlan_tag *tags;

if (is_vlan_dev(upper) &&
bond->nest_level == vlan_get_encap_level(upper) - 1) {
bond->dev->lower_level == upper->lower_level - 1) {
if (upper->addr_assign_type == NET_ADDR_STOLEN) {
alb_send_lp_vid(slave, mac_addr,
vlan_dev_vlan_proto(upper),
Expand Down
30 changes: 10 additions & 20 deletions drivers/net/bonding/bond_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1733,8 +1733,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
goto err_upper_unlink;
}

bond->nest_level = dev_get_nest_level(bond_dev) + 1;

/* If the mode uses primary, then the following is handled by
* bond_change_active_slave().
*/
Expand Down Expand Up @@ -1816,7 +1814,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
slave_disable_netpoll(new_slave);

err_close:
slave_dev->priv_flags &= ~IFF_BONDING;
if (!netif_is_bond_master(slave_dev))
slave_dev->priv_flags &= ~IFF_BONDING;
dev_close(slave_dev);

err_restore_mac:
Expand Down Expand Up @@ -1956,9 +1955,6 @@ static int __bond_release_one(struct net_device *bond_dev,
if (!bond_has_slaves(bond)) {
bond_set_carrier(bond);
eth_hw_addr_random(bond_dev);
bond->nest_level = SINGLE_DEPTH_NESTING;
} else {
bond->nest_level = dev_get_nest_level(bond_dev) + 1;
}

unblock_netpoll_tx();
Expand Down Expand Up @@ -2017,7 +2013,8 @@ static int __bond_release_one(struct net_device *bond_dev,
else
dev_set_mtu(slave_dev, slave->original_mtu);

slave_dev->priv_flags &= ~IFF_BONDING;
if (!netif_is_bond_master(slave_dev))
slave_dev->priv_flags &= ~IFF_BONDING;

bond_free_slave(slave);

Expand Down Expand Up @@ -3442,13 +3439,6 @@ static void bond_fold_stats(struct rtnl_link_stats64 *_res,
}
}

static int bond_get_nest_level(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);

return bond->nest_level;
}

static void bond_get_stats(struct net_device *bond_dev,
struct rtnl_link_stats64 *stats)
{
Expand All @@ -3457,7 +3447,7 @@ static void bond_get_stats(struct net_device *bond_dev,
struct list_head *iter;
struct slave *slave;

spin_lock_nested(&bond->stats_lock, bond_get_nest_level(bond_dev));
spin_lock(&bond->stats_lock);
memcpy(stats, &bond->bond_stats, sizeof(*stats));

rcu_read_lock();
Expand Down Expand Up @@ -4268,7 +4258,6 @@ static const struct net_device_ops bond_netdev_ops = {
.ndo_neigh_setup = bond_neigh_setup,
.ndo_vlan_rx_add_vid = bond_vlan_rx_add_vid,
.ndo_vlan_rx_kill_vid = bond_vlan_rx_kill_vid,
.ndo_get_lock_subclass = bond_get_nest_level,
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_netpoll_setup = bond_netpoll_setup,
.ndo_netpoll_cleanup = bond_netpoll_cleanup,
Expand All @@ -4295,8 +4284,6 @@ void bond_setup(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);

spin_lock_init(&bond->mode_lock);
spin_lock_init(&bond->stats_lock);
bond->params = bonding_defaults;

/* Initialize pointers */
Expand Down Expand Up @@ -4365,6 +4352,7 @@ static void bond_uninit(struct net_device *bond_dev)

list_del(&bond->bond_list);

lockdep_unregister_key(&bond->stats_lock_key);
bond_debug_unregister(bond);
}

Expand Down Expand Up @@ -4768,8 +4756,10 @@ static int bond_init(struct net_device *bond_dev)
if (!bond->wq)
return -ENOMEM;

bond->nest_level = SINGLE_DEPTH_NESTING;
netdev_lockdep_set_classes(bond_dev);
spin_lock_init(&bond->mode_lock);
spin_lock_init(&bond->stats_lock);
lockdep_register_key(&bond->stats_lock_key);
lockdep_set_class(&bond->stats_lock, &bond->stats_lock_key);

list_add_tail(&bond->bond_list, &bn->dev_list);

Expand Down
2 changes: 1 addition & 1 deletion drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3160,7 +3160,7 @@ static int add_vlan_pop_action(struct mlx5e_priv *priv,
struct mlx5_esw_flow_attr *attr,
u32 *action)
{
int nest_level = vlan_get_encap_level(attr->parse_attr->filter_dev);
int nest_level = attr->parse_attr->filter_dev->lower_level;
struct flow_action_entry vlan_act = {
.id = FLOW_ACTION_VLAN_POP,
};
Expand Down
18 changes: 0 additions & 18 deletions drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
Original file line number Diff line number Diff line change
Expand Up @@ -299,22 +299,6 @@ static void nfp_repr_clean(struct nfp_repr *repr)
nfp_port_free(repr->port);
}

static struct lock_class_key nfp_repr_netdev_xmit_lock_key;
static struct lock_class_key nfp_repr_netdev_addr_lock_key;

static void nfp_repr_set_lockdep_class_one(struct net_device *dev,
struct netdev_queue *txq,
void *_unused)
{
lockdep_set_class(&txq->_xmit_lock, &nfp_repr_netdev_xmit_lock_key);
}

static void nfp_repr_set_lockdep_class(struct net_device *dev)
{
lockdep_set_class(&dev->addr_list_lock, &nfp_repr_netdev_addr_lock_key);
netdev_for_each_tx_queue(dev, nfp_repr_set_lockdep_class_one, NULL);
}

int nfp_repr_init(struct nfp_app *app, struct net_device *netdev,
u32 cmsg_port_id, struct nfp_port *port,
struct net_device *pf_netdev)
Expand All @@ -324,8 +308,6 @@ int nfp_repr_init(struct nfp_app *app, struct net_device *netdev,
u32 repr_cap = nn->tlv_caps.repr_cap;
int err;

nfp_repr_set_lockdep_class(netdev);

repr->port = port;
repr->dst = metadata_dst_alloc(0, METADATA_HW_PORT_MUX, GFP_KERNEL);
if (!repr->dst)
Expand Down
22 changes: 0 additions & 22 deletions drivers/net/hamradio/bpqether.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,27 +107,6 @@ struct bpqdev {

static LIST_HEAD(bpq_devices);

/*
* bpqether network devices are paired with ethernet devices below them, so
* form a special "super class" of normal ethernet devices; split their locks
* off into a separate class since they always nest.
*/
static struct lock_class_key bpq_netdev_xmit_lock_key;
static struct lock_class_key bpq_netdev_addr_lock_key;

static void bpq_set_lockdep_class_one(struct net_device *dev,
struct netdev_queue *txq,
void *_unused)
{
lockdep_set_class(&txq->_xmit_lock, &bpq_netdev_xmit_lock_key);
}

static void bpq_set_lockdep_class(struct net_device *dev)
{
lockdep_set_class(&dev->addr_list_lock, &bpq_netdev_addr_lock_key);
netdev_for_each_tx_queue(dev, bpq_set_lockdep_class_one, NULL);
}

/* ------------------------------------------------------------------------ */


Expand Down Expand Up @@ -498,7 +477,6 @@ static int bpq_new_device(struct net_device *edev)
err = register_netdevice(ndev);
if (err)
goto error;
bpq_set_lockdep_class(ndev);

/* List protected by RTNL */
list_add_rcu(&bpq->bpq_list, &bpq_devices);
Expand Down
2 changes: 0 additions & 2 deletions drivers/net/hyperv/netvsc_drv.c
Original file line number Diff line number Diff line change
Expand Up @@ -2335,8 +2335,6 @@ static int netvsc_probe(struct hv_device *dev,
NETIF_F_HW_VLAN_CTAG_RX;
net->vlan_features = net->features;

netdev_lockdep_set_classes(net);

/* MTU range: 68 - 1500 or 65521 */
net->min_mtu = NETVSC_MTU_MIN;
if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2)
Expand Down
2 changes: 0 additions & 2 deletions drivers/net/ipvlan/ipvlan_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,6 @@ static int ipvlan_init(struct net_device *dev)
dev->gso_max_segs = phy_dev->gso_max_segs;
dev->hard_header_len = phy_dev->hard_header_len;

netdev_lockdep_set_classes(dev);

ipvlan->pcpu_stats = netdev_alloc_pcpu_stats(struct ipvl_pcpu_stats);
if (!ipvlan->pcpu_stats)
return -ENOMEM;
Expand Down
18 changes: 0 additions & 18 deletions drivers/net/macsec.c
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ struct macsec_dev {
struct pcpu_secy_stats __percpu *stats;
struct list_head secys;
struct gro_cells gro_cells;
unsigned int nest_level;
};

/**
Expand Down Expand Up @@ -2750,7 +2749,6 @@ static netdev_tx_t macsec_start_xmit(struct sk_buff *skb,

#define MACSEC_FEATURES \
(NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST)
static struct lock_class_key macsec_netdev_addr_lock_key;

static int macsec_dev_init(struct net_device *dev)
{
Expand Down Expand Up @@ -2958,11 +2956,6 @@ static int macsec_get_iflink(const struct net_device *dev)
return macsec_priv(dev)->real_dev->ifindex;
}

static int macsec_get_nest_level(struct net_device *dev)
{
return macsec_priv(dev)->nest_level;
}

static const struct net_device_ops macsec_netdev_ops = {
.ndo_init = macsec_dev_init,
.ndo_uninit = macsec_dev_uninit,
Expand All @@ -2976,7 +2969,6 @@ static const struct net_device_ops macsec_netdev_ops = {
.ndo_start_xmit = macsec_start_xmit,
.ndo_get_stats64 = macsec_get_stats64,
.ndo_get_iflink = macsec_get_iflink,
.ndo_get_lock_subclass = macsec_get_nest_level,
};

static const struct device_type macsec_type = {
Expand All @@ -3001,12 +2993,10 @@ static const struct nla_policy macsec_rtnl_policy[IFLA_MACSEC_MAX + 1] = {
static void macsec_free_netdev(struct net_device *dev)
{
struct macsec_dev *macsec = macsec_priv(dev);
struct net_device *real_dev = macsec->real_dev;

free_percpu(macsec->stats);
free_percpu(macsec->secy.tx_sc.stats);

dev_put(real_dev);
}

static void macsec_setup(struct net_device *dev)
Expand Down Expand Up @@ -3261,14 +3251,6 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
if (err < 0)
return err;

dev_hold(real_dev);

macsec->nest_level = dev_get_nest_level(real_dev) + 1;
netdev_lockdep_set_classes(dev);
lockdep_set_class_and_subclass(&dev->addr_list_lock,
&macsec_netdev_addr_lock_key,
macsec_get_nest_level(dev));

err = netdev_upper_dev_link(real_dev, dev, extack);
if (err < 0)
goto unregister;
Expand Down
19 changes: 0 additions & 19 deletions drivers/net/macvlan.c
Original file line number Diff line number Diff line change
Expand Up @@ -852,8 +852,6 @@ static int macvlan_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
* "super class" of normal network devices; split their locks off into a
* separate class since they always nest.
*/
static struct lock_class_key macvlan_netdev_addr_lock_key;

#define ALWAYS_ON_OFFLOADS \
(NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \
NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL)
Expand All @@ -869,19 +867,6 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
#define MACVLAN_STATE_MASK \
((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))

static int macvlan_get_nest_level(struct net_device *dev)
{
return ((struct macvlan_dev *)netdev_priv(dev))->nest_level;
}

static void macvlan_set_lockdep_class(struct net_device *dev)
{
netdev_lockdep_set_classes(dev);
lockdep_set_class_and_subclass(&dev->addr_list_lock,
&macvlan_netdev_addr_lock_key,
macvlan_get_nest_level(dev));
}

static int macvlan_init(struct net_device *dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
Expand All @@ -900,8 +885,6 @@ static int macvlan_init(struct net_device *dev)
dev->gso_max_segs = lowerdev->gso_max_segs;
dev->hard_header_len = lowerdev->hard_header_len;

macvlan_set_lockdep_class(dev);

vlan->pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
if (!vlan->pcpu_stats)
return -ENOMEM;
Expand Down Expand Up @@ -1161,7 +1144,6 @@ static const struct net_device_ops macvlan_netdev_ops = {
.ndo_fdb_add = macvlan_fdb_add,
.ndo_fdb_del = macvlan_fdb_del,
.ndo_fdb_dump = ndo_dflt_fdb_dump,
.ndo_get_lock_subclass = macvlan_get_nest_level,
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = macvlan_dev_poll_controller,
.ndo_netpoll_setup = macvlan_dev_netpoll_setup,
Expand Down Expand Up @@ -1445,7 +1427,6 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
vlan->dev = dev;
vlan->port = port;
vlan->set_features = MACVLAN_FEATURES;
vlan->nest_level = dev_get_nest_level(lowerdev) + 1;

vlan->mode = MACVLAN_MODE_VEPA;
if (data && data[IFLA_MACVLAN_MODE])
Expand Down
2 changes: 0 additions & 2 deletions drivers/net/ppp/ppp_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -1324,8 +1324,6 @@ static int ppp_dev_init(struct net_device *dev)
{
struct ppp *ppp;

netdev_lockdep_set_classes(dev);

ppp = netdev_priv(dev);
/* Let the netdevice take a reference on the ppp file. This ensures
* that ppp_destroy_interface() won't run before the device gets
Expand Down
16 changes: 12 additions & 4 deletions drivers/net/team/team.c
Original file line number Diff line number Diff line change
Expand Up @@ -1615,7 +1615,6 @@ static int team_init(struct net_device *dev)
int err;

team->dev = dev;
mutex_init(&team->lock);
team_set_no_mode(team);

team->pcpu_stats = netdev_alloc_pcpu_stats(struct team_pcpu_stats);
Expand All @@ -1642,7 +1641,8 @@ static int team_init(struct net_device *dev)
goto err_options_register;
netif_carrier_off(dev);

netdev_lockdep_set_classes(dev);
lockdep_register_key(&team->team_lock_key);
__mutex_init(&team->lock, "team->team_lock_key", &team->team_lock_key);

return 0;

Expand Down Expand Up @@ -1673,6 +1673,7 @@ static void team_uninit(struct net_device *dev)
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)
Expand Down Expand Up @@ -1976,8 +1977,15 @@ static int team_del_slave(struct net_device *dev, struct net_device *port_dev)
err = team_port_del(team, port_dev);
mutex_unlock(&team->lock);

if (!err)
netdev_change_features(dev);
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;
}
Expand Down
1 change: 0 additions & 1 deletion drivers/net/vrf.c
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,6 @@ static int vrf_dev_init(struct net_device *dev)

/* similarly, oper state is irrelevant; set to up to avoid confusion */
dev->operstate = IF_OPER_UP;
netdev_lockdep_set_classes(dev);
return 0;

out_rth:
Expand Down
Loading

0 comments on commit 6592137

Please sign in to comment.