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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <871u4t1d9t.fsf@xmission.com>
Date:	Thu, 12 Sep 2013 22:50:06 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Francesco Ruggeri <fruggeri@...stanetworks.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jiri Pirko <jiri@...nulli.us>,
	Alexander Duyck <alexander.h.duyck@...el.com>,
	Cong Wang <amwang@...hat.com>, netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] net: race condition when removing virtual net_device

Francesco Ruggeri <fruggeri@...stanetworks.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);

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html