Bug#599816: Nested GRE locking bug

2010-10-13 Thread Ben Hutchings
Beatrice Barbe reported a reproducible crash after creating large
numbers of nested GRE tunnels and then pinging with the source address
forced.  I was able to reproduce this using net-2.6.  I'm attaching the
kernel config I used and a script to reproduce this based on the script
she provided.  The magic number of tunnels to create is apparently 37.

With lockdep enabled, I get the following output:

=
[ INFO: possible recursive locking detected ]
2.6.36-rc7-00040-gb0057c5 #5
-
ping/2199 is trying to acquire lock:
 (_xmit_IPGRE){+.}, at: [] dev_queue_xmit+0x37e/0x454

but task is already holding lock:
 (_xmit_IPGRE){+.}, at: [] dev_queue_xmit+0x37e/0x454

other info that might help us debug this:
4 locks held by ping/2199:
 #0:  (sk_lock-AF_INET){+.+.+.}, at: [] raw_sendmsg+0x590/0x64c
 #1:  (rcu_read_lock_bh){.+}, at: [] dev_queue_xmit+0x0/0x454
 #2:  (_xmit_IPGRE){+.}, at: [] dev_queue_xmit+0x37e/0x454
 #3:  (rcu_read_lock_bh){.+}, at: [] dev_queue_xmit+0x0/0x454

stack backtrace:
Pid: 2199, comm: ping Not tainted 2.6.36-rc7-00040-gb0057c5 #5
Call Trace:
 [] ? printk+0xf/0x13
 [] __lock_acquire+0xbda/0x1311
 [] ? __lock_acquire+0x5c3/0x1311
 [] lock_acquire+0x59/0x77
 [] ? dev_queue_xmit+0x37e/0x454
 [] _raw_spin_lock+0x1b/0x2a
 [] ? dev_queue_xmit+0x37e/0x454
 [] dev_queue_xmit+0x37e/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ? ip_append_data+0x536/0x7dc
 [] ip_finish_output+0x29d/0x2c7
 [] ? ip_generic_getfrag+0x0/0x8a
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ip_push_pending_frames+0x260/0x2ad
 [] raw_sendmsg+0x5cf/0x64c
 [] inet_sendmsg+0x46/0x4f
 [] sock_sendmsg+0xa4/0xba
 [] ? might_fault+0x35/0x6f
 [] ? might_fault+0x35/0x6f
 [] ? verify_iovec+0x3e/0x6a
 [] sys_sendmsg+0x149/0x196
 [] ? unlock_page+0x3f/0x42
 [] ? lock_release_non_nested+0x86/0x221
 [] ? might_fault+0x35/0x6f
 [] ? might_fault+0x35/0x6f
 [] sys_socketcall+0x146/0x18b
 [] ? trace_hardirqs_on_thunk+0xc/0x10
 [] syscall_call+0x7/0xb
[ cut here ]
WARNING: at kernel/softirq.c:143 local_bh_enable_ip+0x39/0xa5()
Hardware name: Bochs
Pid: 2199, comm: ping Not tainted 2.6.36-rc7-00040-gb0057c5 #5
Call Trace:
 [] warn_slowpath_common+0x60/0x75
 [] ? local_bh_enable_ip+0x39/0xa5
 [] ? rt_intern_hash+0x4da/0x4f9
 [] warn_slowpath_null+0xf/0x13
 [] local_bh_enable_ip+0x39/0xa5
 [] _raw_spin_unlock_bh+0x25/0x28
 [] rt_intern_hash+0x4da/0x4f9
 [] __ip_route_output_key+0x806/0x860
 [] ip_route_output_flow+0xe/0x3e
 [] ip_route_output_key+0xc/0xe
 [] ipgre_tunnel_xmit+0x1ac/0x757
 [] ? dev_queue_xmit+0x37e/0x454
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? ip_rt_update_pmtu+0x0/0x60
 [] dev_hard_start_xmit+0x33a/0x428
 [] dev_queue_xmit+0x39d/0x454
 [] ip_finish_output+0x29d/0x2c7
 [] ip_output+0x8a/0x8f
 [] ip_local_out+0x50/0x53
 [] ipgre_tunnel_xmit+0x6b8/0x757
 [] ? 

Bug#599816: Nested GRE locking bug

2010-10-13 Thread Eric Dumazet
Le jeudi 14 octobre 2010 à 05:00 +0100, Ben Hutchings a écrit :
> Beatrice Barbe reported a reproducible crash after creating large
> numbers of nested GRE tunnels and then pinging with the source address
> forced.  I was able to reproduce this using net-2.6.  I'm attaching the
> kernel config I used and a script to reproduce this based on the script
> she provided.  The magic number of tunnels to create is apparently 37.
> 
> With lockdep enabled, I get the following output:
> 

Thats a known problem, actually, called stack exhaustion :)

net-next-2.6 contains a fix for this, adding the perc_cpu xmit_recursion
limit. We might push it to net-2.6

Thanks

commit 745e20f1b626b1be4b100af5d4bf7b3439392f8f
Author: Eric Dumazet 
Date:   Wed Sep 29 13:23:09 2010 -0700

net: add a recursion limit in xmit path

As tunnel devices are going to be lockless, we need to make sure a
misconfigured machine wont enter an infinite loop.

Add a percpu variable, and limit to three the number of stacked xmits.

Reported-by: Jesse Gross 
Signed-off-by: Eric Dumazet 
Signed-off-by: David S. Miller 

diff --git a/net/core/dev.c b/net/core/dev.c
index 48ad47f..50dacca 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2177,6 +2177,9 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, 
struct Qdisc *q,
return rc;
 }
 
+static DEFINE_PER_CPU(int, xmit_recursion);
+#define RECURSION_LIMIT 3
+
 /**
  * dev_queue_xmit - transmit a buffer
  * @skb: buffer to transmit
@@ -2242,10 +2245,15 @@ int dev_queue_xmit(struct sk_buff *skb)
 
if (txq->xmit_lock_owner != cpu) {
 
+   if (__this_cpu_read(xmit_recursion) > RECURSION_LIMIT)
+   goto recursion_alert;
+
HARD_TX_LOCK(dev, txq, cpu);
 
if (!netif_tx_queue_stopped(txq)) {
+   __this_cpu_inc(xmit_recursion);
rc = dev_hard_start_xmit(skb, dev, txq);
+   __this_cpu_dec(xmit_recursion);
if (dev_xmit_complete(rc)) {
HARD_TX_UNLOCK(dev, txq);
goto out;
@@ -2257,7 +2265,9 @@ int dev_queue_xmit(struct sk_buff *skb)
   "queue packet!\n", dev->name);
} else {
/* Recursion is detected! It is possible,
-* unfortunately */
+* unfortunately
+*/
+recursion_alert:
if (net_ratelimit())
printk(KERN_CRIT "Dead loop on virtual device "
   "%s, fix it urgently!\n", dev->name);





-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/1287029519.2649.108.ca...@edumazet-laptop



Bug#599816: Nested GRE locking bug

2010-10-19 Thread David Miller
From: Eric Dumazet 
Date: Thu, 14 Oct 2010 06:11:59 +0200

> net-next-2.6 contains a fix for this, adding the perc_cpu
> xmit_recursion limit. We might push it to net-2.6

We need to think a bit more about this.

We are essentially now saying that one can only configure
tunnels 3 levels deep, and no more.

I can guarentee you someone out there uses at least 4,
perhaps more.

And those people will be broken by the new limit.

So putting this into net-2.6 with such a low limit will
be quite dangerous.



-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20101019.015303.104063240.da...@davemloft.net



Bug#599816: Nested GRE locking bug

2010-10-19 Thread Eric Dumazet
Le mardi 19 octobre 2010 à 01:53 -0700, David Miller a écrit :
> From: Eric Dumazet 
> Date: Thu, 14 Oct 2010 06:11:59 +0200
> 
> > net-next-2.6 contains a fix for this, adding the perc_cpu
> > xmit_recursion limit. We might push it to net-2.6
> 
> We need to think a bit more about this.
> 
> We are essentially now saying that one can only configure
> tunnels 3 levels deep, and no more.
> 
> I can guarentee you someone out there uses at least 4,
> perhaps more.
> 
> And those people will be broken by the new limit.
> 
> So putting this into net-2.6 with such a low limit will
> be quite dangerous.

Well limit is actually 4, but I get your point ;)






-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/1287478956.2676.13.ca...@edumazet-laptop



Bug#599816: Nested GRE locking bug

2010-10-25 Thread David Miller
From: Eric Dumazet 
Date: Tue, 19 Oct 2010 11:02:36 +0200

> Le mardi 19 octobre 2010 à 01:53 -0700, David Miller a écrit :
>> From: Eric Dumazet 
>> Date: Thu, 14 Oct 2010 06:11:59 +0200
>> 
>> > net-next-2.6 contains a fix for this, adding the perc_cpu
>> > xmit_recursion limit. We might push it to net-2.6
>> 
>> We need to think a bit more about this.
>> 
>> We are essentially now saying that one can only configure
>> tunnels 3 levels deep, and no more.
>> 
>> I can guarentee you someone out there uses at least 4,
>> perhaps more.
>> 
>> And those people will be broken by the new limit.
>> 
>> So putting this into net-2.6 with such a low limit will
>> be quite dangerous.
> 
> Well limit is actually 4, but I get your point ;)

I'll commit the following to upstream, and submit a combined
patch to -stable.


net: Increase xmit RECURSION_LIMIT to 10.

Three is definitely too low, and we know from reports that GRE tunnels
stacked as deeply as 37 levels cause stack overflows, so pick some
reasonable value between those two.

Signed-off-by: David S. Miller 
---
 net/core/dev.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 78b5a89..2c7da3a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2213,7 +2213,7 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, 
struct Qdisc *q,
 }
 
 static DEFINE_PER_CPU(int, xmit_recursion);
-#define RECURSION_LIMIT 3
+#define RECURSION_LIMIT 10
 
 /**
  * dev_queue_xmit - transmit a buffer
-- 
1.7.3.2




--
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20101025.125347.28807251.da...@davemloft.net



Bug#599816: Nested GRE locking bug

2010-10-25 Thread Eric Dumazet
Le lundi 25 octobre 2010 à 12:53 -0700, David Miller a écrit :

> I'll commit the following to upstream, and submit a combined
> patch to -stable.
> 
> 
> net: Increase xmit RECURSION_LIMIT to 10.
> 
> Three is definitely too low, and we know from reports that GRE tunnels
> stacked as deeply as 37 levels cause stack overflows, so pick some
> reasonable value between those two.
> 
> Signed-off-by: David S. Miller 
> ---
>  net/core/dev.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 78b5a89..2c7da3a 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2213,7 +2213,7 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, 
> struct Qdisc *q,
>  }
>  
>  static DEFINE_PER_CPU(int, xmit_recursion);
> -#define RECURSION_LIMIT 3
> +#define RECURSION_LIMIT 10
>  
>  /**
>   *   dev_queue_xmit - transmit a buffer


Perfect, thanks !






-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/1288037305.3296.0.ca...@edumazet-laptop