mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
To: rostedt@goodmis.org
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	stable@vger.kernel.org, williams@redhat.com, lclaudio@uudg.org,
	jkacur@redhat.com, willemb@google.com,
	Nicolas Dichtel <nicolas.dichtel@6wind.com>
Subject: [PATCH linux-3.10.y 3/3] ip6tnl: fix double free of fb_tnl_dev on exit
Date: Thu, 30 Jan 2014 11:09:23 +0100	[thread overview]
Message-ID: <1391076563-10798-3-git-send-email-nicolas.dichtel@6wind.com> (raw)
In-Reply-To: <1391076563-10798-1-git-send-email-nicolas.dichtel@6wind.com>

This problem was fixed upstream by commit 1e9f3d6f1c40 ("ip6tnl: fix use after
free of fb_tnl_dev").
The upstream patch depends on upstream commit 0bd8762824e7 ("ip6tnl: add x-netns
support"), which was not backported into 3.10 branch.

First, explain the problem: when the ip6_tunnel module is unloaded,
ip6_tunnel_cleanup() is called.
rmmod ip6_tunnel
=> ip6_tunnel_cleanup()
  => rtnl_link_unregister()
    => __rtnl_kill_links()
      => for_each_netdev(net, dev) {
        if (dev->rtnl_link_ops == ops)
        	ops->dellink(dev, &list_kill);
        }
At this point, the FB device is deleted (and all ip6tnl tunnels).
  => unregister_pernet_device()
    => unregister_pernet_operations()
      => ops_exit_list()
        => ip6_tnl_exit_net()
          => ip6_tnl_destroy_tunnels()
            => t = rtnl_dereference(ip6n->tnls_wc[0]);
               unregister_netdevice_queue(t->dev, &list);
We delete the FB device a second time here!

The previous fix removes these lines, which fix this double free. But the patch
introduces a memory leak when a netns is destroyed, because the FB device is
never deleted. By adding an rtnl ops which delete all ip6tnl device excepting
the FB device, we can keep this exlicit removal in ip6_tnl_destroy_tunnels().

CC: Steven Rostedt <rostedt@goodmis.org>
CC: Willem de Bruijn <willemb@google.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv6/ip6_tunnel.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 0516ebbea80b..f21cf476b00c 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1617,6 +1617,15 @@ static int ip6_tnl_changelink(struct net_device *dev, struct nlattr *tb[],
 	return ip6_tnl_update(t, &p);
 }
 
+static void ip6_tnl_dellink(struct net_device *dev, struct list_head *head)
+{
+	struct net *net = dev_net(dev);
+	struct ip6_tnl_net *ip6n = net_generic(net, ip6_tnl_net_id);
+
+	if (dev != ip6n->fb_tnl_dev)
+		unregister_netdevice_queue(dev, head);
+}
+
 static size_t ip6_tnl_get_size(const struct net_device *dev)
 {
 	return
@@ -1681,6 +1690,7 @@ static struct rtnl_link_ops ip6_link_ops __read_mostly = {
 	.validate	= ip6_tnl_validate,
 	.newlink	= ip6_tnl_newlink,
 	.changelink	= ip6_tnl_changelink,
+	.dellink	= ip6_tnl_dellink,
 	.get_size	= ip6_tnl_get_size,
 	.fill_info	= ip6_tnl_fill_info,
 };
-- 
1.8.4.1


  parent reply	other threads:[~2014-01-30 10:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-28 20:57 [PATCH 0/2] [BUG FIXES - 3.10.27] sit: More backports Steven Rostedt
2014-01-28 20:57 ` [PATCH 1/2] sit: Unregister sit devices with rtnl_link_ops Steven Rostedt
2014-01-28 20:57 ` [PATCH 2/2] sit: fix use after free of fb_tunnel_dev Steven Rostedt
2014-01-29  7:52 ` [PATCH 0/2] [BUG FIXES - 3.10.27] sit: More backports David Miller
2014-01-29 12:59   ` Steven Rostedt
2014-01-29 11:04 ` Nicolas Dichtel
2014-01-29 12:57   ` Steven Rostedt
2014-01-29 16:04     ` Nicolas Dichtel
2014-01-29 17:21       ` Willem de Bruijn
2014-01-29 20:48       ` Steven Rostedt
2014-01-30  9:28         ` Nicolas Dichtel
2014-01-30 10:09           ` [PATCH linux-3.10.y 1/3] sit: fix double free of fb_tunnel_dev on exit Nicolas Dichtel
2014-01-30 10:09             ` [PATCH linux-3.10.y 2/3] Revert "ip6tnl: fix use after free of fb_tnl_dev" Nicolas Dichtel
2014-01-30 10:09             ` Nicolas Dichtel [this message]
2014-01-30 13:31           ` [PATCH 0/2] [BUG FIXES - 3.10.27] sit: More backports Steven Rostedt
2014-01-30 13:42             ` Nicolas Dichtel
2014-01-30 22:10               ` Steven Rostedt
2014-01-31  8:24                 ` [PATCH linux-3.10.y v2 1/3] sit: fix double free of fb_tunnel_dev on exit Nicolas Dichtel
2014-01-31  8:24                   ` [PATCH linux-3.10.y v2 2/3] Revert "ip6tnl: fix use after free of fb_tnl_dev" Nicolas Dichtel
2014-01-31  8:24                   ` [PATCH linux-3.10.y v2 3/3] ip6tnl: fix double free of fb_tnl_dev on exit Nicolas Dichtel
2014-01-31 17:19                   ` [PATCH linux-3.10.y v2 1/3] sit: fix double free of fb_tunnel_dev " Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1391076563-10798-3-git-send-email-nicolas.dichtel@6wind.com \
    --to=nicolas.dichtel@6wind.com \
    --cc=jkacur@redhat.com \
    --cc=lclaudio@uudg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=stable@vger.kernel.org \
    --cc=willemb@google.com \
    --cc=williams@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®