| Message ID | 20181224132751.GA22184@jordon-HP-15-Notebook-PC (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use vm_insert_range | expand |
On Mon, Dec 24, 2018 at 6:53 PM Souptick Joarder <jrdr.linux@gmail.com> wrote: > > Convert to use vm_insert_range() to map range of kernel > memory to user vma. > > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> > Reviewed-by: Matthew Wilcox <willy@infradead.org> > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > --- > drivers/xen/gntdev.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c > index b0b02a5..430d4cb 100644 > --- a/drivers/xen/gntdev.c > +++ b/drivers/xen/gntdev.c > @@ -1084,7 +1084,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) > int index = vma->vm_pgoff; > int count = vma_pages(vma); > struct gntdev_grant_map *map; > - int i, err = -EINVAL; > + int err = -EINVAL; > > if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags & VM_SHARED)) > return -EINVAL; > @@ -1145,12 +1145,9 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) > goto out_put_map; > > if (!use_ptemod) { > - for (i = 0; i < count; i++) { > - err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE, > - map->pages[i]); > - if (err) > - goto out_put_map; > - } Looking into the original code, the loop should run from i =0 to *i < map->count*. There is no error check for *count > map->count* and we might end up overrun the map->pages[i] boundary. While converting this code with suggested vm_insert_range(), this can be fixed. > + err = vm_insert_range(vma, vma->vm_start, map->pages, count); > + if (err) > + goto out_put_map; > } else { > #ifdef CONFIG_X86 > /* > -- > 1.9.1 >
On 1/2/19 1:58 PM, Souptick Joarder wrote: > On Mon, Dec 24, 2018 at 6:53 PM Souptick Joarder <jrdr.linux@gmail.com> wrote: >> Convert to use vm_insert_range() to map range of kernel >> memory to user vma. >> >> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> >> Reviewed-by: Matthew Wilcox <willy@infradead.org> >> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> >> --- >> drivers/xen/gntdev.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c >> index b0b02a5..430d4cb 100644 >> --- a/drivers/xen/gntdev.c >> +++ b/drivers/xen/gntdev.c >> @@ -1084,7 +1084,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) >> int index = vma->vm_pgoff; >> int count = vma_pages(vma); >> struct gntdev_grant_map *map; >> - int i, err = -EINVAL; >> + int err = -EINVAL; >> >> if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags & VM_SHARED)) >> return -EINVAL; >> @@ -1145,12 +1145,9 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) >> goto out_put_map; >> >> if (!use_ptemod) { >> - for (i = 0; i < count; i++) { >> - err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE, >> - map->pages[i]); >> - if (err) >> - goto out_put_map; >> - } > Looking into the original code, the loop should run from i =0 to *i < > map->count*. > There is no error check for *count > map->count* and we might end up > overrun the map->pages[i] boundary. I don't think we can have map->count != count (see gntdev_find_map_index()). But for clarity I agree using map->count might be better. > > While converting this code with suggested vm_insert_range(), this can be fixed. And count can be dropped altogether. Thanks. -boris > > >> + err = vm_insert_range(vma, vma->vm_start, map->pages, count); >> + if (err) >> + goto out_put_map; >> } else { >> #ifdef CONFIG_X86 >> /* >> -- >> 1.9.1 >>
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index b0b02a5..430d4cb 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -1084,7 +1084,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) int index = vma->vm_pgoff; int count = vma_pages(vma); struct gntdev_grant_map *map; - int i, err = -EINVAL; + int err = -EINVAL; if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags & VM_SHARED)) return -EINVAL; @@ -1145,12 +1145,9 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) goto out_put_map; if (!use_ptemod) { - for (i = 0; i < count; i++) { - err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE, - map->pages[i]); - if (err) - goto out_put_map; - } + err = vm_insert_range(vma, vma->vm_start, map->pages, count); + if (err) + goto out_put_map; } else { #ifdef CONFIG_X86 /*