From: Keir Fraser <keir.fraser@citrix.com>
Date: Mon, 19 Apr 2010 16:19:56 +0000 (+0100)
Subject: xen/blktap: fix various checks
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=a032e3dfc7d0944464dc6f486871e6ba823e5a75;p=legacy%2Flinux-2.6.18-xen.git

xen/blktap: fix various checks

- array indices got checked after having indexed the array already
- several were off by one
- BLKTAP_IOCTL_FREEINTF should not be used on other than the control
  device (or the logic should be changed to that when thus used only
  the respective device can be freed)
- BLKTAP_IOCTL_MINOR can reasonably also be used on non-control
- devices
  (returning that device's minor and ignoring the passed in argument)

Signed-off-by: Jan Beulich <jbeulich@novell.com>
---

diff --git a/drivers/xen/blktap/blktap.c b/drivers/xen/blktap/blktap.c
index 04e79d7c..ffa98022 100644
--- a/drivers/xen/blktap/blktap.c
+++ b/drivers/xen/blktap/blktap.c
@@ -558,11 +558,11 @@ void signal_tapdisk(int idx)
 	 * if the userland tools set things up wrong, this could be negative;
 	 * just don't try to signal in this case
 	 */
-	if (idx < 0)
+	if (idx < 0 || idx >= MAX_TAP_DEV)
 		return;
 
 	info = tapfds[idx];
-	if ((idx < 0) || (idx > MAX_TAP_DEV) || !info)
+	if (!info)
 		return;
 
 	if (info->pid > 0) {
@@ -585,10 +585,13 @@ static int blktap_open(struct inode *inode, struct file *filp)
 	/* ctrl device, treat differently */
 	if (!idx)
 		return 0;
+	if (idx < 0 || idx >= MAX_TAP_DEV) {
+		WPRINTK("No device /dev/xen/blktap%d\n", idx);
+		return -ENODEV;
+	}
 
 	info = tapfds[idx];
-
-	if ((idx < 0) || (idx > MAX_TAP_DEV) || !info) {
+	if (!info) {
 		WPRINTK("Unable to open device /dev/xen/blktap%d\n",
 			idx);
 		return -ENODEV;
@@ -850,9 +853,11 @@ static int blktap_ioctl(struct inode *inode, struct file *filp,
 		unsigned long dev = arg;
 		unsigned long flags;
 
-		info = tapfds[dev];
+		if (info || dev >= MAX_TAP_DEV)
+			return -EINVAL;
 
-		if ((dev > MAX_TAP_DEV) || !info)
+		info = tapfds[dev];
+		if (!info)
 			return 0; /* should this be an error? */
 
 		spin_lock_irqsave(&pending_free_lock, flags);
@@ -863,16 +868,19 @@ static int blktap_ioctl(struct inode *inode, struct file *filp,
 		return 0;
 	}
 	case BLKTAP_IOCTL_MINOR:
-	{
-		unsigned long dev = arg;
+		if (!info) {
+			unsigned long dev = arg;
 
-		info = tapfds[dev];
+			if (dev >= MAX_TAP_DEV)
+				return -EINVAL;
 
-		if ((dev > MAX_TAP_DEV) || !info)
-			return -EINVAL;
+			info = tapfds[dev];
+			if (!info)
+				return -EINVAL;
+		}
 
 		return info->minor;
-	}
+
 	case BLKTAP_IOCTL_MAJOR:
 		return blktap_major;
 
@@ -906,9 +914,11 @@ static void blktap_kick_user(int idx)
 {
 	tap_blkif_t *info;
 
-	info = tapfds[idx];
+	if (idx < 0 || idx >= MAX_TAP_DEV)
+		return;
 
-	if ((idx < 0) || (idx > MAX_TAP_DEV) || !info)
+	info = tapfds[idx];
+	if (!info)
 		return;
 
 	wake_up_interruptible(&info->wait);
@@ -1054,9 +1064,8 @@ static void fast_flush_area(pending_req_t *req, int k_idx, int u_idx,
 	struct mm_struct *mm;
 	
 
-	info = tapfds[tapidx];
-
-	if ((tapidx < 0) || (tapidx > MAX_TAP_DEV) || !info) {
+	if ((tapidx < 0) || (tapidx >= MAX_TAP_DEV)
+	    || !(info = tapfds[tapidx])) {
 		WPRINTK("fast_flush: Couldn't get info!\n");
 		return;
 	}
@@ -1303,7 +1312,7 @@ static int do_block_io_op(blkif_t *blkif)
 	rmb(); /* Ensure we see queued requests up to 'rp'. */
 
 	/*Check blkif has corresponding UE ring*/
-	if (blkif->dev_num < 0) {
+	if (blkif->dev_num < 0 || blkif->dev_num >= MAX_TAP_DEV) {
 		/*oops*/
 		if (print_dbug) {
 			WPRINTK("Corresponding UE " 
@@ -1315,8 +1324,7 @@ static int do_block_io_op(blkif_t *blkif)
 
 	info = tapfds[blkif->dev_num];
 
-	if (blkif->dev_num > MAX_TAP_DEV || !info ||
-	    !test_bit(0, &info->dev_inuse)) {
+	if (!info || !test_bit(0, &info->dev_inuse)) {
 		if (print_dbug) {
 			WPRINTK("Can't get UE info!\n");
 			print_dbug = 0;
@@ -1419,7 +1427,7 @@ static void dispatch_rw_block_io(blkif_t *blkif,
 	struct mm_struct *mm;
 	struct vm_area_struct *vma = NULL;
 
-	if (blkif->dev_num < 0 || blkif->dev_num > MAX_TAP_DEV)
+	if (blkif->dev_num < 0 || blkif->dev_num >= MAX_TAP_DEV)
 		goto fail_response;
 
 	info = tapfds[blkif->dev_num];
@@ -1740,7 +1748,7 @@ static int __init blkif_init(void)
 	/* tapfds[0] is always NULL */
 	blktap_next_minor++;
 
-	DPRINTK("Created misc_dev [/dev/xen/blktap%d]\n",i);
+	DPRINTK("Created misc_dev %d:0 [/dev/xen/blktap0]\n", ret);
 
 	/* Make sure the xen class exists */
 	if ((class = get_xen_class()) != NULL) {
diff --git a/drivers/xen/blktap2/control.c b/drivers/xen/blktap2/control.c
index 82b8579c..dd3da95a 100644
--- a/drivers/xen/blktap2/control.c
+++ b/drivers/xen/blktap2/control.c
@@ -135,7 +135,7 @@ blktap_control_ioctl(struct inode *inode, struct file *filp,
 	case BLKTAP2_IOCTL_FREE_TAP:
 		dev = arg;
 
-		if (dev > MAX_BLKTAP_DEVICE || !blktaps[dev])
+		if (dev >= MAX_BLKTAP_DEVICE || !blktaps[dev])
 			return -EINVAL;
 
 		blktap_control_destroy_device(blktaps[dev]);
diff --git a/drivers/xen/blktap2/ring.c b/drivers/xen/blktap2/ring.c
index 1824019c..5a361205 100644
--- a/drivers/xen/blktap2/ring.c
+++ b/drivers/xen/blktap2/ring.c
@@ -216,7 +216,7 @@ blktap_ring_open(struct inode *inode, struct file *filp)
 	struct blktap *tap;
 
 	idx = iminor(inode);
-	if (idx < 0 || idx > MAX_BLKTAP_DEVICE || blktaps[idx] == NULL) {
+	if (idx < 0 || idx >= MAX_BLKTAP_DEVICE || blktaps[idx] == NULL) {
 		BTERR("unable to open device blktap%d\n", idx);
 		return -ENODEV;
 	}