* [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit
@ 2013-10-01 9:33 Steffen Klassert
2013-10-01 9:34 ` [PATCH net v2 2/4] ip_tunnel: Add fallback tunnels to the hash lists Steffen Klassert
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Steffen Klassert @ 2013-10-01 9:33 UTC (permalink / raw)
To: David Miller; +Cc: Pravin Shelar, netdev
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH net v2 2/4] ip_tunnel: Add fallback tunnels to the hash lists 2013-10-01 9:33 [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit Steffen Klassert @ 2013-10-01 9:34 ` Steffen Klassert 2013-10-01 16:43 ` David Miller 2013-10-01 9:35 ` [PATCH net 3/4] ip_tunnel_core: Change __skb_push back to skb_push Steffen Klassert ` (2 subsequent siblings) 3 siblings, 1 reply; 23+ messages in thread From: Steffen Klassert @ 2013-10-01 9:34 UTC (permalink / raw) To: David Miller; +Cc: Pravin Shelar, netdev 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH net v2 2/4] ip_tunnel: Add fallback tunnels to the hash lists 2013-10-01 9:34 ` [PATCH net v2 2/4] ip_tunnel: Add fallback tunnels to the hash lists Steffen Klassert @ 2013-10-01 16:43 ` David Miller 0 siblings, 0 replies; 23+ messages in thread From: David Miller @ 2013-10-01 16:43 UTC (permalink / raw) To: steffen.klassert; +Cc: pshelar, netdev 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH net 3/4] ip_tunnel_core: Change __skb_push back to skb_push 2013-10-01 9:33 [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit Steffen Klassert 2013-10-01 9:34 ` [PATCH net v2 2/4] ip_tunnel: Add fallback tunnels to the hash lists Steffen Klassert @ 2013-10-01 9:35 ` Steffen Klassert 2013-10-01 16:43 ` David Miller 2013-10-01 9:37 ` [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device Steffen Klassert 2013-10-01 16:43 ` [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit David Miller 3 siblings, 1 reply; 23+ messages in thread From: Steffen Klassert @ 2013-10-01 9:35 UTC (permalink / raw) To: David Miller; +Cc: Pravin Shelar, netdev 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH net 3/4] ip_tunnel_core: Change __skb_push back to skb_push 2013-10-01 9:35 ` [PATCH net 3/4] ip_tunnel_core: Change __skb_push back to skb_push Steffen Klassert @ 2013-10-01 16:43 ` David Miller 0 siblings, 0 replies; 23+ messages in thread From: David Miller @ 2013-10-01 16:43 UTC (permalink / raw) To: steffen.klassert; +Cc: pshelar, netdev 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device 2013-10-01 9:33 [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit Steffen Klassert 2013-10-01 9:34 ` [PATCH net v2 2/4] ip_tunnel: Add fallback tunnels to the hash lists Steffen Klassert 2013-10-01 9:35 ` [PATCH net 3/4] ip_tunnel_core: Change __skb_push back to skb_push Steffen Klassert @ 2013-10-01 9:37 ` Steffen Klassert 2013-10-01 10:00 ` Nicolas Dichtel 2013-10-01 16:43 ` [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device David Miller 2013-10-01 16:43 ` [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit David Miller 3 siblings, 2 replies; 23+ messages in thread From: Steffen Klassert @ 2013-10-01 9:37 UTC (permalink / raw) To: David Miller; +Cc: Pravin Shelar, Nicolas Dichtel, netdev 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device 2013-10-01 9:37 ` [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device Steffen Klassert @ 2013-10-01 10:00 ` Nicolas Dichtel 2013-10-01 10:40 ` Steffen Klassert 2013-10-01 16:43 ` [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device David Miller 1 sibling, 1 reply; 23+ messages in thread From: Nicolas Dichtel @ 2013-10-01 10:00 UTC (permalink / raw) To: Steffen Klassert; +Cc: David Miller, Pravin Shelar, netdev 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? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device 2013-10-01 10:00 ` Nicolas Dichtel @ 2013-10-01 10:40 ` Steffen Klassert 2013-10-01 11:30 ` Nicolas Dichtel 0 siblings, 1 reply; 23+ messages in thread From: Steffen Klassert @ 2013-10-01 10:40 UTC (permalink / raw) To: Nicolas Dichtel; +Cc: David Miller, Pravin Shelar, netdev 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device 2013-10-01 10:40 ` Steffen Klassert @ 2013-10-01 11:30 ` Nicolas Dichtel 2013-10-01 16:04 ` [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel Nicolas Dichtel 0 siblings, 1 reply; 23+ messages in thread From: Nicolas Dichtel @ 2013-10-01 11:30 UTC (permalink / raw) To: Steffen Klassert; +Cc: David Miller, Pravin Shelar, netdev 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel 2013-10-01 11:30 ` Nicolas Dichtel @ 2013-10-01 16:04 ` Nicolas Dichtel 2013-10-01 16:05 ` [PATCH net 2/2] ip6tnl: " Nicolas Dichtel 2013-10-01 16:59 ` [PATCH net 1/2] sit: " David Miller 0 siblings, 2 replies; 23+ messages in thread From: Nicolas Dichtel @ 2013-10-01 16:04 UTC (permalink / raw) To: davem; +Cc: netdev, steffen.klassert, pshelar, 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH net 2/2] ip6tnl: allow to use rtnl ops on fb tunnel 2013-10-01 16:04 ` [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel Nicolas Dichtel @ 2013-10-01 16:05 ` Nicolas Dichtel 2013-10-01 16:59 ` David Miller 2013-10-01 16:59 ` [PATCH net 1/2] sit: " David Miller 1 sibling, 1 reply; 23+ messages in thread From: Nicolas Dichtel @ 2013-10-01 16:05 UTC (permalink / raw) To: davem; +Cc: netdev, steffen.klassert, pshelar, 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH net 2/2] ip6tnl: allow to use rtnl ops on fb tunnel 2013-10-01 16:05 ` [PATCH net 2/2] ip6tnl: " Nicolas Dichtel @ 2013-10-01 16:59 ` David Miller 2013-10-02 7:16 ` Nicolas Dichtel 0 siblings, 1 reply; 23+ messages in thread From: David Miller @ 2013-10-01 16:59 UTC (permalink / raw) To: nicolas.dichtel; +Cc: netdev, steffen.klassert, pshelar 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 2/2] ip6tnl: allow to use rtnl ops on fb tunnel 2013-10-01 16:59 ` David Miller @ 2013-10-02 7:16 ` Nicolas Dichtel 0 siblings, 0 replies; 23+ messages in thread From: Nicolas Dichtel @ 2013-10-02 7:16 UTC (permalink / raw) To: David Miller; +Cc: netdev, steffen.klassert, pshelar 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel 2013-10-01 16:04 ` [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel Nicolas Dichtel 2013-10-01 16:05 ` [PATCH net 2/2] ip6tnl: " Nicolas Dichtel @ 2013-10-01 16:59 ` David Miller 2013-10-02 7:15 ` Nicolas Dichtel 1 sibling, 1 reply; 23+ messages in thread From: David Miller @ 2013-10-01 16:59 UTC (permalink / raw) To: nicolas.dichtel; +Cc: netdev, steffen.klassert, pshelar 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? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel 2013-10-01 16:59 ` [PATCH net 1/2] sit: " David Miller @ 2013-10-02 7:15 ` Nicolas Dichtel 2013-10-02 7:36 ` Nicolas Dichtel 2013-10-21 23:30 ` Willem de Bruijn 0 siblings, 2 replies; 23+ messages in thread From: Nicolas Dichtel @ 2013-10-02 7:15 UTC (permalink / raw) To: David Miller; +Cc: netdev, steffen.klassert, pshelar 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel 2013-10-02 7:15 ` Nicolas Dichtel @ 2013-10-02 7:36 ` Nicolas Dichtel 2013-10-02 21:08 ` David Miller 2013-10-21 23:30 ` Willem de Bruijn 1 sibling, 1 reply; 23+ messages in thread From: Nicolas Dichtel @ 2013-10-02 7:36 UTC (permalink / raw) To: David Miller; +Cc: netdev, steffen.klassert, pshelar 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? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel 2013-10-02 7:36 ` Nicolas Dichtel @ 2013-10-02 21:08 ` David Miller 2013-10-03 8:06 ` Nicolas Dichtel 0 siblings, 1 reply; 23+ messages in thread From: David Miller @ 2013-10-02 21:08 UTC (permalink / raw) To: nicolas.dichtel; +Cc: netdev, steffen.klassert, pshelar 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel 2013-10-02 21:08 ` David Miller @ 2013-10-03 8:06 ` Nicolas Dichtel 2013-10-03 17:37 ` David Miller 0 siblings, 1 reply; 23+ messages in thread From: Nicolas Dichtel @ 2013-10-03 8:06 UTC (permalink / raw) To: David Miller; +Cc: netdev, steffen.klassert, pshelar 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? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel 2013-10-03 8:06 ` Nicolas Dichtel @ 2013-10-03 17:37 ` David Miller 0 siblings, 0 replies; 23+ messages in thread From: David Miller @ 2013-10-03 17:37 UTC (permalink / raw) To: nicolas.dichtel; +Cc: netdev, steffen.klassert, pshelar 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel 2013-10-02 7:15 ` Nicolas Dichtel 2013-10-02 7:36 ` Nicolas Dichtel @ 2013-10-21 23:30 ` Willem de Bruijn 2013-10-22 7:34 ` Nicolas Dichtel 1 sibling, 1 reply; 23+ messages in thread From: Willem de Bruijn @ 2013-10-21 23:30 UTC (permalink / raw) To: nicolas.dichtel Cc: David Miller, netdev, steffen.klassert, pshelar, gregkh, stable 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel 2013-10-21 23:30 ` Willem de Bruijn @ 2013-10-22 7:34 ` Nicolas Dichtel 0 siblings, 0 replies; 23+ messages in thread From: Nicolas Dichtel @ 2013-10-22 7:34 UTC (permalink / raw) To: Willem de Bruijn Cc: David Miller, netdev, steffen.klassert, pshelar, gregkh, stable 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device 2013-10-01 9:37 ` [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device Steffen Klassert 2013-10-01 10:00 ` Nicolas Dichtel @ 2013-10-01 16:43 ` David Miller 1 sibling, 0 replies; 23+ messages in thread From: David Miller @ 2013-10-01 16:43 UTC (permalink / raw) To: steffen.klassert; +Cc: pshelar, nicolas.dichtel, netdev 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! ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit 2013-10-01 9:33 [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit Steffen Klassert ` (2 preceding siblings ...) 2013-10-01 9:37 ` [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device Steffen Klassert @ 2013-10-01 16:43 ` David Miller 3 siblings, 0 replies; 23+ messages in thread From: David Miller @ 2013-10-01 16:43 UTC (permalink / raw) To: steffen.klassert; +Cc: pshelar, netdev 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2013-10-22 7:34 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-01 9:33 [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit Steffen Klassert 2013-10-01 9:34 ` [PATCH net v2 2/4] ip_tunnel: Add fallback tunnels to the hash lists Steffen Klassert 2013-10-01 16:43 ` David Miller 2013-10-01 9:35 ` [PATCH net 3/4] ip_tunnel_core: Change __skb_push back to skb_push Steffen Klassert 2013-10-01 16:43 ` David Miller 2013-10-01 9:37 ` [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device Steffen Klassert 2013-10-01 10:00 ` Nicolas Dichtel 2013-10-01 10:40 ` Steffen Klassert 2013-10-01 11:30 ` Nicolas Dichtel 2013-10-01 16:04 ` [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel Nicolas Dichtel 2013-10-01 16:05 ` [PATCH net 2/2] ip6tnl: " Nicolas Dichtel 2013-10-01 16:59 ` David Miller 2013-10-02 7:16 ` Nicolas Dichtel 2013-10-01 16:59 ` [PATCH net 1/2] sit: " David Miller 2013-10-02 7:15 ` Nicolas Dichtel 2013-10-02 7:36 ` Nicolas Dichtel 2013-10-02 21:08 ` David Miller 2013-10-03 8:06 ` Nicolas Dichtel 2013-10-03 17:37 ` David Miller 2013-10-21 23:30 ` Willem de Bruijn 2013-10-22 7:34 ` Nicolas Dichtel 2013-10-01 16:43 ` [PATCH net 4/4] ip_tunnel: Remove double unregister of the fallback device David Miller 2013-10-01 16:43 ` [PATCH net v2 1/4] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).