On Sun, 13 Sep 2020 09:17:26 +0000 linmiaohe wrote:
>
> I reviewed the code carefully these days and I found vma_merge() do only fput() the vm_file of the linked vma in remove_next cases.
> This gpf is much likely because the ->mmap() callback can change vma->vm_file and fput the original file. But my previous commit
> failed to catch this case and always fput() the original file, hence add an extra fput().
> The below patch would make the things right:
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 080f44bcf7a8..80ea11bf12fa 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1816,7 +1816,11 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> 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);
> + /* ->mmap() can change vma->vm_file and fput the original file. So
> + * fput the vma->vm_file here or we would add an extra fput for file
> + * and cause general protection fault ultimately.
> + */
> + fput(vma->vm_file);
> vm_area_free(vma);
> vma = merge;
> /* Update vm_flags and possible addr to pick up the change. We don't
>
> It's very nice of you if you could help test this patch as I can't reproduce it in my product environment. And many thanks
> for a possible Tested-by tag.
Take another look at the Cc list and the link below.
https://lore.kernel.org/lkml/20200911120222.GT87483@xxxxxxxx/
