All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.