netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
@ 2008-02-11  9:22 James Chapman
  2008-02-11 18:57 ` Jarek Poplawski
  0 siblings, 1 reply; 51+ messages in thread
From: James Chapman @ 2008-02-11  9:22 UTC (permalink / raw)
  To: netdev

Fix locking issues in the pppol2tp driver which can cause a kernel
crash on SMP boxes when hundreds of L2TP sessions are created/deleted
simultaneously (ISP environment). The driver was violating read_lock()
and write_lock() scheduling rules so we now consistently use the _irq
variants of the lock functions.

Signed-off-by: James Chapman <jchapman@katalix.com>

--
This patch has been verified by the ISP that discovered the problem.

If the patch is accepted, it should be pushed to the stable 2.6.23 and
2.6.24 trees.

Index: linux-2.6.24/drivers/net/pppol2tp.c
===================================================================
--- linux-2.6.24.orig/drivers/net/pppol2tp.c
+++ linux-2.6.24/drivers/net/pppol2tp.c
@@ -301,15 +301,16 @@ pppol2tp_session_find(struct pppol2tp_tu
 		pppol2tp_session_id_hash(tunnel, session_id);
 	struct pppol2tp_session *session;
 	struct hlist_node *walk;
+	unsigned long flags;
 
-	read_lock(&tunnel->hlist_lock);
+	read_lock_irqsave(&tunnel->hlist_lock, flags);
 	hlist_for_each_entry(session, walk, session_list, hlist) {
 		if (session->tunnel_addr.s_session == session_id) {
-			read_unlock(&tunnel->hlist_lock);
+			read_unlock_irqrestore(&tunnel->hlist_lock, flags);
 			return session;
 		}
 	}
-	read_unlock(&tunnel->hlist_lock);
+	read_unlock_irqrestore(&tunnel->hlist_lock, flags);
 
 	return NULL;
 }
@@ -319,15 +320,16 @@ pppol2tp_session_find(struct pppol2tp_tu
 static struct pppol2tp_tunnel *pppol2tp_tunnel_find(u16 tunnel_id)
 {
 	struct pppol2tp_tunnel *tunnel = NULL;
+	unsigned long flags;
 
-	read_lock(&pppol2tp_tunnel_list_lock);
+	read_lock_irqsave(&pppol2tp_tunnel_list_lock, flags);
 	list_for_each_entry(tunnel, &pppol2tp_tunnel_list, list) {
 		if (tunnel->stats.tunnel_id == tunnel_id) {
-			read_unlock(&pppol2tp_tunnel_list_lock);
+			read_unlock_irqrestore(&pppol2tp_tunnel_list_lock, flags);
 			return tunnel;
 		}
 	}
-	read_unlock(&pppol2tp_tunnel_list_lock);
+	read_unlock_irqrestore(&pppol2tp_tunnel_list_lock, flags);
 
 	return NULL;
 }
@@ -1099,6 +1101,7 @@ static void pppol2tp_tunnel_closeall(str
 	struct hlist_node *tmp;
 	struct pppol2tp_session *session;
 	struct sock *sk;
+	unsigned long flags;
 
 	if (tunnel == NULL)
 		BUG();
@@ -1106,7 +1109,7 @@ static void pppol2tp_tunnel_closeall(str
 	PRINTK(tunnel->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
 	       "%s: closing all sessions...\n", tunnel->name);
 
-	write_lock(&tunnel->hlist_lock);
+	write_lock_irqsave(&tunnel->hlist_lock, flags);
 	for (hash = 0; hash < PPPOL2TP_HASH_SIZE; hash++) {
 again:
 		hlist_for_each_safe(walk, tmp, &tunnel->session_hlist[hash]) {
@@ -1126,7 +1129,7 @@ again:
 			 * disappear as we're jumping between locks.
 			 */
 			sock_hold(sk);
-			write_unlock(&tunnel->hlist_lock);
+			write_unlock_irqrestore(&tunnel->hlist_lock, flags);
 			lock_sock(sk);
 
 			if (sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND)) {
@@ -1148,11 +1151,11 @@ again:
 			 * list so we are guaranteed to make forward
 			 * progress.
 			 */
-			write_lock(&tunnel->hlist_lock);
+			write_lock_irqsave(&tunnel->hlist_lock, flags);
 			goto again;
 		}
 	}
-	write_unlock(&tunnel->hlist_lock);
+	write_unlock_irqrestore(&tunnel->hlist_lock, flags);
 }
 
 /* Really kill the tunnel.
@@ -1160,10 +1163,12 @@ again:
  */
 static void pppol2tp_tunnel_free(struct pppol2tp_tunnel *tunnel)
 {
+	unsigned long flags;
+
 	/* Remove from socket list */
-	write_lock(&pppol2tp_tunnel_list_lock);
+	write_lock_irqsave(&pppol2tp_tunnel_list_lock, flags);
 	list_del_init(&tunnel->list);
-	write_unlock(&pppol2tp_tunnel_list_lock);
+	write_unlock_irqrestore(&pppol2tp_tunnel_list_lock, flags);
 
 	atomic_dec(&pppol2tp_tunnel_count);
 	kfree(tunnel);
@@ -1212,6 +1217,7 @@ end:
 static void pppol2tp_session_destruct(struct sock *sk)
 {
 	struct pppol2tp_session *session = NULL;
+	unsigned long flags;
 
 	if (sk->sk_user_data != NULL) {
 		struct pppol2tp_tunnel *tunnel;
@@ -1239,9 +1245,9 @@ static void pppol2tp_session_destruct(st
 				/* Delete the session socket from the
 				 * hash
 				 */
-				write_lock(&tunnel->hlist_lock);
+				write_lock_irqsave(&tunnel->hlist_lock, flags);
 				hlist_del_init(&session->hlist);
-				write_unlock(&tunnel->hlist_lock);
+				write_unlock_irqrestore(&tunnel->hlist_lock, flags);
 
 				atomic_dec(&pppol2tp_session_count);
 			}
@@ -1312,6 +1318,7 @@ static struct sock *pppol2tp_prepare_tun
 	struct sock *sk;
 	struct pppol2tp_tunnel *tunnel;
 	struct sock *ret = NULL;
+	unsigned long flags;
 
 	/* Get the tunnel UDP socket from the fd, which was opened by
 	 * the userspace L2TP daemon.
@@ -1386,9 +1393,9 @@ static struct sock *pppol2tp_prepare_tun
 
 	/* Add tunnel to our list */
 	INIT_LIST_HEAD(&tunnel->list);
-	write_lock(&pppol2tp_tunnel_list_lock);
+	write_lock_irqsave(&pppol2tp_tunnel_list_lock, flags);
 	list_add(&tunnel->list, &pppol2tp_tunnel_list);
-	write_unlock(&pppol2tp_tunnel_list_lock);
+	write_unlock_irqrestore(&pppol2tp_tunnel_list_lock, flags);
 	atomic_inc(&pppol2tp_tunnel_count);
 
 	/* Bump the reference count. The tunnel context is deleted
@@ -1462,6 +1469,7 @@ static int pppol2tp_connect(struct socke
 	struct pppol2tp_tunnel *tunnel;
 	struct dst_entry *dst;
 	int error = 0;
+	unsigned long irqflags;
 
 	lock_sock(sk);
 
@@ -1593,11 +1601,11 @@ static int pppol2tp_connect(struct socke
 	sk->sk_user_data = session;
 
 	/* Add session to the tunnel's hash list */
-	write_lock(&tunnel->hlist_lock);
+	write_lock_irqsave(&tunnel->hlist_lock, irqflags);
 	hlist_add_head(&session->hlist,
 		       pppol2tp_session_id_hash(tunnel,
 						session->tunnel_addr.s_session));
-	write_unlock(&tunnel->hlist_lock);
+	write_unlock_irqrestore(&tunnel->hlist_lock, irqflags);
 
 	atomic_inc(&pppol2tp_session_count);
 
@@ -2198,8 +2206,9 @@ static struct pppol2tp_session *next_ses
 	int found = 0;
 	int next = 0;
 	int i;
+	unsigned long flags;
 
-	read_lock(&tunnel->hlist_lock);
+	read_lock_irqsave(&tunnel->hlist_lock, flags);
 	for (i = 0; i < PPPOL2TP_HASH_SIZE; i++) {
 		hlist_for_each_entry(session, walk, &tunnel->session_hlist[i], hlist) {
 			if (curr == NULL) {
@@ -2217,7 +2226,7 @@ static struct pppol2tp_session *next_ses
 		}
 	}
 out:
-	read_unlock(&tunnel->hlist_lock);
+	read_unlock_irqrestore(&tunnel->hlist_lock, flags);
 	if (!found)
 		session = NULL;
 
@@ -2227,14 +2236,15 @@ out:
 static struct pppol2tp_tunnel *next_tunnel(struct pppol2tp_tunnel *curr)
 {
 	struct pppol2tp_tunnel *tunnel = NULL;
+	unsigned long flags;
 
-	read_lock(&pppol2tp_tunnel_list_lock);
+	read_lock_irqsave(&pppol2tp_tunnel_list_lock, flags);
 	if (list_is_last(&curr->list, &pppol2tp_tunnel_list)) {
 		goto out;
 	}
 	tunnel = list_entry(curr->list.next, struct pppol2tp_tunnel, list);
 out:
-	read_unlock(&pppol2tp_tunnel_list_lock);
+	read_unlock_irqrestore(&pppol2tp_tunnel_list_lock, flags);
 
 	return tunnel;
 }

^ permalink raw reply	[flat|nested] 51+ messages in thread
end of thread, other threads:[~2008-03-04  4:49 UTC | newest]

Thread overview: 51+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-11  9:22 [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver James Chapman
2008-02-11 18:57 ` Jarek Poplawski
2008-02-11 22:19   ` James Chapman
2008-02-11 22:49     ` Jarek Poplawski
2008-02-11 22:55       ` Jarek Poplawski
2008-02-11 23:42         ` James Chapman
2008-02-12 10:42           ` Jarek Poplawski
2008-02-11 23:41       ` James Chapman
2008-02-12  5:30         ` David Miller
2008-02-12 10:58           ` James Chapman
2008-02-12 13:24             ` Jarek Poplawski
2008-02-13  6:00             ` David Miller
2008-02-13  7:29               ` Jarek Poplawski
2008-02-14 13:00             ` Jarek Poplawski
2008-02-18 22:09               ` James Chapman
2008-02-18 23:01                 ` Jarek Poplawski
2008-02-19  9:09                   ` James Chapman
2008-02-19  4:29                 ` David Miller
2008-02-19  9:03                   ` James Chapman
2008-02-19 10:30                     ` Jarek Poplawski
2008-02-19 10:36                       ` Jarek Poplawski
2008-02-19 14:37                       ` James Chapman
2008-02-19 23:06                 ` Jarek Poplawski
2008-02-19 23:28                   ` Jarek Poplawski
2008-02-20 16:02                   ` James Chapman
2008-02-20 18:38                     ` Jarek Poplawski
2008-02-20 22:37                       ` James Chapman
2008-02-21  8:59                         ` Jarek Poplawski
2008-02-21  9:53                           ` James Chapman
2008-02-21 12:08                             ` Jarek Poplawski
2008-02-21 17:09                               ` Jarek Poplawski
2008-02-25 12:19                                 ` James Chapman
2008-02-25 13:05                                   ` Jarek Poplawski
2008-02-25 13:39                                     ` Jarek Poplawski
2008-02-25 14:02                                       ` Jarek Poplawski
2008-02-25 21:58                                       ` Jarek Poplawski
2008-02-26 12:14                                         ` James Chapman
2008-02-26 13:03                                           ` Jarek Poplawski
2008-02-26 13:18                                             ` Jarek Poplawski
2008-02-26 20:00                                           ` Jarek Poplawski
2008-03-02 20:29                                             ` James Chapman
2008-03-03  8:22                                               ` Jarek Poplawski
2008-03-03  9:35                                                 ` Jarek Poplawski
2008-02-27 10:54                                           ` [PATCH][PPPOL2TP] add missing sock_put() in pppol2tp_recv_dequeue() Jarek Poplawski
2008-03-02 20:31                                             ` James Chapman
2008-03-04  4:49                                               ` David Miller
2008-02-27 11:48                                           ` [PATCH][PPPOL2TP] add missing sock_put() in pppol2tp_tunnel_closeall() Jarek Poplawski
2008-03-02 20:32                                             ` James Chapman
2008-03-04  4:49                                               ` David Miller
2008-02-22 14:16                             ` [PATCH][NET] sock.c: sk_dst_lock lockdep keys and names per af_family Jarek Poplawski
2008-02-12  7:19         ` [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver Jarek Poplawski

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).