|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [linux-2.6.18-xen] xen/gntdev: add range check for IOCTL_GNTDEV_UNMAP_GRANT_REF arguments,
# HG changeset patch
# User Keir Fraser <keir@xxxxxxx>
# Date 1292580555 0
# Node ID 5a403282fd93f8df9e7953358bf5c118daf033b9
# Parent e4ee8e5b6f937f6c848aae567c9b7ff63acb41f4
xen/gntdev: add range check for IOCTL_GNTDEV_UNMAP_GRANT_REF arguments,
..., move all user memory accesses out of semaphore protected region,
simplify, clean up, and remove some bogus printk()-s in gntdev_ioctl()
and its helpers.
The purpose of free_list_sem seems questionable: It is only ever being
acquired for write access, and always with grants_sem already held
in write mode.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
---
drivers/xen/gntdev/gntdev.c | 207 +++++++++++++++++---------------------------
1 files changed, 81 insertions(+), 126 deletions(-)
diff -r e4ee8e5b6f93 -r 5a403282fd93 drivers/xen/gntdev/gntdev.c
--- a/drivers/xen/gntdev/gntdev.c Fri Dec 17 10:08:41 2010 +0000
+++ b/drivers/xen/gntdev/gntdev.c Fri Dec 17 10:09:15 2010 +0000
@@ -203,18 +203,11 @@ static inline unsigned long get_kernel_v
* value of *offset to the offset that should be mmap()-ed in order to map the
* grant reference.
*/
-static int add_grant_reference(struct file *flip,
+static int add_grant_reference(gntdev_file_private_data_t *private_data,
struct ioctl_gntdev_grant_ref *op,
uint64_t *offset)
{
- gntdev_file_private_data_t *private_data
- = (gntdev_file_private_data_t *) flip->private_data;
-
uint32_t slot_index;
-
- if (unlikely(private_data->free_list_size == 0)) {
- return -ENOMEM;
- }
slot_index = private_data->free_list[--private_data->free_list_size];
private_data->free_list[private_data->free_list_size]
@@ -239,19 +232,17 @@ static int add_grant_reference(struct fi
* previous invocation of find_contiguous_free_range(), during the same
* invocation of the driver.
*/
-static int add_grant_references(struct file *flip,
- int count,
+static int add_grant_references(gntdev_file_private_data_t *private_data,
+ uint32_t count,
struct ioctl_gntdev_grant_ref *ops,
uint32_t first_slot)
{
- gntdev_file_private_data_t *private_data
- = (gntdev_file_private_data_t *) flip->private_data;
- int i;
+ uint32_t i;
for (i = 0; i < count; ++i) {
/* First, mark the slot's entry in the free list as invalid. */
- int free_list_index =
+ uint32_t free_list_index =
private_data->grants[first_slot+i].u.free_list_index;
private_data->free_list[free_list_index] =
GNTDEV_FREE_LIST_INVALID;
@@ -271,16 +262,16 @@ static int add_grant_references(struct f
* GNTDEV_SLOT_INVALID. This will reduce the recorded size of the free list to
* the number of valid entries.
*/
-static void compress_free_list(struct file *flip)
-{
- gntdev_file_private_data_t *private_data
- = (gntdev_file_private_data_t *) flip->private_data;
- int i, j = 0, old_size, slot_index;
+static void compress_free_list(gntdev_file_private_data_t *private_data)
+{
+ uint32_t i, j = 0, old_size;
old_size = private_data->free_list_size;
for (i = 0; i < old_size; ++i) {
if (private_data->free_list[i] != GNTDEV_FREE_LIST_INVALID) {
if (i > j) {
+ int32_t slot_index;
+
slot_index = private_data->free_list[i];
private_data->free_list[j] = slot_index;
private_data->grants[slot_index].u
@@ -300,19 +291,11 @@ static void compress_free_list(struct fi
*
* Returns the index of the first slot if a range is found, otherwise -ENOMEM.
*/
-static int find_contiguous_free_range(struct file *flip,
+static int find_contiguous_free_range(gntdev_file_private_data_t *private_data,
uint32_t num_slots)
{
- gntdev_file_private_data_t *private_data
- = (gntdev_file_private_data_t *) flip->private_data;
-
- int i;
- int start_index = private_data->next_fit_index;
- int range_start = 0, range_length;
-
- if (private_data->free_list_size < num_slots) {
- return -ENOMEM;
- }
+ uint32_t i, start_index = private_data->next_fit_index;
+ uint32_t range_start = 0, range_length;
/* First search from the start_index to the end of the array. */
range_length = 0;
@@ -872,89 +855,82 @@ private_data_initialised:
case IOCTL_GNTDEV_MAP_GRANT_REF:
{
struct ioctl_gntdev_map_grant_ref op;
+ struct ioctl_gntdev_grant_ref *refs = NULL;
+
+ if (copy_from_user(&op, (void __user *)arg, sizeof(op)))
+ return -EFAULT;
+ if (unlikely(op.count <= 0))
+ return -EINVAL;
+
+ if (op.count > 1 && op.count <= private_data->grants_size) {
+ struct ioctl_gntdev_grant_ref *u;
+
+ refs = kmalloc(op.count * sizeof(*refs), GFP_KERNEL);
+ if (!refs)
+ return -ENOMEM;
+ u = ((struct ioctl_gntdev_map_grant_ref *)arg)->refs;
+ if (copy_from_user(refs, (void __user *)u,
+ sizeof(*refs) * op.count)) {
+ kfree(refs);
+ return -EFAULT;
+ }
+ }
+
down_write(&private_data->grants_sem);
down_write(&private_data->free_list_sem);
- if ((rc = copy_from_user(&op, (void __user *) arg,
- sizeof(op)))) {
- rc = -EFAULT;
+ if (unlikely(op.count > private_data->free_list_size)) {
+ rc = -ENOMEM;
goto map_out;
}
- if (unlikely(op.count <= 0)) {
- rc = -EINVAL;
- goto map_out;
- }
if (op.count == 1) {
- if ((rc = add_grant_reference(flip, &op.refs[0],
+ if ((rc = add_grant_reference(private_data, op.refs,
&op.index)) < 0) {
printk(KERN_ERR "Adding grant reference "
"failed (%d).\n", rc);
goto map_out;
}
} else {
- struct ioctl_gntdev_grant_ref *refs, *u;
- refs = kmalloc(op.count * sizeof(*refs), GFP_KERNEL);
- if (!refs) {
- rc = -ENOMEM;
- goto map_out;
- }
- u = ((struct ioctl_gntdev_map_grant_ref *)arg)->refs;
- if ((rc = copy_from_user(refs,
- (void __user *)u,
- sizeof(*refs) * op.count))) {
- printk(KERN_ERR "Copying refs from user failed"
- " (%d).\n", rc);
- rc = -EINVAL;
- goto map_out;
- }
- if ((rc = find_contiguous_free_range(flip, op.count))
- < 0) {
+ if ((rc = find_contiguous_free_range(private_data,
+ op.count)) < 0) {
printk(KERN_ERR "Finding contiguous range "
"failed (%d).\n", rc);
- kfree(refs);
goto map_out;
}
op.index = rc << PAGE_SHIFT;
- if ((rc = add_grant_references(flip, op.count,
+ if ((rc = add_grant_references(private_data, op.count,
refs, rc))) {
printk(KERN_ERR "Adding grant references "
"failed (%d).\n", rc);
- kfree(refs);
goto map_out;
}
- compress_free_list(flip);
- kfree(refs);
- }
- if ((rc = copy_to_user((void __user *) arg,
- &op,
- sizeof(op)))) {
- printk(KERN_ERR "Copying result back to user failed "
- "(%d)\n", rc);
+ compress_free_list(private_data);
+ }
+
+ map_out:
+ up_write(&private_data->free_list_sem);
+ up_write(&private_data->grants_sem);
+
+ kfree(refs);
+
+ if (!rc && copy_to_user((void __user *)arg, &op, sizeof(op)))
rc = -EFAULT;
- goto map_out;
- }
- map_out:
- up_write(&private_data->grants_sem);
- up_write(&private_data->free_list_sem);
return rc;
}
case IOCTL_GNTDEV_UNMAP_GRANT_REF:
{
struct ioctl_gntdev_unmap_grant_ref op;
- int i, start_index;
+ uint32_t i, start_index;
+
+ if (copy_from_user(&op, (void __user *)arg, sizeof(op)))
+ return -EFAULT;
+
+ start_index = op.index >> PAGE_SHIFT;
+ if (start_index + op.count > private_data->grants_size)
+ return -EINVAL;
down_write(&private_data->grants_sem);
- down_write(&private_data->free_list_sem);
-
- if ((rc = copy_from_user(&op,
- (void __user *) arg,
- sizeof(op)))) {
- rc = -EFAULT;
- goto unmap_out;
- }
-
- start_index = op.index >> PAGE_SHIFT;
/* First, check that all pages are in the NOT_YET_MAPPED
* state.
@@ -984,6 +960,8 @@ private_data_initialised:
}
}
+ down_write(&private_data->free_list_sem);
+
/* Unmap pages and add them to the free list.
*/
for (i = 0; i < op.count; ++i) {
@@ -996,9 +974,9 @@ private_data_initialised:
++private_data->free_list_size;
}
+ up_write(&private_data->free_list_sem);
unmap_out:
up_write(&private_data->grants_sem);
- up_write(&private_data->free_list_sem);
return rc;
}
case IOCTL_GNTDEV_GET_OFFSET_FOR_VADDR:
@@ -1007,25 +985,16 @@ private_data_initialised:
struct vm_area_struct *vma;
unsigned long vaddr;
- if ((rc = copy_from_user(&op,
- (void __user *) arg,
- sizeof(op)))) {
+ if (copy_from_user(&op, (void __user *)arg, sizeof(op)))
+ return -EFAULT;
+
+ vaddr = (unsigned long)op.vaddr;
+
+ down_read(¤t->mm->mmap_sem);
+ vma = find_vma(current->mm, vaddr);
+ if (!vma || vma->vm_ops != &gntdev_vmops) {
rc = -EFAULT;
goto get_offset_out;
- }
- vaddr = (unsigned long)op.vaddr;
-
- down_read(¤t->mm->mmap_sem);
- vma = find_vma(current->mm, vaddr);
- if (vma == NULL) {
- rc = -EFAULT;
- goto get_offset_unlock_out;
- }
- if ((!vma->vm_ops) || (vma->vm_ops != &gntdev_vmops)) {
- printk(KERN_ERR "The vaddr specified does not belong "
- "to a gntdev instance: %#lx\n", vaddr);
- rc = -EFAULT;
- goto get_offset_unlock_out;
}
if (vma->vm_start != vaddr) {
printk(KERN_ERR "The vaddr specified in an "
@@ -1034,45 +1003,31 @@ private_data_initialised:
"%#lx; vaddr = %#lx\n",
vma->vm_start, vaddr);
rc = -EFAULT;
- goto get_offset_unlock_out;
+ goto get_offset_out;
}
op.offset = vma->vm_pgoff << PAGE_SHIFT;
op.count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+ get_offset_out:
up_read(¤t->mm->mmap_sem);
- if ((rc = copy_to_user((void __user *) arg,
- &op,
- sizeof(op)))) {
+ if (!rc && copy_to_user((void __user *)arg, &op, sizeof(op)))
rc = -EFAULT;
- goto get_offset_out;
- }
- goto get_offset_out;
- get_offset_unlock_out:
- up_read(¤t->mm->mmap_sem);
- get_offset_out:
return rc;
}
case IOCTL_GNTDEV_SET_MAX_GRANTS:
{
struct ioctl_gntdev_set_max_grants op;
- if ((rc = copy_from_user(&op,
- (void __user *) arg,
- sizeof(op)))) {
- rc = -EFAULT;
- goto set_max_out;
- }
+
+ if (copy_from_user(&op, (void __user *)arg, sizeof(op)))
+ return -EFAULT;
+ if (op.count > MAX_GRANTS_LIMIT)
+ return -EINVAL;
+
down_write(&private_data->grants_sem);
- if (private_data->grants) {
+ if (unlikely(private_data->grants))
rc = -EBUSY;
- goto set_max_unlock_out;
- }
- if (op.count > MAX_GRANTS_LIMIT) {
- rc = -EINVAL;
- goto set_max_unlock_out;
- }
- rc = init_private_data(private_data, op.count);
- set_max_unlock_out:
+ else
+ rc = init_private_data(private_data, op.count);
up_write(&private_data->grants_sem);
- set_max_out:
return rc;
}
default:
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |