[PATCH] fix locking

[PATCH] fix locking

[Michael Rozhavsky]

[-- Attachment #1: Type: text/plain, Size: 77 bytes --]

Hi,

I've found two problems with locking in networking code.

--

	Michael


[-- Attachment #2: tcp_ipv4.c.diff --]
[-- Type: text/plain, Size: 390 bytes --]

--- ../../../linux-2.4.22/net/ipv4/tcp_ipv4.c	Mon Aug 25 04:44:44 2003
+++ tcp_ipv4.c	Fri Sep  5 16:56:54 2003
@@ -907,9 +907,9 @@
 	req->expires = jiffies + TCP_TIMEOUT_INIT;
 	req->retrans = 0;
 	req->sk = NULL;
-	req->dl_next = lopt->syn_table[h];
 
 	write_lock(&tp->syn_wait_lock);
+	req->dl_next = lopt->syn_table[h];
 	lopt->syn_table[h] = req;
 	write_unlock(&tp->syn_wait_lock);
 

[-- Attachment #3: sit.c.diff --]
[-- Type: text/plain, Size: 656 bytes --]

--- sit.c.orig	Fri Sep  5 17:07:55 2003
+++ sit.c	Fri Sep  5 17:08:37 2003
@@ -125,14 +125,14 @@
 {
 	struct ip_tunnel **tp;
 
+	write_lock_bh(&ipip6_lock);
 	for (tp = ipip6_bucket(t); *tp; tp = &(*tp)->next) {
 		if (t == *tp) {
-			write_lock_bh(&ipip6_lock);
 			*tp = t->next;
-			write_unlock_bh(&ipip6_lock);
 			break;
 		}
 	}
+	write_unlock_bh(&ipip6_lock);
 }
 
 static void ipip6_tunnel_link(struct ip_tunnel *t)
@@ -141,8 +141,8 @@
 
 	write_lock_bh(&ipip6_lock);
 	t->next = *tp;
-	write_unlock_bh(&ipip6_lock);
 	*tp = t;
+	write_unlock_bh(&ipip6_lock);
 }
 
 struct ip_tunnel * ipip6_tunnel_locate(struct ip_tunnel_parm *parms, int create)

Re: [PATCH] fix locking

[David S. Miller]

On Fri, 5 Sep 2003 17:26:49 -0700
Michael Rozhavsky <mike@minantech.com> wrote:

> I've found two problems with locking in networking code.

The tcp_ipv4.c part is not a bug.  We hold the socket lock on 'sk' and
therefore nobody else can add any entries to lopt->syn_table[]
entries.

We only need to hold the lock during the table update
because _readers_ of the hash table do not take the 'sk'
lock when do traverse the hash.

The sit.c change does look correct, and therefore I'll apply
it after some more review...

Re: [PATCH] fix locking

[David S. Miller]

On Fri, 5 Sep 2003 17:26:49 -0700
Michael Rozhavsky <mike@minantech.com> wrote:

> I've found two problems with locking in networking code.

Not applied, in both cases a higher order lock guarentees
that nobody else can add elements to the list.

We only need to lock to make sure readers (who don't take
the higher level lock) see a consistent list state.

In the tcp_ipv4.c case, the higher level lock is the listening
socket lock.  In the sit.c case the higher level lock is the
RTNL semaphore.