Subject: 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) 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 --- 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; }