From fc1dd36992bb041b4470120aecf8986910c56088 Mon Sep 17 00:00:00 2001 From: Kirill Tkhai Date: Fri, 30 Mar 2018 19:38:27 +0300 Subject: [PATCH 1/2] net: Remove net_rwsem from {, un}register_netdevice_notifier() These functions take net_rwsem, while wireless_nlevent_flush() also takes it. But down_read() can't be taken recursive, because of rw_semaphore design, which prevents it to be occupied by only readers forever. Since we take pernet_ops_rwsem in {,un}register_netdevice_notifier(), net list can't change, so these down_read()/up_read() can be removed. Fixes: f0b07bb151b0 "net: Introduce net_rwsem to protect net_namespace_list" Signed-off-by: Kirill Tkhai Signed-off-by: David S. Miller --- net/core/dev.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 07da7add4845..8edb58829124 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1633,7 +1633,6 @@ int register_netdevice_notifier(struct notifier_block *nb) goto unlock; if (dev_boot_phase) goto unlock; - down_read(&net_rwsem); for_each_net(net) { for_each_netdev(net, dev) { err = call_netdevice_notifier(nb, NETDEV_REGISTER, dev); @@ -1647,7 +1646,6 @@ int register_netdevice_notifier(struct notifier_block *nb) call_netdevice_notifier(nb, NETDEV_UP, dev); } } - up_read(&net_rwsem); unlock: rtnl_unlock(); @@ -1671,7 +1669,6 @@ rollback: } outroll: - up_read(&net_rwsem); raw_notifier_chain_unregister(&netdev_chain, nb); goto unlock; } @@ -1704,7 +1701,6 @@ int unregister_netdevice_notifier(struct notifier_block *nb) if (err) goto unlock; - down_read(&net_rwsem); for_each_net(net) { for_each_netdev(net, dev) { if (dev->flags & IFF_UP) { @@ -1715,7 +1711,6 @@ int unregister_netdevice_notifier(struct notifier_block *nb) call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev); } } - up_read(&net_rwsem); unlock: rtnl_unlock(); up_write(&pernet_ops_rwsem); From 554873e517115c4b7207259f1cadfd77d90b5395 Mon Sep 17 00:00:00 2001 From: Kirill Tkhai Date: Fri, 30 Mar 2018 19:38:37 +0300 Subject: [PATCH 2/2] net: Do not take net_rwsem in __rtnl_link_unregister() This function calls call_netdevice_notifier(), which also may take net_rwsem. So, we can't use net_rwsem here. This patch makes callers of this functions take pernet_ops_rwsem, like register_netdevice_notifier() does. This will protect the modifications of net_namespace_list, and allows notifiers to take it (they won't have to care about context). Since __rtnl_link_unregister() is used on module load and unload (which are not frequent operations), this looks for me better, than make all call_netdevice_notifier() always executing in "protected net_namespace_list" context. Also, this fixes the problem we had a deal in 328fbe747ad4 "Close race between {un, }register_netdevice_notifier and ...", and guarantees __rtnl_link_unregister() does not skip exitting net. Signed-off-by: Kirill Tkhai Signed-off-by: David S. Miller --- drivers/net/dummy.c | 2 ++ drivers/net/ifb.c | 2 ++ net/core/net_namespace.c | 1 + net/core/rtnetlink.c | 6 +++--- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c index 30b1c8512049..0d15a12a4560 100644 --- a/drivers/net/dummy.c +++ b/drivers/net/dummy.c @@ -219,6 +219,7 @@ static int __init dummy_init_module(void) { int i, err = 0; + down_write(&pernet_ops_rwsem); rtnl_lock(); err = __rtnl_link_register(&dummy_link_ops); if (err < 0) @@ -233,6 +234,7 @@ static int __init dummy_init_module(void) out: rtnl_unlock(); + up_write(&pernet_ops_rwsem); return err; } diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c index 0008da7e9d4c..5f2897ec0edc 100644 --- a/drivers/net/ifb.c +++ b/drivers/net/ifb.c @@ -330,6 +330,7 @@ static int __init ifb_init_module(void) { int i, err; + down_write(&pernet_ops_rwsem); rtnl_lock(); err = __rtnl_link_register(&ifb_link_ops); if (err < 0) @@ -344,6 +345,7 @@ static int __init ifb_init_module(void) out: rtnl_unlock(); + up_write(&pernet_ops_rwsem); return err; } diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 7fdf321d4997..a11e03f920d3 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -51,6 +51,7 @@ static bool init_net_initialized; * outside. */ DECLARE_RWSEM(pernet_ops_rwsem); +EXPORT_SYMBOL_GPL(pernet_ops_rwsem); #define MIN_PERNET_OPS_ID \ ((sizeof(struct net_generic) + sizeof(void *) - 1) / sizeof(void *)) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index e86b28482ca7..45936922d7e2 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -412,17 +412,17 @@ static void __rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops) * __rtnl_link_unregister - Unregister rtnl_link_ops from rtnetlink. * @ops: struct rtnl_link_ops * to unregister * - * The caller must hold the rtnl_mutex. + * The caller must hold the rtnl_mutex and guarantee net_namespace_list + * integrity (hold pernet_ops_rwsem for writing to close the race + * with setup_net() and cleanup_net()). */ void __rtnl_link_unregister(struct rtnl_link_ops *ops) { struct net *net; - down_read(&net_rwsem); for_each_net(net) { __rtnl_kill_links(net, ops); } - up_read(&net_rwsem); list_del(&ops->list); } EXPORT_SYMBOL_GPL(__rtnl_link_unregister);