Re: linux-next: net->netns_ids is used after calling idr_destroy for it

[Cong Wang]

On Tue, Nov 15, 2016 at 10:04 AM, Cong Wang <xiyou.wangc...@gmail.com> wrote:
> On Mon, Nov 14, 2016 at 10:23 PM, Andrei Vagin <ava...@gmail.com> wrote:
>> Hi Nicolas,
>>
>> cleanup_net() calls idr_destroy(net->netns_ids) for network namespaces
>> and then it calls unregister_netdevice_many() which calls
>> idr_alloc(net0>netns_ids). It looks wrong, doesn't it?
>>
>
> netns id is designed to allocate lazily, but yeah it makes no sense
> to allocate id for the netns being destroyed, not to mention idr is freed.
>
> I will send a patch.

Could you try the attached patch? I just did some quick netns creation/destroy
tests.

Thanks!

diff --git a/net/core/dev.c b/net/core/dev.c
index 6deba68..f9a2969 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6685,7 +6685,7 @@ static void net_set_todo(struct net_device *dev)
        dev_net(dev)->dev_unreg_count++;
 }
 
-static void rollback_registered_many(struct list_head *head)
+static void rollback_registered_many(struct list_head *head, bool send_rtmsg)
 {
        struct net_device *dev, *tmp;
        LIST_HEAD(close_head);
@@ -6737,8 +6737,8 @@ static void rollback_registered_many(struct list_head 
*head)
                */
                call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
 
-               if (!dev->rtnl_link_ops ||
-                   dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
+               if ((!dev->rtnl_link_ops ||
+                   dev->rtnl_link_state == RTNL_LINK_INITIALIZED) && 
send_rtmsg)
                        skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U,
                                                     GFP_KERNEL);
 
@@ -6777,7 +6777,7 @@ static void rollback_registered(struct net_device *dev)
        LIST_HEAD(single);
 
        list_add(&dev->unreg_list, &single);
-       rollback_registered_many(&single);
+       rollback_registered_many(&single, true);
        list_del(&single);
 }
 
@@ -7769,6 +7769,18 @@ void unregister_netdevice_queue(struct net_device *dev, 
struct list_head *head)
 }
 EXPORT_SYMBOL(unregister_netdevice_queue);
 
+static void __unregister_netdevice_many(struct list_head *head, bool 
send_rtmsg)
+{
+       struct net_device *dev;
+
+       if (!list_empty(head)) {
+               rollback_registered_many(head, send_rtmsg);
+               list_for_each_entry(dev, head, unreg_list)
+                       net_set_todo(dev);
+               list_del(head);
+       }
+}
+
 /**
  *     unregister_netdevice_many - unregister many devices
  *     @head: list of devices
@@ -7778,14 +7790,7 @@ EXPORT_SYMBOL(unregister_netdevice_queue);
  */
 void unregister_netdevice_many(struct list_head *head)
 {
-       struct net_device *dev;
-
-       if (!list_empty(head)) {
-               rollback_registered_many(head);
-               list_for_each_entry(dev, head, unreg_list)
-                       net_set_todo(dev);
-               list_del(head);
-       }
+       __unregister_netdevice_many(head, true);
 }
 EXPORT_SYMBOL(unregister_netdevice_many);
 
@@ -8239,7 +8244,7 @@ static void __net_exit default_device_exit_batch(struct 
list_head *net_list)
                                unregister_netdevice_queue(dev, &dev_kill_list);
                }
        }
-       unregister_netdevice_many(&dev_kill_list);
+       __unregister_netdevice_many(&dev_kill_list, false);
        rtnl_unlock();
 }