[Xen-devel] [PATCH] linux/blktap: fix various checks

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

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

Written and tested on 2.6.32.11 and made apply to the 2.6.18 tree
without further testing.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>

--- sle11sp1-2010-04-12.orig/drivers/xen/blktap/blktap.c        2010-04-19 
09:24:00.000000000 +0200
+++ sle11sp1-2010-04-12/drivers/xen/blktap/blktap.c     2010-04-19 
11:59:19.000000000 +0200
@@ -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) {
@@ -586,10 +586,13 @@ static int blktap_open(struct inode *ino
        /* 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;
@@ -852,9 +855,11 @@ static int blktap_ioctl(struct inode *in
                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);
@@ -865,16 +870,19 @@ static int blktap_ioctl(struct inode *in
                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;
 
@@ -908,9 +916,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);
@@ -1056,9 +1066,8 @@ static void fast_flush_area(pending_req_
        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;
        }
@@ -1306,7 +1315,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 " 
@@ -1318,8 +1327,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;
@@ -1427,7 +1435,7 @@ static void dispatch_rw_block_io(blkif_t
        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];
@@ -1748,7 +1756,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) {
--- sle11sp1-2010-04-12.orig/drivers/xen/blktap2/control.c      2009-11-06 
10:51:17.000000000 +0100
+++ sle11sp1-2010-04-12/drivers/xen/blktap2/control.c   2010-04-14 
15:41:58.000000000 +0200
@@ -136,7 +136,7 @@ blktap_control_ioctl(struct inode *inode
        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]);
--- sle11sp1-2010-04-12.orig/drivers/xen/blktap2/ring.c 2009-11-06 
10:51:32.000000000 +0100
+++ sle11sp1-2010-04-12/drivers/xen/blktap2/ring.c      2010-04-14 
15:42:20.000000000 +0200
@@ -215,7 +215,7 @@ blktap_ring_open(struct inode *inode, st
        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;
        }

Attachment: xen-blktap-checks.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel