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

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

[Steven Rostedt]

In our test labs we discovered a bug with the latest 3.10-rt kernel.
When investigating, I found that it was actually a bug in the 3.10.18
kernel that we based on. With that, I bisected it down to this commit:

commit 506cdb8909a1a739c7585c680c6bd4b3d1247564
Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date:   Tue Oct 1 18:05:00 2013 +0200

    ip6tnl: allow to use rtnl ops on fb tunnel
    
    [ Upstream commit bb8140947a247b9aa15652cc24dc555ebb0b64b0 ]
    
    rtnl ops where introduced by c075b13098b3 ("ip6tnl: advertise tunnel param
    rtnl"), but I forget to assign rtnl ops to fb tunnels.
    
    Now that it is done, we must remove the explicit call to
    unregister_netdevice_queue(), because  the fallback tunnel is added to the
    in ip6_tnl_destroy_tunnels() when checking rtnl_link_ops of all netdevices
    is valid since commit 0bd8762824e7 ("ip6tnl: add x-netns support")).


The bug we see is caused by simply loading and unloading the ip6_tunnel
module.

	modprobe ip6_tunnel; rmmod ip6_tunnel

Which causes the following oops:

[   43.423028] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
[   43.424010] IP: [<ffffffffa0534f51>] ip6_tnl_exit_net+0x71/0x93 [ip6_tunnel]
[   43.424010] PGD 776f4067 PUD 7810a067 PMD 0 
[   43.424010] Oops: 0000 [#1] PREEMPT SMP 
[   43.424010] Modules linked in: ip6_tunnel(-) tunnel6 ipt_MASQUERADE sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables uinput snd_hda_codec_idt kvm_i
ntel snd_hda_intel snd_hda_codec kvm snd_hwdep snd_seq snd_seq_device snd_pcm snd_page_alloc snd_timer snd shpchp i2c_i801 soundcore microcode pata_acpi firewire_ohci firewire_core
 crc_itu_t ata_generic i915 drm_kms_helper drm i2c_algo_bit i2c_core video
[   43.424010] CPU: 1 PID: 2731 Comm: rmmod Not tainted 3.10.15-test+ #105
[   43.424010] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS SDBLI944.86P 05/08/2007
[   43.424010] task: ffff880078b01460 ti: ffff880077bf4000 task.ti: ffff880077bf4000
[   43.424010] RIP: 0010:[<ffffffffa0534f51>]  [<ffffffffa0534f51>] ip6_tnl_exit_net+0x71/0x93 [ip6_tunnel]
[   43.424010] RSP: 0018:ffff880077bf5e08  EFLAGS: 00010246
[   43.424010] RAX: 0000000000000000 RBX: 0000000000000100 RCX: 0000000000000003
[   43.424010] RDX: ffff88007d480000 RSI: ffff880077bf5e08 RDI: ffff880077bf4000
[   43.424010] RBP: ffff880077bf5e38 R08: ffff880077bf5d68 R09: ffffffff81aa20d0
[   43.424010] R10: ffffffff81aa20d0 R11: ffffffff81aa20d0 R12: 0000000000000000
[   43.424010] R13: ffff88007794b400 R14: ffff880077bf5e08 R15: 0000000000000001
[   43.424010] FS:  00007fbc2ee27700(0000) GS:ffff88007d480000(0000) knlGS:0000000000000000
[   43.424010] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[   43.424010] CR2: 0000000000000008 CR3: 0000000077bd0000 CR4: 00000000000007e0
[   43.424010] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   43.424010] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   43.424010] Stack:
[   43.424010]  ffff880077bf5e08 ffff880077bf5e08 ffffffffa0536f50 ffffffff81aa0900
[   43.424010]  ffff880077bf5e78 0000000000000000 ffff880077bf5e68 ffffffff81408df4
[   43.424010]  0000000000000000 ffffffffa0536f50 ffffffff81aa1820 ffff880077bf5e78
[   43.424010] Call Trace:
[   43.424010]  [<ffffffff81408df4>] ops_exit_list+0x27/0x50
[   43.424010]  [<ffffffff814090ba>] unregister_pernet_operations+0x61/0x93
[   43.424010]  [<ffffffff81409122>] unregister_pernet_device+0x36/0x47
[   43.424010]  [<ffffffffa05367d4>] ip6_tunnel_cleanup+0x70/0x72 [ip6_tunnel]
[   43.424010]  [<ffffffff81083ef5>] SyS_delete_module+0x20b/0x27d
[   43.424010]  [<ffffffff81244cae>] ? trace_hardirqs_on_thunk+0x3a/0x3c
[   43.424010]  [<ffffffff81503902>] system_call_fastpath+0x16/0x1b
[   43.424010] Code: 24 08 4c 89 f6 e8 8c 88 ed e0 4d 8b 24 24 4d 85 e4 75 ea 48 83 c3 08 48 81 fb 00 01 00 00 75 d6 49 8b 85 08 01 00 00 48 8d 75 d0 <48> 8b 78 08 e8 62 88 ed e0 48 8d 7d d0 e8 84 77 ed e0 e8 90 62 
[   43.424010] RIP  [<ffffffffa0534f51>] ip6_tnl_exit_net+0x71/0x93 [ip6_tunnel]
[   43.424010]  RSP <ffff880077bf5e08>
[   43.424010] CR2: 0000000000000008
[   43.708059] ---[ end trace ea2c125633de7c64 ]---


(gdb) li *ip6_tnl_exit_net+0x71
0xf51 is in ip6_tnl_exit_net (/home/rostedt/work/git/linux-trace.git/net/ipv6/ip6_tunnel.c:1715).
1710                            t = rtnl_dereference(t->next);
1711                    }
1712            }
1713
1714            t = rtnl_dereference(ip6n->tnls_wc[0]);
1715            unregister_netdevice_queue(t->dev, &list);
1716            unregister_netdevice_many(&list);
1717    }
1718
1719    static int __net_init ip6_tnl_init_net(struct net *net)

Thus, this got called with ip6n->tnsl_wc[0] as NULL.

I ran the following trace command on this:

# modprobe ip6_tunnel
# trace-cmd start -p function_graph -g SyS_delete_module
# rmmod ip6_tunnel

and traced the flow of functions that lead up to the crash: Full dump
can be found here: http://rostedt.homelinux.com/private/ip6_tunnel.trace


ip6_tnl_dev_uninit() which is called by rollback_registered_many() sets
tnls_wc[0] to NULL. Later unregistered_pernet_device() gets called,
which eventually calls ip6_tnl_exit_net() which references the
tnls_wc[0] unconditionally. This looks to be where the bug happens.

Finally, after digging through all this, I looked at the original
commit that was backported to 3.10 and noticed that the backport
doesn't include the entire change. It also has:

+++ b/net/ipv6/ip6_tunnel.c
@@ -1731,8 +1731,6 @@ static void __net_exit ip6_tnl_destroy_tunnels(struct ip
                }
        }
 
-       t = rtnl_dereference(ip6n->tnls_wc[0]);
-       unregister_netdevice_queue(t->dev, &list);
        unregister_netdevice_many(&list);
 }
 

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?

-- Steve

[PATCH net] ip6tnl: fix use after free of fb_tnl_dev

[Nicolas Dichtel]

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@google.com> for
sit interfaces.

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

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 583b77e2f69b..c1e11b5d6ccc 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1635,6 +1635,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
@@ -1699,6 +1708,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,
 };
@@ -1715,9 +1725,9 @@ static struct xfrm6_tunnel ip6ip6_handler __read_mostly = {
 	.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)
 {
-	struct net *net = dev_net(ip6n->fb_tnl_dev);
+	struct ip6_tnl_net *ip6n = net_generic(net, ip6_tnl_net_id);
 	struct net_device *dev, *aux;
 	int h;
 	struct ip6_tnl *t;
@@ -1785,10 +1795,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();
 }
 
-- 
1.8.4.1

Re: [PATCH net] ip6tnl: fix use after free of fb_tnl_dev

[Willem de Bruijn]

On Thu, Nov 14, 2013 at 9:47 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> 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@google.com> for
> sit interfaces.
>
> CC: Willem de Bruijn <willemb@google.com>
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Acked-by: Willem de Bruijn <willemb@google.com>

Also ran a test similar to the one for sit: `modprobe ip6_tunnel;
rmmod ip6_tunnel` with CONFIG_DEBUG_SLAB=y. This exposed the bug at
HEAD, completes successfully with the patch applied.

Re: [PATCH net] ip6tnl: fix use after free of fb_tnl_dev

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 14 Nov 2013 15:47:03 +0100

> 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@google.com> for
> sit interfaces.
> 
> CC: Willem de Bruijn <willemb@google.com>
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Applied and queued up for -stable, thanks for being so proactive about this
Nicolas.

Re: [PATCH net] ip6tnl: fix use after free of fb_tnl_dev

[Steven Rostedt]

On Thu, 14 Nov 2013 17:05:29 -0500 (EST)
David Miller <davem@davemloft.net> wrote:

 
> Applied and queued up for -stable, thanks for being so proactive about this
> Nicolas.

I should also note that I added this back to 3.10.18 and the bug goes
away.

If it's not too late:

Tested-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

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

[Greg Kroah-Hartman]

On Wed, Nov 13, 2013 at 09:14:30PM -0500, Steven Rostedt wrote:
> In our test labs we discovered a bug with the latest 3.10-rt kernel.
> When investigating, I found that it was actually a bug in the 3.10.18
> kernel that we based on. With that, I bisected it down to this commit:
> 
> commit 506cdb8909a1a739c7585c680c6bd4b3d1247564
> Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date:   Tue Oct 1 18:05:00 2013 +0200
> 
>     ip6tnl: allow to use rtnl ops on fb tunnel
>     
>     [ Upstream commit bb8140947a247b9aa15652cc24dc555ebb0b64b0 ]
>     
>     rtnl ops where introduced by c075b13098b3 ("ip6tnl: advertise tunnel param
>     rtnl"), but I forget to assign rtnl ops to fb tunnels.
>     
>     Now that it is done, we must remove the explicit call to
>     unregister_netdevice_queue(), because  the fallback tunnel is added to the
>     in ip6_tnl_destroy_tunnels() when checking rtnl_link_ops of all netdevices
>     is valid since commit 0bd8762824e7 ("ip6tnl: add x-netns support")).
> 
> 
> The bug we see is caused by simply loading and unloading the ip6_tunnel
> module.
> 
> 	modprobe ip6_tunnel; rmmod ip6_tunnel
> 
> Which causes the following oops:
> 
> [   43.423028] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
> [   43.424010] IP: [<ffffffffa0534f51>] ip6_tnl_exit_net+0x71/0x93 [ip6_tunnel]
> [   43.424010] PGD 776f4067 PUD 7810a067 PMD 0 
> [   43.424010] Oops: 0000 [#1] PREEMPT SMP 
> [   43.424010] Modules linked in: ip6_tunnel(-) tunnel6 ipt_MASQUERADE sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables uinput snd_hda_codec_idt kvm_i
> ntel snd_hda_intel snd_hda_codec kvm snd_hwdep snd_seq snd_seq_device snd_pcm snd_page_alloc snd_timer snd shpchp i2c_i801 soundcore microcode pata_acpi firewire_ohci firewire_core
>  crc_itu_t ata_generic i915 drm_kms_helper drm i2c_algo_bit i2c_core video
> [   43.424010] CPU: 1 PID: 2731 Comm: rmmod Not tainted 3.10.15-test+ #105
> [   43.424010] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS SDBLI944.86P 05/08/2007
> [   43.424010] task: ffff880078b01460 ti: ffff880077bf4000 task.ti: ffff880077bf4000
> [   43.424010] RIP: 0010:[<ffffffffa0534f51>]  [<ffffffffa0534f51>] ip6_tnl_exit_net+0x71/0x93 [ip6_tunnel]
> [   43.424010] RSP: 0018:ffff880077bf5e08  EFLAGS: 00010246
> [   43.424010] RAX: 0000000000000000 RBX: 0000000000000100 RCX: 0000000000000003
> [   43.424010] RDX: ffff88007d480000 RSI: ffff880077bf5e08 RDI: ffff880077bf4000
> [   43.424010] RBP: ffff880077bf5e38 R08: ffff880077bf5d68 R09: ffffffff81aa20d0
> [   43.424010] R10: ffffffff81aa20d0 R11: ffffffff81aa20d0 R12: 0000000000000000
> [   43.424010] R13: ffff88007794b400 R14: ffff880077bf5e08 R15: 0000000000000001
> [   43.424010] FS:  00007fbc2ee27700(0000) GS:ffff88007d480000(0000) knlGS:0000000000000000
> [   43.424010] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [   43.424010] CR2: 0000000000000008 CR3: 0000000077bd0000 CR4: 00000000000007e0
> [   43.424010] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [   43.424010] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [   43.424010] Stack:
> [   43.424010]  ffff880077bf5e08 ffff880077bf5e08 ffffffffa0536f50 ffffffff81aa0900
> [   43.424010]  ffff880077bf5e78 0000000000000000 ffff880077bf5e68 ffffffff81408df4
> [   43.424010]  0000000000000000 ffffffffa0536f50 ffffffff81aa1820 ffff880077bf5e78
> [   43.424010] Call Trace:
> [   43.424010]  [<ffffffff81408df4>] ops_exit_list+0x27/0x50
> [   43.424010]  [<ffffffff814090ba>] unregister_pernet_operations+0x61/0x93
> [   43.424010]  [<ffffffff81409122>] unregister_pernet_device+0x36/0x47
> [   43.424010]  [<ffffffffa05367d4>] ip6_tunnel_cleanup+0x70/0x72 [ip6_tunnel]
> [   43.424010]  [<ffffffff81083ef5>] SyS_delete_module+0x20b/0x27d
> [   43.424010]  [<ffffffff81244cae>] ? trace_hardirqs_on_thunk+0x3a/0x3c
> [   43.424010]  [<ffffffff81503902>] system_call_fastpath+0x16/0x1b
> [   43.424010] Code: 24 08 4c 89 f6 e8 8c 88 ed e0 4d 8b 24 24 4d 85 e4 75 ea 48 83 c3 08 48 81 fb 00 01 00 00 75 d6 49 8b 85 08 01 00 00 48 8d 75 d0 <48> 8b 78 08 e8 62 88 ed e0 48 8d 7d d0 e8 84 77 ed e0 e8 90 62 
> [   43.424010] RIP  [<ffffffffa0534f51>] ip6_tnl_exit_net+0x71/0x93 [ip6_tunnel]
> [   43.424010]  RSP <ffff880077bf5e08>
> [   43.424010] CR2: 0000000000000008
> [   43.708059] ---[ end trace ea2c125633de7c64 ]---
> 
> 
> (gdb) li *ip6_tnl_exit_net+0x71
> 0xf51 is in ip6_tnl_exit_net (/home/rostedt/work/git/linux-trace.git/net/ipv6/ip6_tunnel.c:1715).
> 1710                            t = rtnl_dereference(t->next);
> 1711                    }
> 1712            }
> 1713
> 1714            t = rtnl_dereference(ip6n->tnls_wc[0]);
> 1715            unregister_netdevice_queue(t->dev, &list);
> 1716            unregister_netdevice_many(&list);
> 1717    }
> 1718
> 1719    static int __net_init ip6_tnl_init_net(struct net *net)
> 
> Thus, this got called with ip6n->tnsl_wc[0] as NULL.
> 
> I ran the following trace command on this:
> 
> # modprobe ip6_tunnel
> # trace-cmd start -p function_graph -g SyS_delete_module
> # rmmod ip6_tunnel
> 
> and traced the flow of functions that lead up to the crash: Full dump
> can be found here: http://rostedt.homelinux.com/private/ip6_tunnel.trace
> 
> 
> ip6_tnl_dev_uninit() which is called by rollback_registered_many() sets
> tnls_wc[0] to NULL. Later unregistered_pernet_device() gets called,
> which eventually calls ip6_tnl_exit_net() which references the
> tnls_wc[0] unconditionally. This looks to be where the bug happens.
> 
> Finally, after digging through all this, I looked at the original
> commit that was backported to 3.10 and noticed that the backport
> doesn't include the entire change. It also has:
> 
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -1731,8 +1731,6 @@ static void __net_exit ip6_tnl_destroy_tunnels(struct ip
>                 }
>         }
>  
> -       t = rtnl_dereference(ip6n->tnls_wc[0]);
> -       unregister_netdevice_queue(t->dev, &list);
>         unregister_netdevice_many(&list);
>  }
>  
> 
> 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?

thanks,

greg k-h

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

[Steven Rostedt]

On Sun, 8 Dec 2013 16:25:31 -0800
Greg Kroah-Hartman <gregkh@linuxfoundation.org> 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@google.com> for
sit interfaces.

CC: Willem de Bruijn <willemb@google.com>
Reported-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 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();
 }
 

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

[Steven Rostedt]

On Mon, 9 Dec 2013 09:48:50 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
sit interfaces.
> 
> CC: Willem de Bruijn <willemb@google.com>
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Note, this is missing:

    Acked-by: Willem de Bruijn <willemb@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

from the real commit. That's because I backported the original email,
not the commit that was added to git.

-- Steve

> ---
>  net/ipv6/ip6_tunnel.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 

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

[David Miller]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Sun, 8 Dec 2013 16:25:31 -0800

> On Wed, Nov 13, 2013 at 09:14:30PM -0500, Steven Rostedt wrote:
>> +++ b/net/ipv6/ip6_tunnel.c
>> @@ -1731,8 +1731,6 @@ static void __net_exit ip6_tnl_destroy_tunnels(struct ip
>>                 }
>>         }
>>  
>> -       t = rtnl_dereference(ip6n->tnls_wc[0]);
>> -       unregister_netdevice_queue(t->dev, &list);
>>         unregister_netdevice_many(&list);
>>  }
>>  
>> 
>> 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?

The original patch submitted told me to leave this part of the patch
out of the backport, explaining that it wasn't necessary in older
kernels.

Can someone please sort this out?

Nicolas please provide some guidance here, thanks.

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

[Nicolas Dichtel]

Le 11/12/2013 22:53, David Miller a écrit :
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Date: Sun, 8 Dec 2013 16:25:31 -0800
>
>> On Wed, Nov 13, 2013 at 09:14:30PM -0500, Steven Rostedt wrote:
>>> +++ b/net/ipv6/ip6_tunnel.c
>>> @@ -1731,8 +1731,6 @@ static void __net_exit ip6_tnl_destroy_tunnels(struct ip
>>>                  }
>>>          }
>>>
>>> -       t = rtnl_dereference(ip6n->tnls_wc[0]);
>>> -       unregister_netdevice_queue(t->dev, &list);
>>>          unregister_netdevice_many(&list);
>>>   }
>>>
>>>
>>> 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?
>
> The original patch submitted told me to leave this part of the patch
> out of the backport, explaining that it wasn't necessary in older
> kernels.
Yes, and this was right (in upstream commit, I remove this part because
the fb device is deleted by the loop which check dev->rtnl_ops) , but ...

>
> Can someone please sort this out?
>
> Nicolas please provide some guidance here, thanks.
The original patch left a bug, which was fixed upstream with this commit:
1e9f3d6f1c40 ip6tnl: fix use after free of fb_tnl_dev

The problem is a bit different in 3.10.y, because there is no x-vrf 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().
Thus the fix is different and in fact, the above patch is good.

Steven, will you submit this patch properly or should I do this?

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

[Nicolas Dichtel]

Le 11/12/2013 22:53, David Miller a �crit :
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Date: Sun, 8 Dec 2013 16:25:31 -0800
>
>> On Wed, Nov 13, 2013 at 09:14:30PM -0500, Steven Rostedt wrote:
>>> +++ b/net/ipv6/ip6_tunnel.c
>>> @@ -1731,8 +1731,6 @@ static void __net_exit ip6_tnl_destroy_tunnels(struct ip
>>>                  }
>>>          }
>>>
>>> -       t = rtnl_dereference(ip6n->tnls_wc[0]);
>>> -       unregister_netdevice_queue(t->dev, &list);
>>>          unregister_netdevice_many(&list);
>>>   }
>>>
>>>
>>> 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?
>
> The original patch submitted told me to leave this part of the patch
> out of the backport, explaining that it wasn't necessary in older
> kernels.
Yes, and this was right (in upstream commit, I remove this part because
the fb device is deleted by the loop which check dev->rtnl_ops) , but ...

>
> Can someone please sort this out?
>
> Nicolas please provide some guidance here, thanks.
The original patch left a bug, which was fixed upstream with this commit:
1e9f3d6f1c40 ip6tnl: fix use after free of fb_tnl_dev

The problem is a bit different in 3.10.y, because there is no x-vrf 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().
Thus the fix is different and in fact, the above patch is good.

Steven, will you submit this patch properly or should I do this?

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

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 12 Dec 2013 10:53:49 +0100

> Steven, will you submit this patch properly or should I do this?

Nicolas please submit the change, thanks.

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

[Nicolas Dichtel]

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(-)

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);
 }
 
-- 
1.8.4.1

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

[David Miller]

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.

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

[Greg KH]

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

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

[Luis Henriques]

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

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

[Nicolas Dichtel]

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

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

[Nicolas Dichtel]

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