[PATCH 2.4.22-pre] nbd: fix race conditions

[Paul Clements <Paul.Clements@SteelEye.com>]

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

This patch is similar to one I posted yesterday for 2.6. It fixes the
following race conditions in nbd:
 
1) adds locking and properly orders the code in NBD_CLEAR_SOCK to
eliminate races with other code
 
2) adds an lo->sock check to nbd_clear_que to eliminate races between
do_nbd_request and nbd_clear_que, which resulted in the dequeuing of
active requests
 
3) adds an lo->sock check to NBD_DO_IT to eliminate races with
NBD_CLEAR_SOCK, which caused an Oops when "nbd-client -d" was called
 

Marcelo,

If we can get this into 2.4.22, that would be great. I know there's at
least one person who's consistently getting oopses with nbd.

Thanks,
Paul

[-- Attachment #2: nbd-race_fixes_2_4_21.diff --]
[-- Type: text/x-diff, Size: 2027 bytes --]

--- linux-2.4.21-PRISTINE/drivers/block/nbd.c	Fri Jun 13 10:51:32 2003
+++ linux-2.4.21/drivers/block/nbd.c	Sat Aug  9 13:25:49 2003
@@ -428,23 +428,24 @@ static int nbd_ioctl(struct inode *inode
                 return 0 ;
  
 	case NBD_CLEAR_SOCK:
+		error = 0;
+		down(&lo->tx_lock);
+		lo->sock = NULL;
+		up(&lo->tx_lock);
+		spin_lock(&lo->queue_lock);
+		file = lo->file;
+		lo->file = NULL;
+		spin_unlock(&lo->queue_lock);
 		nbd_clear_que(lo);
 		spin_lock(&lo->queue_lock);
 		if (!list_empty(&lo->queue_head)) {
-			spin_unlock(&lo->queue_lock);
-			printk(KERN_ERR "nbd: Some requests are in progress -> can not turn off.\n");
-			return -EBUSY;
-		}
-		file = lo->file;
-		if (!file) {
-			spin_unlock(&lo->queue_lock);
-			return -EINVAL;
+			printk(KERN_ERR "nbd: disconnect: some requests are in progress -> please try again.\n");
+			error = -EBUSY;
 		}
-		lo->file = NULL;
-		lo->sock = NULL;
 		spin_unlock(&lo->queue_lock);
-		fput(file);
-		return 0;
+		if (file)
+			fput(file);
+		return error;
 	case NBD_SET_SOCK:
 		if (lo->file)
 			return -EBUSY;
@@ -491,9 +492,12 @@ static int nbd_ioctl(struct inode *inode
 		 * there should be a more generic interface rather than
 		 * calling socket ops directly here */
 		down(&lo->tx_lock);
-		printk(KERN_WARNING "nbd: shutting down socket\n");
-		lo->sock->ops->shutdown(lo->sock, SEND_SHUTDOWN|RCV_SHUTDOWN);
-		lo->sock = NULL;
+		if (lo->sock) {
+			printk(KERN_WARNING "nbd: shutting down socket\n");
+			lo->sock->ops->shutdown(lo->sock,
+				SEND_SHUTDOWN|RCV_SHUTDOWN);
+			lo->sock = NULL;
+		}
 		up(&lo->tx_lock);
 		spin_lock(&lo->queue_lock);
 		file = lo->file;
@@ -505,6 +509,13 @@ static int nbd_ioctl(struct inode *inode
 			fput(file);
 		return lo->harderror;
 	case NBD_CLEAR_QUE:
+		down(&lo->tx_lock);
+		if (lo->sock) {
+			up(&lo->tx_lock);
+			return 0; /* probably should be error, but that would
+				   * break "nbd-client -d", so just return 0 */
+		}
+		up(&lo->tx_lock);
 		nbd_clear_que(lo);
 		return 0;
 #ifdef PARANOIA