Re: [PATCH 1/1] net: race condition when removing virtual net_device

[ebiederm@xmission.com (Eric W. Biederman)]

Francesco Ruggeri <fruggeri@aristanetworks.com> writes:

> That would be great. There would still be one scenario to take care of though:
>
> - veth interfaces v0 and v1 are in namespaces ns0 and ns1.
> - process p0 unregisters v0, which also causes v1 to be unregistered.
> When p0 enters netdev_run_todo both v0 and v1 are in net_todo_list and
> have been unlisted from their namespaces.
> - then in p0's netdev_run_todo:

So I looked at this a little more and this problem appears largely
specific to veth.  In the normal case the caller of dellink has to hold
a reference to the network namespace to find the device to delete.

So I think the solution is just to warp the interface of the second
device into the network namespace of the device we are actually
deleting.

I will buy that similar situations can happen with other virtual devices
that have one foot in two network namespaces, and I expect the same
solution will apply.

So the patch below looks like the solution.  If there is more than one
device that needs this treatment perhaps the code should be moved
into a helper function rather than expanded inline.

Does this look like it will fix your issue?

Eric


diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index da86652..5922066 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -423,6 +423,19 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
        unregister_netdevice_queue(dev, head);
 
        if (peer) {
+               struct net *net = dev_net(dev);
+               if (dev_net(peer) != net) {
+                       /* Move the peer to the same net to avoid teardown races */
+                       char peer_name[IFNAMSIZ];
+                       int err;
+                       snprintf(fb_name, IFNAMSIZ, "dev%d", peer->ifindex);
+                       err = dev_change_net_namespace(peer, net, peer_name);
+                       if (err) {
+                               pr_emerg("%s: failed to move %s to peers net: %d\n",
+                                        __func__, peer->name, err);
+                               BUG();
+                       }
+               }
                priv = netdev_priv(peer);
                RCU_INIT_POINTER(priv->peer, NULL);
                unregister_netdevice_queue(peer, head);