On Tue, 14 Jul 2020 11:07:44 +0800 linmiaohe <linmiaohe@xxxxxxxxxx> wrote:
> From: Miaohe Lin <linmiaohe@xxxxxxxxxx>
>
> The vm_flags may be changed after call_mmap() because drivers may set some
> flags for their own purpose. As a result, we failed to merge the adjacent
> vma due to the different vm_flags as userspace can't pass in the same one.
> Try to merge vma after call_mmap() to fix this issue.
>
> Signed-off-by: Hongxiang Lou <louhongxiang@xxxxxxxxxx>
> Signed-off-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
> ---
> mm/mmap.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 59a4682ebf3f..9568117471f8 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1689,7 +1689,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> struct list_head *uf)
> {
> struct mm_struct *mm = current->mm;
> - struct vm_area_struct *vma, *prev;
> + struct vm_area_struct *vma, *prev, *merge;
> int error;
> struct rb_node **rb_link, *rb_parent;
> unsigned long charged = 0;
> @@ -1773,6 +1773,20 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> if (error)
> goto unmap_and_free_vma;
>
> + /* If vm_flags changed after call_mmap(), we should try merge vma again
> + * as we may succeed this time.
> + */
> + if (unlikely(vm_flags != vma->vm_flags && prev)) {
> + merge = vma_merge(mm, prev, vma->vm_start, vma->vm_end, vma->vm_flags,
> + NULL, vma->vm_file, vma->vm_pgoff, NULL, NULL_VM_UFFD_CTX);
> + if (merge) {
> + fput(file);
> + vm_area_free(vma);
> + vma = merge;
> + goto unmap_writable;
Shouldn't we update local variable `vm_flags' here, to pick up the
change? And possibly `addr'?
> + }
> + }
> +
> /* Can addr have changed??
> *
> * Answer: Yes, several device drivers can do it in their
> @@ -1795,6 +1809,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> vma_link(mm, vma, prev, rb_link, rb_parent);
> /* Once vma denies write, undo our temporary denial count */
> if (file) {
> +unmap_writable:
> if (vm_flags & VM_SHARED)
> mapping_unmap_writable(file->f_mapping);
> if (vm_flags & VM_DENYWRITE)
