Patchwork [linux-3.10.y] ip6tnl: fix use after free of fb_tnl_dev

login
register
mail settings
Submitter Nicolas Dichtel
Date Dec. 13, 2013, 9:06 a.m.
Message ID <1386925595-4995-1-git-send-email-nicolas.dichtel@6wind.com>
Download mbox | patch
Permalink /patch/300942/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Nicolas Dichtel - Dec. 13, 2013, 9:06 a.m.
The upstream commit bb8140947a24 ("ip6tnl: allow to use rtnl ops on fb tunnel")
(backported into linux-3.10.y) left a bug which was fixed upstream by commit
1e9f3d6f1c40 ("ip6tnl: fix use after free of fb_tnl_dev").

The problem is a bit different in linux-3.10.y, because there is no x-netns
support (upstream commit 0bd8762824e7 ("ip6tnl: add x-netns support")).
When ip6_tunnel.ko is unloaded, FB device is deleted by rtnl_link_unregister()
and then we try to delete it again in ip6_tnl_destroy_tunnels().

This patch removes the second deletion.

Reported-by: Steven Rostedt <rostedt@goodmis.org>
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv6/ip6_tunnel.c | 2 --
 1 file changed, 2 deletions(-)
David Miller - Dec. 17, 2013, 7:40 p.m.
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Fri, 13 Dec 2013 10:06:35 +0100

> The upstream commit bb8140947a24 ("ip6tnl: allow to use rtnl ops on fb tunnel")
> (backported into linux-3.10.y) left a bug which was fixed upstream by commit
> 1e9f3d6f1c40 ("ip6tnl: fix use after free of fb_tnl_dev").
> 
> The problem is a bit different in linux-3.10.y, because there is no x-netns
> support (upstream commit 0bd8762824e7 ("ip6tnl: add x-netns support")).
> When ip6_tunnel.ko is unloaded, FB device is deleted by rtnl_link_unregister()
> and then we try to delete it again in ip6_tnl_destroy_tunnels().
> 
> This patch removes the second deletion.
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Greg please queue this up for 3.10 -stable if you haven't already.

Thanks a lot.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH - Dec. 17, 2013, 7:54 p.m.
On Tue, Dec 17, 2013 at 02:40:02PM -0500, David Miller wrote:
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Fri, 13 Dec 2013 10:06:35 +0100
> 
> > The upstream commit bb8140947a24 ("ip6tnl: allow to use rtnl ops on fb tunnel")
> > (backported into linux-3.10.y) left a bug which was fixed upstream by commit
> > 1e9f3d6f1c40 ("ip6tnl: fix use after free of fb_tnl_dev").
> > 
> > The problem is a bit different in linux-3.10.y, because there is no x-netns
> > support (upstream commit 0bd8762824e7 ("ip6tnl: add x-netns support")).
> > When ip6_tunnel.ko is unloaded, FB device is deleted by rtnl_link_unregister()
> > and then we try to delete it again in ip6_tnl_destroy_tunnels().
> > 
> > This patch removes the second deletion.
> > 
> > Reported-by: Steven Rostedt <rostedt@goodmis.org>
> > Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> 
> Greg please queue this up for 3.10 -stable if you haven't already.

Thanks, will do.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luis Henriques - Dec. 19, 2013, 10:07 a.m.
On Tue, Dec 17, 2013 at 02:40:02PM -0500, David Miller wrote:
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Fri, 13 Dec 2013 10:06:35 +0100
> 
> > The upstream commit bb8140947a24 ("ip6tnl: allow to use rtnl ops on fb tunnel")
> > (backported into linux-3.10.y) left a bug which was fixed upstream by commit
> > 1e9f3d6f1c40 ("ip6tnl: fix use after free of fb_tnl_dev").
> > 
> > The problem is a bit different in linux-3.10.y, because there is no x-netns
> > support (upstream commit 0bd8762824e7 ("ip6tnl: add x-netns support")).
> > When ip6_tunnel.ko is unloaded, FB device is deleted by rtnl_link_unregister()
> > and then we try to delete it again in ip6_tnl_destroy_tunnels().
> > 
> > This patch removes the second deletion.
> > 
> > Reported-by: Steven Rostedt <rostedt@goodmis.org>
> > Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> 
> Greg please queue this up for 3.10 -stable if you haven't already.

As I'm picking the networking patches into the 3.11 kernel as well, I
believe this fix is also applicable.  I'm queuing it for the 3.11 kernel.

Cheers,
--
Luis


> 
> Thanks a lot.
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Dichtel - Dec. 19, 2013, 10:23 a.m.
Le 19/12/2013 11:07, Luis Henriques a écrit :
> On Tue, Dec 17, 2013 at 02:40:02PM -0500, David Miller wrote:
>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> Date: Fri, 13 Dec 2013 10:06:35 +0100
>>
>>> The upstream commit bb8140947a24 ("ip6tnl: allow to use rtnl ops on fb tunnel")
>>> (backported into linux-3.10.y) left a bug which was fixed upstream by commit
>>> 1e9f3d6f1c40 ("ip6tnl: fix use after free of fb_tnl_dev").
>>>
>>> The problem is a bit different in linux-3.10.y, because there is no x-netns
>>> support (upstream commit 0bd8762824e7 ("ip6tnl: add x-netns support")).
>>> When ip6_tunnel.ko is unloaded, FB device is deleted by rtnl_link_unregister()
>>> and then we try to delete it again in ip6_tnl_destroy_tunnels().
>>>
>>> This patch removes the second deletion.
>>>
>>> Reported-by: Steven Rostedt <rostedt@goodmis.org>
>>> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>
>> Greg please queue this up for 3.10 -stable if you haven't already.
>
> As I'm picking the networking patches into the 3.11 kernel as well, I
> believe this fix is also applicable.  I'm queuing it for the 3.11 kernel.
Yes, I agree.


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

Patch

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 0516ebbea80b..209bb4d6e188 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1711,8 +1711,6 @@  static void __net_exit ip6_tnl_destroy_tunnels(struct ip6_tnl_net *ip6n)
 		}
 	}
 
-	t = rtnl_dereference(ip6n->tnls_wc[0]);
-	unregister_netdevice_queue(t->dev, &list);
 	unregister_netdevice_many(&list);
 }