On 12/16/2010 07:49 PM, Jeremy Fitzhardinge wrote:
> On 12/16/2010 04:17 PM, Daniel De Graaf wrote:
>> This allows userspace to perform mmap() on the gntdev device and then
>> immediately close the filehandle or remove the mapping using the
>> remove ioctl, with the mapped area remaining valid until unmapped.
>> This also fixes an infinite loop when a gntdev device is closed
>> without first unmapping all areas.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
>> ---
>> drivers/xen/gntdev.c | 67
>> +++++++++++++++++++++++--------------------------
>> 1 files changed, 31 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
>> index 6a3c9e4..f1fc8fa 100644
>> --- a/drivers/xen/gntdev.c
>> +++ b/drivers/xen/gntdev.c
>> @@ -66,16 +66,18 @@ struct granted_page {
>>
>> struct grant_map {
>> struct list_head next;
>> - struct gntdev_priv *priv;
>> struct vm_area_struct *vma;
>> int index;
>> int count;
>> + atomic_t users;
>
> Does this need to be atomic? Won't it be happening under spinlock anyway?
gntdev_put_map will not be called under spinlock if it is called from an
munmap(), especially one that happens after the file is closed.
>> @@ -517,11 +506,13 @@ static long gntdev_ioctl_unmap_grant_ref(struct
>> gntdev_priv *priv,
>>
>> spin_lock(&priv->lock);
>> map = gntdev_find_map_index(priv, op.index >> PAGE_SHIFT, op.count);
>> - if (map)
>> - err = gntdev_del_map(map);
>> + if (map) {
>> + list_del(&map->next);
>> + gntdev_put_map(map);
>> + err = 0;
>> + } else
>> + err = -EINVAL;
>
> What prevents unmap_grant_ref being called multiple times?
gntdev_find_map_index searches in priv->list for the mapping; if
found, it removes it from that list. A second search will just
return -EINVAL, even if the pages are still mapped.
>> spin_unlock(&priv->lock);
>> - if (!err)
>> - gntdev_free_map(map);
>> return err;
>> }
>>
>> @@ -599,13 +590,15 @@ static int gntdev_mmap(struct file *flip, struct
>> vm_area_struct *vma)
>> map = gntdev_find_map_index(priv, index, count);
>> if (!map)
>> goto unlock_out;
>> - if (map->vma)
>> + if (use_ptemod && map->vma)
>> goto unlock_out;
>
> Does this depend on the later hvm patch?
Whoops, that ended up in the wrong patch. I'll correct the pair.
>> if (priv->mm != vma->vm_mm) {
>> printk("%s: Huh? Other mm?\n", __FUNCTION__);
>> goto unlock_out;
>> }
>>
>> + atomic_inc(&map->users);
>> +
>> vma->vm_ops = &gntdev_vmops;
>>
>> vma->vm_flags |= VM_RESERVED;
>> @@ -614,7 +607,9 @@ static int gntdev_mmap(struct file *flip, struct
>> vm_area_struct *vma)
>> vma->vm_flags |= VM_PFNMAP;
>>
>> vma->vm_private_data = map;
>> - map->vma = vma;
>> +
>> + if (use_ptemod)
>> + map->vma = vma;
>>
>> map->is_ro = !(vma->vm_flags & VM_WRITE);
>>
>
> J
>
--
Daniel De Graaf
National Security Agency
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home