[PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit

[PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit

[Steffen Klassert]

We might extend the used aera of a skb beyond the total
headroom when we install the ipip header. Fix this by
calling skb_cow_head() unconditionally.

Bug was introduced with commit c544193214
("GRE: Refactor GRE tunneling code.")

Cc: Pravin Shelar <pshelar@nicira.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---

v2: Add a reference to the commit that introduced bug to the commit message

 net/ipv4/ip_tunnel.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index d3fbad4..dfc6d8a 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -642,13 +642,13 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	max_headroom = LL_RESERVED_SPACE(rt->dst.dev) + sizeof(struct iphdr)
 			+ rt->dst.header_len;
-	if (max_headroom > dev->needed_headroom) {
+	if (max_headroom > dev->needed_headroom)
 		dev->needed_headroom = max_headroom;
-		if (skb_cow_head(skb, dev->needed_headroom)) {
-			dev->stats.tx_dropped++;
-			dev_kfree_skb(skb);
-			return;
-		}
+
+	if (skb_cow_head(skb, dev->needed_headroom)) {
+		dev->stats.tx_dropped++;
+		dev_kfree_skb(skb);
+		return;
 	}
 
 	err = iptunnel_xmit(rt, skb, fl4.saddr, fl4.daddr, protocol,
-- 
1.7.9.5

[PATCH net v2 2/4] ip_tunnel: Add fallback tunnels to the hash lists

[Steffen Klassert]

Currently we can not update the tunnel parameters of
the fallback tunnels because we don't find them in the
hash lists. Fix this by adding them on initialization.

Bug was introduced with commit c544193214
("GRE: Refactor GRE tunneling code.")

Cc: Pravin Shelar <pshelar@nicira.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---

v2: Add a reference to the commit that introduced bug to the commit message

 net/ipv4/ip_tunnel.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index dfc6d8a..895a353 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -853,8 +853,10 @@ int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
 	/* FB netdevice is special: we have one, and only one per netns.
 	 * Allowing to move it to another netns is clearly unsafe.
 	 */
-	if (!IS_ERR(itn->fb_tunnel_dev))
+	if (!IS_ERR(itn->fb_tunnel_dev)) {
 		itn->fb_tunnel_dev->features |= NETIF_F_NETNS_LOCAL;
+		ip_tunnel_add(itn, netdev_priv(itn->fb_tunnel_dev));
+	}
 	rtnl_unlock();
 
 	return PTR_RET(itn->fb_tunnel_dev);
-- 
1.7.9.5

[PATCH net 3/4] ip_tunnel_core: Change __skb_push back to skb_push

[Steffen Klassert]

Git commit 0e6fbc5b ("ip_tunnels: extend iptunnel_xmit()")
moved the IP header installation to iptunnel_xmit() and
changed skb_push() to __skb_push(). This makes possible
bugs hard to track down, so change it back to skb_push().

Cc: Pravin Shelar <pshelar@nicira.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/ip_tunnel_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index d6c856b..c31e3ad 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -61,7 +61,7 @@ int iptunnel_xmit(struct rtable *rt, struct sk_buff *skb,
 	memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
 
 	/* Push down and install the IP header. */
-	__skb_push(skb, sizeof(struct iphdr));
+	skb_push(skb, sizeof(struct iphdr));
 	skb_reset_network_header(skb);
 
 	iph = ip_hdr(skb);
-- 
1.7.9.5

[PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device

[Steffen Klassert]

When queueing the netdevices for removal, we queue the
fallback device twice in ip_tunnel_destroy(). The first
time when we queue all netdevices in the namespace and
then again explicitly. Fix this by removing the explicit
queueing of the fallback device.

Bug was introduced when network namespace support was added
with commit 6c742e714d8 ("ipip: add x-netns support").

Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/ip_tunnel.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 895a353..63a6d6d 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -886,8 +886,6 @@ static void ip_tunnel_destroy(struct ip_tunnel_net *itn, struct list_head *head,
 			if (!net_eq(dev_net(t->dev), net))
 				unregister_netdevice_queue(t->dev, head);
 	}
-	if (itn->fb_tunnel_dev)
-		unregister_netdevice_queue(itn->fb_tunnel_dev, head);
 }
 
 void ip_tunnel_delete_net(struct ip_tunnel_net *itn, struct rtnl_link_ops *ops)
-- 
1.7.9.5

Re: [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device

[Nicolas Dichtel]

Le 01/10/2013 11:37, Steffen Klassert a écrit :
> When queueing the netdevices for removal, we queue the
> fallback device twice in ip_tunnel_destroy(). The first
> time when we queue all netdevices in the namespace and
> then again explicitly. Fix this by removing the explicit
> queueing of the fallback device.
>
> Bug was introduced when network namespace support was added
> with commit 6c742e714d8 ("ipip: add x-netns support").
>
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Note that there is the same problem in 
net/ipv6/ip6_tunnel.c:ip6_tnl_destroy_tunnels() introduced by 0bd8762824e7
("ip6tnl: add x-netns support").

Should I send a patch for it or would you take care of it?

Re: [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device

[Steffen Klassert]

On Tue, Oct 01, 2013 at 12:00:44PM +0200, Nicolas Dichtel wrote:
> 
> Note that there is the same problem in
> net/ipv6/ip6_tunnel.c:ip6_tnl_destroy_tunnels() introduced by
> 0bd8762824e7
> ("ip6tnl: add x-netns support").
> 
> Should I send a patch for it or would you take care of it?

I don't mind either way. But you noticed the problem, so
feel free to send a patch yourself.

Re: [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device

[Nicolas Dichtel]

Le 01/10/2013 12:40, Steffen Klassert a écrit :
> On Tue, Oct 01, 2013 at 12:00:44PM +0200, Nicolas Dichtel wrote:
>>
>> Note that there is the same problem in
>> net/ipv6/ip6_tunnel.c:ip6_tnl_destroy_tunnels() introduced by
>> 0bd8762824e7
>> ("ip6tnl: add x-netns support").
>>
>> Should I send a patch for it or would you take care of it?
>
> I don't mind either way. But you noticed the problem, so
> feel free to send a patch yourself.
>
Ok, I will dot it.

Thank you

[PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel

[Nicolas Dichtel]

rtnl ops where introduced by ba3e3f50a0e5 ("sit: advertise tunnel param via
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 queue
in sit_destroy_tunnels() when checking rtnl_link_ops of all netdevices (this
is valid since commit 5e6700b3bf98 ("sit: add support of x-netns")).

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv6/sit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index afd5605aea7c..19269453a8ea 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1666,6 +1666,7 @@ static int __net_init sit_init_net(struct net *net)
 		goto err_alloc_dev;
 	}
 	dev_net_set(sitn->fb_tunnel_dev, net);
+	sitn->fb_tunnel_dev->rtnl_link_ops = &sit_link_ops;
 	/* FB netdevice is special: we have one, and only one per netns.
 	 * Allowing to move it to another netns is clearly unsafe.
 	 */
@@ -1700,7 +1701,6 @@ static void __net_exit sit_exit_net(struct net *net)
 
 	rtnl_lock();
 	sit_destroy_tunnels(sitn, &list);
-	unregister_netdevice_queue(sitn->fb_tunnel_dev, &list);
 	unregister_netdevice_many(&list);
 	rtnl_unlock();
 }
-- 
1.8.2.1

[PATCH net 2/2] ip6tnl: allow to use rtnl ops on fb tunnel

[Nicolas Dichtel]

rtnl ops where introduced by c075b13098b3 ("ip6tnl: advertise tunnel param via
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 queue
in ip6_tnl_destroy_tunnels() when checking rtnl_link_ops of all netdevices (this
is valid since commit 0bd8762824e7 ("ip6tnl: add x-netns support")).

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv6/ip6_tunnel.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 2d8f4829575b..a791552e0422 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1731,8 +1731,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);
 }
 
@@ -1752,6 +1750,7 @@ static int __net_init ip6_tnl_init_net(struct net *net)
 	if (!ip6n->fb_tnl_dev)
 		goto err_alloc_dev;
 	dev_net_set(ip6n->fb_tnl_dev, net);
+	ip6n->fb_tnl_dev->rtnl_link_ops = &ip6_link_ops;
 	/* FB netdevice is special: we have one, and only one per netns.
 	 * Allowing to move it to another netns is clearly unsafe.
 	 */
-- 
1.8.2.1

Re: [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit

[David Miller]

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 1 Oct 2013 11:33:59 +0200

> We might extend the used aera of a skb beyond the total
> headroom when we install the ipip header. Fix this by
> calling skb_cow_head() unconditionally.
> 
> Bug was introduced with commit c544193214
> ("GRE: Refactor GRE tunneling code.")
> 
> Cc: Pravin Shelar <pshelar@nicira.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

Applied and queued up for -stable.

Re: [PATCH net v2 2/4] ip_tunnel: Add fallback tunnels to the hash lists

[David Miller]

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 1 Oct 2013 11:34:48 +0200

> Currently we can not update the tunnel parameters of
> the fallback tunnels because we don't find them in the
> hash lists. Fix this by adding them on initialization.
> 
> Bug was introduced with commit c544193214
> ("GRE: Refactor GRE tunneling code.")
> 
> Cc: Pravin Shelar <pshelar@nicira.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

Applied and queued up for -stable.

Re: [PATCH net 3/4] ip_tunnel_core: Change __skb_push back to skb_push

[David Miller]

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 1 Oct 2013 11:35:51 +0200

> Git commit 0e6fbc5b ("ip_tunnels: extend iptunnel_xmit()")
> moved the IP header installation to iptunnel_xmit() and
> changed skb_push() to __skb_push(). This makes possible
> bugs hard to track down, so change it back to skb_push().
> 
> Cc: Pravin Shelar <pshelar@nicira.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

Applied and queued up for -stable.

Re: [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device

[David Miller]

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 1 Oct 2013 11:37:37 +0200

> When queueing the netdevices for removal, we queue the
> fallback device twice in ip_tunnel_destroy(). The first
> time when we queue all netdevices in the namespace and
> then again explicitly. Fix this by removing the explicit
> queueing of the fallback device.
> 
> Bug was introduced when network namespace support was added
> with commit 6c742e714d8 ("ipip: add x-netns support").
> 
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

Applied and queued up for -stable.

Thanks!

Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue,  1 Oct 2013 18:04:59 +0200

> rtnl ops where introduced by ba3e3f50a0e5 ("sit: advertise tunnel param via
> 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 queue
> in sit_destroy_tunnels() when checking rtnl_link_ops of all netdevices (this
> is valid since commit 5e6700b3bf98 ("sit: add support of x-netns")).
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Applied and queued up for -stable.

But I imagine since the x-netns changes aren't in various -stable
branches this will need to be adjusted a bit?

Re: [PATCH net 2/2] ip6tnl: allow to use rtnl ops on fb tunnel

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue,  1 Oct 2013 18:05:00 +0200

> rtnl ops where introduced by c075b13098b3 ("ip6tnl: advertise tunnel param via
> 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 queue
> in ip6_tnl_destroy_tunnels() when checking rtnl_link_ops of all netdevices (this
> is valid since commit 0bd8762824e7 ("ip6tnl: add x-netns support")).
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Applied and queued up for -stable, and I have similar concerns about
the backport issues.

Thanks.

Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel

[Nicolas Dichtel]

Le 01/10/2013 18:59, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Tue,  1 Oct 2013 18:04:59 +0200
>
>> rtnl ops where introduced by ba3e3f50a0e5 ("sit: advertise tunnel param via
>> 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 queue
>> in sit_destroy_tunnels() when checking rtnl_link_ops of all netdevices (this
>> is valid since commit 5e6700b3bf98 ("sit: add support of x-netns")).
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>
> Applied and queued up for -stable.
>
> But I imagine since the x-netns changes aren't in various -stable
> branches this will need to be adjusted a bit?
Yes, it's what I've tried to say in the commit log ;-)

In fact, before the x-netns changes, we must keep the 
unregister_netdevice_queue() line.

Re: [PATCH net 2/2] ip6tnl: allow to use rtnl ops on fb tunnel

[Nicolas Dichtel]

Le 01/10/2013 18:59, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Tue,  1 Oct 2013 18:05:00 +0200
>
>> rtnl ops where introduced by c075b13098b3 ("ip6tnl: advertise tunnel param via
>> 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 queue
>> in ip6_tnl_destroy_tunnels() when checking rtnl_link_ops of all netdevices (this
>> is valid since commit 0bd8762824e7 ("ip6tnl: add x-netns support")).
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>
> Applied and queued up for -stable, and I have similar concerns about
> the backport issues.
>
> Thanks.
>
Yes, same here: before the x-netns changes, we must keep the 
unregister_netdevice_queue() line.

Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel

[Nicolas Dichtel]

Le 02/10/2013 09:15, Nicolas Dichtel a écrit :
> Le 01/10/2013 18:59, David Miller a écrit :
>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> Date: Tue,  1 Oct 2013 18:04:59 +0200
>>
>>> rtnl ops where introduced by ba3e3f50a0e5 ("sit: advertise tunnel param via
>>> 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 queue
>>> in sit_destroy_tunnels() when checking rtnl_link_ops of all netdevices (this
>>> is valid since commit 5e6700b3bf98 ("sit: add support of x-netns")).
>>>
>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>
>> Applied and queued up for -stable.
Another things about ipip: between 0974658da47c ("ipip: advertise tunnel param
via rtnl", v3.8) and fd58156e456d ("IPIP: Use ip-tunneling code.", v3.10) the
fb device of ipip module has the same problem.
Should I send a patch?

Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Wed, 02 Oct 2013 09:36:02 +0200

> Le 02/10/2013 09:15, Nicolas Dichtel a écrit :
>> Le 01/10/2013 18:59, David Miller a écrit :
>>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>> Date: Tue,  1 Oct 2013 18:04:59 +0200
>>>
>>>> rtnl ops where introduced by ba3e3f50a0e5 ("sit: advertise tunnel
>>>> param via
>>>> 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 queue
>>>> in sit_destroy_tunnels() when checking rtnl_link_ops of all netdevices
>>>> (this
>>>> is valid since commit 5e6700b3bf98 ("sit: add support of x-netns")).
>>>>
>>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>>
>>> Applied and queued up for -stable.
> Another things about ipip: between 0974658da47c ("ipip: advertise
> tunnel param
> via rtnl", v3.8) and fd58156e456d ("IPIP: Use ip-tunneling code.",
> v3.10) the
> fb device of ipip module has the same problem.
> Should I send a patch?

Yes please do, thanks for noticing this.

Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel

[Nicolas Dichtel]

Le 02/10/2013 23:08, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Wed, 02 Oct 2013 09:36:02 +0200
>
>> Le 02/10/2013 09:15, Nicolas Dichtel a écrit :
>>> Le 01/10/2013 18:59, David Miller a écrit :
>>>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>>> Date: Tue,  1 Oct 2013 18:04:59 +0200
>>>>
>>>>> rtnl ops where introduced by ba3e3f50a0e5 ("sit: advertise tunnel
>>>>> param via
>>>>> 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 queue
>>>>> in sit_destroy_tunnels() when checking rtnl_link_ops of all netdevices
>>>>> (this
>>>>> is valid since commit 5e6700b3bf98 ("sit: add support of x-netns")).
>>>>>
>>>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>>>
>>>> Applied and queued up for -stable.
>> Another things about ipip: between 0974658da47c ("ipip: advertise
>> tunnel param
>> via rtnl", v3.8) and fd58156e456d ("IPIP: Use ip-tunneling code.",
>> v3.10) the
>> fb device of ipip module has the same problem.
>> Should I send a patch?
>
> Yes please do, thanks for noticing this.
>
In fact, I just notice that 3.9 branch is EoL (bug is only in 3.8 and 3.9).
Should I still send a patch ? If yes, based on which tree/branch?

Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 03 Oct 2013 10:06:24 +0200

> In fact, I just notice that 3.9 branch is EoL (bug is only in 3.8 and
> 3.9).
> Should I still send a patch ? If yes, based on which tree/branch?

No stable backports are needed then.

Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel

[Willem de Bruijn]

On Wed, Oct 2, 2013 at 3:15 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> Le 01/10/2013 18:59, David Miller a écrit :
>
>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> Date: Tue,  1 Oct 2013 18:04:59 +0200
>>
>>> rtnl ops where introduced by ba3e3f50a0e5 ("sit: advertise tunnel param
>>> via
>>> 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 queue
>>> in sit_destroy_tunnels() when checking rtnl_link_ops of all netdevices
>>> (this
>>> is valid since commit 5e6700b3bf98 ("sit: add support of x-netns")).
>>>
>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>
>>
>> Applied and queued up for -stable.
>>
>> But I imagine since the x-netns changes aren't in various -stable
>> branches this will need to be adjusted a bit?
>
> Yes, it's what I've tried to say in the commit log ;-)
>
> In fact, before the x-netns changes, we must keep the
> unregister_netdevice_queue() line.

In 3.11 linux-stable, this patch was merged between 3.11.4 and 3.11.5
in commit 3783100, after the x-netns changes in commit 5e6700b3bf, but
the unregister_netdevice_queue was kept.

I think that caused the following bug. In 3.11.6, a simple `modprobe
sit && rmmod sit` hits the BUG at net/core/dev.c:5039:

  BUG_ON(dev->reg_state != NETREG_REGISTERED);

The device is actually NETREG_RELEASED at one point because the device
is now unregistered twice. The issue goes away by porting the
remainder of the original commit: the one liner that removes the call
to unregister_netdevice_queue.

+++ b/net/ipv6/sit.c
@@ -1708,7 +1708,6 @@ static void __net_exit sit_exit_net(struct net *net)

        sit_destroy_tunnels(sitn, &list);
-       unregister_netdevice_queue(sitn->fb_tunnel_dev, &list);
        unregister_netdevice_many(&list);

If correct, let me know if I should send a proper one-line patch
against 3.11.y. Since I haven't looked at this code before, I found it
safer to report the issue first.

5e6700b3bf was not applied to 3.10 stable, so that branch is not affected.

Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel

[Nicolas Dichtel]

Le 22/10/2013 01:30, Willem de Bruijn a écrit :
> On Wed, Oct 2, 2013 at 3:15 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>> Le 01/10/2013 18:59, David Miller a écrit :
>>
>>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>> Date: Tue,  1 Oct 2013 18:04:59 +0200
>>>
>>>> rtnl ops where introduced by ba3e3f50a0e5 ("sit: advertise tunnel param
>>>> via
>>>> 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 queue
>>>> in sit_destroy_tunnels() when checking rtnl_link_ops of all netdevices
>>>> (this
>>>> is valid since commit 5e6700b3bf98 ("sit: add support of x-netns")).
>>>>
>>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>>
>>>
>>> Applied and queued up for -stable.
>>>
>>> But I imagine since the x-netns changes aren't in various -stable
>>> branches this will need to be adjusted a bit?
>>
>> Yes, it's what I've tried to say in the commit log ;-)
>>
>> In fact, before the x-netns changes, we must keep the
>> unregister_netdevice_queue() line.
>
> In 3.11 linux-stable, this patch was merged between 3.11.4 and 3.11.5
> in commit 3783100, after the x-netns changes in commit 5e6700b3bf, but
> the unregister_netdevice_queue was kept.
>
> I think that caused the following bug. In 3.11.6, a simple `modprobe
> sit && rmmod sit` hits the BUG at net/core/dev.c:5039:
>
>    BUG_ON(dev->reg_state != NETREG_REGISTERED);
>
> The device is actually NETREG_RELEASED at one point because the device
> is now unregistered twice. The issue goes away by porting the
> remainder of the original commit: the one liner that removes the call
> to unregister_netdevice_queue.
>
> +++ b/net/ipv6/sit.c
> @@ -1708,7 +1708,6 @@ static void __net_exit sit_exit_net(struct net *net)
>
>          sit_destroy_tunnels(sitn, &list);
> -       unregister_netdevice_queue(sitn->fb_tunnel_dev, &list);
>          unregister_netdevice_many(&list);
>
> If correct, let me know if I should send a proper one-line patch
> against 3.11.y. Since I haven't looked at this code before, I found it
> safer to report the issue first.
Yes, this line should be removed, like it was done in the original patch
(x-netns for sit is part of 3.11).

>
> 5e6700b3bf was not applied to 3.10 stable, so that branch is not affected.
Right.