Le 29/01/2014 21:48, Steven Rostedt a écrit :
On Wed, 29 Jan 2014 17:04:12 +0100
Nicolas Dichtel <nicolas.dichtel@xxxxxxxxx> wrote:
Your patch serie seems to be the good way to go (note that patch 1/2 does not
compile) but I think the fix is smaller because we don't have x-netns.
Here is my proposal, if you agree, I will send the same patch for ip6_tunnnel,
which have the netns leak.
Hold on. Seems that the kernels that were being tested in QA had more
code than what I was testing. Clark had backported "sit: fix use after
free of fb_tunnel_dev" and that was what was causing the
unlist_netdevice() to be missed.
When I started working on vanilla 3.10.27 as well, I first did my
original patch (which just removes the call to
unregister_netdevice_queue() from sit_exit_net()). I asked to have that
added to our kernel for testing, and they told me it was already there
via Clark's backport. Then I did the full backport as well and looked
for the leak. I'm now thinking that the full backport is not needed as
that was what caused the leak.
According to commit 9434266f2c645d4fcf62a03a8e36ad8075e37943 "sit: fix
use after free of fb_tunnel_dev", it states:
Bug: The fallback device is created in sit_init_net and assumed to
be freed in sit_exit_net. First, it is dereferenced in that
function, in sit_destroy_tunnels:
struct net *net = dev_net(sitn->fb_tunnel_dev);
Prior to this, rtnl_unlink_register has removed all devices that
match rtnl_link_ops == sit_link_ops.
Commit 205983c43700 added the line
+ sitn->fb_tunnel_dev->rtnl_link_ops = &sit_link_ops;
which cases the fallback device to match here and be freed before it
is last dereferenced.
Commit 205983c43700 was backported to 3.10, but without commit
5e6700b3bf98 "sit: add support of x-netns" which was what added the
net = dev_net(sitn->fb_tunnel_dev);
Which looks to me that the only reason I need to port back commit
5e6700b3bf98 is if I add the full backport of 9434266f2c645d4f.
Seems to me that my original patch may be good enough. The one that I
said this series obsoletes.
Note, I've talked with the people that are doing the testing, and I'm
having them revert all changes except for that one fix and rerun the
tests again. I should know the results by tomorrow.
Let me know if "sit: fix use after free of fb_tunnel_dev" still needs
to be backported due to some other way that the fallback device can be
dereferenced after being freed.
Steve, I think the patch I sent yesterday is the good fix. At the end, it's
a backport of Willem's patch. Note that he also ack that patch.
The first version you sent (which removes
unregister_netdevice_queue(sitn->fb_tunnel_dev, &list)) will introduce a
memory leak when the user destroy a netns.
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html