WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] linux/blktap: fix various checks
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Mon, 19 Apr 2010 14:20:01 +0100
Delivery-date: Mon, 19 Apr 2010 06:22:49 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
- 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
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] linux/blktap: fix various checks, Jan Beulich <=