Commit 309d08d9 authored by Liu Zixian's avatar Liu Zixian Committed by Linus Torvalds

mm/mmap.c: fix mmap return value when vma is merged after call_mmap()

On success, mmap should return the begin address of newly mapped area,
but patch "mm: mmap: merge vma after call_mmap() if possible" set
vm_start of newly merged vma to return value addr.  Users of mmap will
get wrong address if vma is merged after call_mmap().  We fix this by
moving the assignment to addr before merging vma.

We have a driver which changes vm_flags, and this bug is found by our
testcases.

Fixes: d70cec89 ("mm: mmap: merge vma after call_mmap() if possible")
Signed-off-by: default avatarLiu Zixian <liuzixian4@huawei.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Reviewed-by: default avatarJason Gunthorpe <jgg@nvidia.com>
Reviewed-by: default avatarDavid Hildenbrand <david@redhat.com>
Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: Hongxiang Lou <louhongxiang@huawei.com>
Cc: Hu Shiyuan <hushiyuan@huawei.com>
Cc: Matthew Wilcox <willy@infradead.org>
Link: https://lkml.kernel.org/r/20201203085350.22624-1-liuzixian4@huawei.comSigned-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 7a5bde37
...@@ -1808,6 +1808,17 @@ unsigned long mmap_region(struct file *file, unsigned long addr, ...@@ -1808,6 +1808,17 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
if (error) if (error)
goto unmap_and_free_vma; goto unmap_and_free_vma;
/* Can addr have changed??
*
* Answer: Yes, several device drivers can do it in their
* f_op->mmap method. -DaveM
* Bug: If addr is changed, prev, rb_link, rb_parent should
* be updated for vma_link()
*/
WARN_ON_ONCE(addr != vma->vm_start);
addr = vma->vm_start;
/* If vm_flags changed after call_mmap(), we should try merge vma again /* If vm_flags changed after call_mmap(), we should try merge vma again
* as we may succeed this time. * as we may succeed this time.
*/ */
...@@ -1822,25 +1833,12 @@ unsigned long mmap_region(struct file *file, unsigned long addr, ...@@ -1822,25 +1833,12 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
fput(vma->vm_file); fput(vma->vm_file);
vm_area_free(vma); vm_area_free(vma);
vma = merge; vma = merge;
/* Update vm_flags and possible addr to pick up the change. We don't /* Update vm_flags to pick up the change. */
* warn here if addr changed as the vma is not linked by vma_link().
*/
addr = vma->vm_start;
vm_flags = vma->vm_flags; vm_flags = vma->vm_flags;
goto unmap_writable; goto unmap_writable;
} }
} }
/* Can addr have changed??
*
* Answer: Yes, several device drivers can do it in their
* f_op->mmap method. -DaveM
* Bug: If addr is changed, prev, rb_link, rb_parent should
* be updated for vma_link()
*/
WARN_ON_ONCE(addr != vma->vm_start);
addr = vma->vm_start;
vm_flags = vma->vm_flags; vm_flags = vma->vm_flags;
} else if (vm_flags & VM_SHARED) { } else if (vm_flags & VM_SHARED) {
error = shmem_zero_setup(vma); error = shmem_zero_setup(vma);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment