Mailing List Archive

# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# Date 1271693996 -3600
# Node ID 347237fd42d855bb301159c85e9f962d8521a21f
# Parent eb21d96a6aaef9719a3c83501e52e21dede2c7a4
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>
---
drivers/xen/blktap/blktap.c | 56 ++++++++++++++++++++++++------------------
drivers/xen/blktap2/control.c | 2 -
drivers/xen/blktap2/ring.c | 2 -
3 files changed, 34 insertions(+), 26 deletions(-)

diff -r eb21d96a6aae -r 347237fd42d8 drivers/xen/blktap/blktap.c
--- a/drivers/xen/blktap/blktap.c Mon Apr 19 17:19:23 2010 +0100
+++ b/drivers/xen/blktap/blktap.c Mon Apr 19 17:19:56 2010 +0100
@@ -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 *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;
@@ -850,9 +853,11 @@ static int blktap_ioctl(struct inode *in
unsigned long dev = arg;
unsigned long flags;

+ if (info || dev >= MAX_TAP_DEV)
+ return -EINVAL;
+
info = tapfds[dev];
-
- if ((dev > MAX_TAP_DEV) || !info)
+ 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 *in
return 0;
}
case BLKTAP_IOCTL_MINOR:
- {
- unsigned long dev = arg;
-
- info = tapfds[dev];
-
- if ((dev > MAX_TAP_DEV) || !info)
- return -EINVAL;
+ if (!info) {
+ unsigned long dev = arg;
+
+ if (dev >= MAX_TAP_DEV)
+ 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;

+ if (idx < 0 || idx >= MAX_TAP_DEV)
+ return;
+
info = tapfds[idx];
-
- if ((idx < 0) || (idx > MAX_TAP_DEV) || !info)
+ if (!info)
return;

wake_up_interruptible(&info->wait);
@@ -1054,9 +1064,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;
}
@@ -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
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 -r eb21d96a6aae -r 347237fd42d8 drivers/xen/blktap2/control.c
--- a/drivers/xen/blktap2/control.c Mon Apr 19 17:19:23 2010 +0100
+++ b/drivers/xen/blktap2/control.c Mon Apr 19 17:19:56 2010 +0100
@@ -135,7 +135,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]);
diff -r eb21d96a6aae -r 347237fd42d8 drivers/xen/blktap2/ring.c
--- a/drivers/xen/blktap2/ring.c Mon Apr 19 17:19:23 2010 +0100
+++ b/drivers/xen/blktap2/ring.c Mon Apr 19 17:19:56 2010 +0100
@@ -216,7 +216,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;
}

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