Problem with __check_and_rekey

Hi All!

We have found a deadlock in kernel version 2.4.21.

With sysreq we get this call trace:

Trace; c01a9946 <secure_tcp_sequence_number+52/b0>
Trace; c0256cbc <tcp_v4_conn_request+418/4a8>
[...]
Trace; c023e890 <ip_rcv_finish+0/219>
Trace; c022c88b <netif_receive_skb+11b/148>
Trace; c022c939 <process_backlog+81/124>
Trace; c022ca6f <net_rx_action+93/144>
Trace; c011ee3d <do_softirq+7d/dc>
Trace; c010a2eb <do_IRQ+db/ec>
Trace; c01a88e7 <SHATransform+d3/114>
Trace; c01a8b04 <extract_entropy+1dc/328>
Trace; c01a8c6b <get_random_bytes+1b/40>
Trace; c01a98c8 <__check_and_rekey+5c/88>
Trace; c01a9946 <secure_tcp_sequence_number+52/b0>
Trace; c02559cd <tcp_v4_connect+2f9/3fc>
Trace; c026304d <inet_stream_connect+10d/268>
Trace; c0225de7 <sys_connect+5b/78>
Trace; c0262482 <inet_setsockopt+2a/34>

First call of __check_and_rekey locks ip_lock.

But when we harvest entropy, there is an interrupt triggered by an incoming
packet. Because of the incoming SYN packet we try to generate another
sequence number. Hovewer ip_lock is already locked...

We created this patch to avoid the problem:

--- tcp_ipv4.c~ Tue Jun 24 22:44:52 2003
+++ tcp_ipv4.c  Tue Aug 12 14:21:33 2003

@@ -872,8 +872,10 @@
                        tp->write_seq = ip_randomisn();
                else
 #endif
+               local_bh_disable();
                tp->write_seq = secure_tcp_sequence_number(sk->saddr, sk->daddr,
                                                           sk->sport, usin->sin_port);
+               local_bh_enable();
        }
 
 #ifdef CONFIG_GRKERNSEC_RANDID

-- 
Szalay Attila                     BalaBit IT Biztonságtechnikai Kft.
tel:(36-1)-371-05-40              1116 Bp. Csurgoi ut 20/b
fax:(36-1)-208-08-75              http://www.balabit.hu/

On Tue, 12 Aug 2003 22:55:27 +0900 (JST)
YOSHIFUJI Hideaki / _$B5HF#1QL@ [off-list ref] wrote:

quoted

+               local_bh_disable();
                tp->write_seq = secure_tcp_sequence_number(sk->saddr, sk->daddr,
                                                           sk->sport, usin->sin_port);
+               local_bh_enable();

You must forgot braces.
But anyway, I can't find these lines in linux-2.4.21 (or even in 2.6.x).
Are you sure you're working on the vanilla kernel?

It doesn't matter, a proper fix was put into drivers/char/random.c
already.

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1019.3.9 -> 1.1019.3.10
#	drivers/char/random.c	1.17    -> 1.18   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/07/28	olof@austin.ibm.com	1.1019.3.10
# [RANDOM]: Fix SMP deadlock in __check_and_rekey().
# --------------------------------------------
#
diff -Nru a/drivers/char/random.c b/drivers/char/random.c

--- a/drivers/char/random.c	Tue Aug 12 06:56:52 2003
+++ b/drivers/char/random.c	Tue Aug 12 06:56:52 2003

@@ -251,6 +251,8 @@
 #include <linux/random.h>
 #include <linux/poll.h>
 #include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
 
 #include <asm/processor.h>
 #include <asm/uaccess.h>

@@ -2069,7 +2071,7 @@
 static struct keydata *__check_and_rekey(time_t time)
 {
 	struct keydata *keyptr;
-	spin_lock(&ip_lock);
+	spin_lock_bh(&ip_lock);
 	keyptr = &ip_keydata[ip_cnt&1];
 	if (!keyptr->rekey_time || (time - keyptr->rekey_time) > REKEY_INTERVAL) {
 		keyptr = &ip_keydata[1^(ip_cnt&1)];

@@ -2079,7 +2081,7 @@
 		mb();
 		ip_cnt++;
 	}
-	spin_unlock(&ip_lock);
+	spin_unlock_bh(&ip_lock);
 	return keyptr;
 }
 

In article [off-list ref] (at Tue, 12 Aug 2003 15:20:50 +0200), SZALAY Attila [off-list ref] says:

You must forgot braces.
But anyway, I can't find these lines in linux-2.4.21 (or even in 2.6.x).
Are you sure you're working on the vanilla kernel?

-- 
Hideaki YOSHIFUJI @ USAGI Project [off-list ref]
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

On Tue, 12 Aug 2003 16:05:16 +0200
SZALAY Attila [off-list ref] wrote:

The patch in vanilla kernel:

As I said in another email, your patch is unnecessary and
the bug has been fixed in current kernels already.

Hi ALl!


On 2003 Aug 12, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:

In article [off-list ref] (at Tue, 12 Aug 2003 15:20:50 +0200), SZALAY Attila [off-list ref] says:

But anyway, I can't find these lines in linux-2.4.21 (or even in 2.6.x).
Are you sure you're working on the vanilla kernel?

Sorry, no.

The patch in vanilla kernel:

--- tcp_ipv4.c~ Tue Aug 12 16:01:31 2003
+++ tcp_ipv4.c  Tue Aug 12 16:03:35 2003

@@ -843,9 +843,12 @@
        if (err)
                goto failure;
 
-       if (!tp->write_seq)
+       if (!tp->write_seq) {
+               local_bh_disable();
                tp->write_seq = secure_tcp_sequence_number(sk->saddr, sk->daddr,
                                                           sk->sport, usin->sin_port);
+               local_bh_enable();
+       }
 
        sk->protinfo.af_inet.id = tp->write_seq^jiffies;

-- 
Szalay Attila                     BalaBit IT Biztonságtechnikai Kft.
tel:(36-1)-371-05-40              1116 Bp. Csurgoi ut 20/b
fax:(36-1)-208-08-75              http://www.balabit.hu/

In article [off-list ref] (at Tue, 12 Aug 2003 06:53:46 -0700), "David S. Miller" [off-list ref] says:

It doesn't matter, a proper fix was put into drivers/char/random.c
already.

:

-	spin_lock(&ip_lock);
+	spin_lock_bh(&ip_lock);

:

-	spin_unlock(&ip_lock);
+	spin_unlock_bh(&ip_lock);
 	return keyptr;

Ah, exactly.  Thanks for information.

--yoshfuji