Re: [BUG] stable v3.10.16+ introduced by "ip6tnl: allow to use rtnlops on fb tunnel"

[Steven Rostedt]

On Sun, 8 Dec 2013 16:25:31 -0800
Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:


> > Which, when applied to 3.10.18, fixes the bug. Was there a reason that
> > this part of the commit wasn't backported? or was this just an oversight?
> 
> It looks like it was left out to me as well.
> 
> David, any objection to me making this fixup in the 3.10-stable tree?

I'm not sure my fix up was the right thing to do. I believe upstream
commit 1e9f3d6f1c403 "ip6tnl: fix use after free of fb_tnl_dev" which
was ported back to 3.12 stable, is the correct fix.

I even backported that commit to 3.10 and tested it, and the bug did
indeed go away.

-- Steve

ip6tnl: fix use after free of fb_tnl_dev

commit 1e9f3d6f1c403dd2b6270f654b4747147aa2306f upstream.

Bug has been introduced by commit bb8140947a24 ("ip6tnl: allow to use rtnl ops
on fb tunnel").

When ip6_tunnel.ko is unloaded, FB device is delete by rtnl_link_unregister()
and then we try to use the pointer in ip6_tnl_destroy_tunnels().

Let's add an handler for dellink, which will never remove the FB tunnel. With
this patch it will no more be possible to remove it via 'ip link del ip6tnl0',
but it's safer.

The same fix was already proposed by Willem de Bruijn <willemb@xxxxxxxxxx> for
sit interfaces.

CC: Willem de Bruijn <willemb@xxxxxxxxxx>
Reported-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@xxxxxxxxx>
---
 net/ipv6/ip6_tunnel.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Index: linux-trace.git/net/ipv6/ip6_tunnel.c
===================================================================
--- linux-trace.git.orig/net/ipv6/ip6_tunnel.c
+++ linux-trace.git/net/ipv6/ip6_tunnel.c
@@ -1617,6 +1617,15 @@ static int ip6_tnl_changelink(struct net
 	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
 	.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,
 };
@@ -1697,10 +1707,11 @@ static struct xfrm6_tunnel ip6ip6_handle
 	.priority	=	1,
 };
 
-static void __net_exit ip6_tnl_destroy_tunnels(struct ip6_tnl_net *ip6n)
+static void __net_exit ip6_tnl_destroy_tunnels(struct net *net)
 {
 	int h;
 	struct ip6_tnl *t;
+	struct ip6_tnl_net *ip6n = net_generic(net, ip6_tnl_net_id);
 	LIST_HEAD(list);
 
 	for (h = 0; h < HASH_SIZE; h++) {
@@ -1755,10 +1766,8 @@ err_alloc_dev:
 
 static void __net_exit ip6_tnl_exit_net(struct net *net)
 {
-	struct ip6_tnl_net *ip6n = net_generic(net, ip6_tnl_net_id);
-
 	rtnl_lock();
-	ip6_tnl_destroy_tunnels(ip6n);
+	ip6_tnl_destroy_tunnels(net);
 	rtnl_unlock();
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/