Deadlock in MD5 Signature support

Deadlock in MD5 Signature support

[Leigh Brown]

It was all going so well, and then it deadlocked.  What can be done
about this?

SysRq : Show Regs

Pid: 2330, comm:              torture
EIP: 0060:[<c02a6240>] CPU: 0
EIP is at _spin_lock+0x5/0xf
 EFLAGS: 00000282    Not tainted  (2.6.19.1 #2)
EAX: c03187e4 EBX: 00000000 ECX: fa01a8c0 EDX: c52f81e0
ESI: df983240 EDI: c51f6800 EBP: dfc44a20 DS: 007b ES: 007b
CR0: 8005003b CR2: 08124008 CR3: 1ff72a80 CR4: 000006f0
 [<c026e4e4>] __tcp_get_md5sig_pool+0xd/0x3f
 [<c027a563>] tcp_v4_do_calc_md5_hash+0x20/0x1a7
 [<c011a142>] __activate_task+0x1c/0x29
 [<c011a946>] try_to_wake_up+0x27a/0x285
 [<c027a830>] tcp_v4_inbound_md5_hash+0xe1/0x1b2
 [<c027b2dc>] tcp_v4_do_rcv+0xd/0xd3
 [<c027b8ba>] tcp_v4_rcv+0x518/0x81b
 [<c026212e>] ip_local_deliver+0x17d/0x235
 [<c0262619>] ip_rcv+0x433/0x475
 [<c01c7988>] __next_cpu+0x12/0x1f
 [<c011b3a7>] find_busiest_group+0x184/0x475
 [<c024511b>] netif_receive_skb+0x303/0x322
 [<f8872be6>] tg3_rx+0x278/0x385 [tg3]
 [<f8872dbe>] tg3_poll+0xcb/0x17c [tg3]
 [<c02452ca>] net_rx_action+0x98/0x198
 [<c0125a7a>] __do_softirq+0x5e/0xbd
 [<c0125b07>] do_softirq+0x2e/0x32
 [<c010500e>] do_IRQ+0x82/0xa2
 [<c010371e>] common_interrupt+0x1a/0x20
 [<c02a623e>] _spin_lock+0x3/0xf
 [<c026e441>] tcp_alloc_md5sig_pool+0xc/0xa2
 [<c027a27b>] tcp_v4_md5_do_add+0x6a/0x11a
 [<c027a536>] tcp_v4_parse_md5_keys+0xeb/0xf8
 [<c02a61b8>] _spin_lock_bh+0x8/0x18
 [<c026dc26>] do_tcp_setsockopt+0x2f1/0x314
 [<c023ebd1>] sock_common_setsockopt+0x1c/0x21
 [<c023c72e>] sys_setsockopt+0x8d/0xac
 [<c023cd86>] sys_socketcall+0x1e7/0x242
 [<c0102d2d>] sysenter_past_esp+0x56/0x79
 =======================

Re: Deadlock in MD5 Signature support

[David Miller]

From: "Leigh Brown" <leigh@solinno.co.uk>
Date: Sun, 17 Dec 2006 18:13:06 -0000 (GMT)

> It was all going so well, and then it deadlocked.  What can be done
> about this?

Thanks for the trace, I'll take a look Leigh.

Re: Deadlock in MD5 Signature support

[David Miller]

From: "Leigh Brown" <leigh@solinno.co.uk>
Date: Sun, 17 Dec 2006 18:13:06 -0000 (GMT)

> It was all going so well, and then it deadlocked.  What can be done
> about this?

Sorry for taking so long about this.

The problem is that tcp_md5sig_pool_lock is taken in both
software-interrupt and non-software-interrupt contexts, but
it is always acquired/released using spin_lock/spin_unlock.

The most conservative fix, which I'll use for 2.6.21 and
2.6.x-stable, is the following.

I invite anyone to rewrite this md5 signature pool mutual
exclusion code, it's not exactly the best :-)

Thanks for reporting this problem.

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index ac6516c..74c4d10 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2266,12 +2266,12 @@ void tcp_free_md5sig_pool(void)
 {
 	struct tcp_md5sig_pool **pool = NULL;
 
-	spin_lock(&tcp_md5sig_pool_lock);
+	spin_lock_bh(&tcp_md5sig_pool_lock);
 	if (--tcp_md5sig_users == 0) {
 		pool = tcp_md5sig_pool;
 		tcp_md5sig_pool = NULL;
 	}
-	spin_unlock(&tcp_md5sig_pool_lock);
+	spin_unlock_bh(&tcp_md5sig_pool_lock);
 	if (pool)
 		__tcp_free_md5sig_pool(pool);
 }
@@ -2314,36 +2314,36 @@ struct tcp_md5sig_pool **tcp_alloc_md5sig_pool(void)
 	int alloc = 0;
 
 retry:
-	spin_lock(&tcp_md5sig_pool_lock);
+	spin_lock_bh(&tcp_md5sig_pool_lock);
 	pool = tcp_md5sig_pool;
 	if (tcp_md5sig_users++ == 0) {
 		alloc = 1;
-		spin_unlock(&tcp_md5sig_pool_lock);
+		spin_unlock_bh(&tcp_md5sig_pool_lock);
 	} else if (!pool) {
 		tcp_md5sig_users--;
-		spin_unlock(&tcp_md5sig_pool_lock);
+		spin_unlock_bh(&tcp_md5sig_pool_lock);
 		cpu_relax();
 		goto retry;
 	} else
-		spin_unlock(&tcp_md5sig_pool_lock);
+		spin_unlock_bh(&tcp_md5sig_pool_lock);
 
 	if (alloc) {
 		/* we cannot hold spinlock here because this may sleep. */
 		struct tcp_md5sig_pool **p = __tcp_alloc_md5sig_pool();
-		spin_lock(&tcp_md5sig_pool_lock);
+		spin_lock_bh(&tcp_md5sig_pool_lock);
 		if (!p) {
 			tcp_md5sig_users--;
-			spin_unlock(&tcp_md5sig_pool_lock);
+			spin_unlock_bh(&tcp_md5sig_pool_lock);
 			return NULL;
 		}
 		pool = tcp_md5sig_pool;
 		if (pool) {
 			/* oops, it has already been assigned. */
-			spin_unlock(&tcp_md5sig_pool_lock);
+			spin_unlock_bh(&tcp_md5sig_pool_lock);
 			__tcp_free_md5sig_pool(p);
 		} else {
 			tcp_md5sig_pool = pool = p;
-			spin_unlock(&tcp_md5sig_pool_lock);
+			spin_unlock_bh(&tcp_md5sig_pool_lock);
 		}
 	}
 	return pool;
@@ -2354,11 +2354,11 @@ EXPORT_SYMBOL(tcp_alloc_md5sig_pool);
 struct tcp_md5sig_pool *__tcp_get_md5sig_pool(int cpu)
 {
 	struct tcp_md5sig_pool **p;
-	spin_lock(&tcp_md5sig_pool_lock);
+	spin_lock_bh(&tcp_md5sig_pool_lock);
 	p = tcp_md5sig_pool;
 	if (p)
 		tcp_md5sig_users++;
-	spin_unlock(&tcp_md5sig_pool_lock);
+	spin_unlock_bh(&tcp_md5sig_pool_lock);
 	return (p ? *per_cpu_ptr(p, cpu) : NULL);
 }