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 <[EMAIL PROTECTED]> -- 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; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html