Re: general protection fault in unlink_file_vma

[linmiaohe]

Hillf Danton wrote:
>> On Thu, 10 Sep 2020 07:43:41 +0530 Souptick Joarder wrote:
>> On Wed, Sep 9, 2020 at 9:45 AM Hillf Danton wrote:
>> > Tue, 08 Sep 2020 17:19:17 -0700
>> > > syzbot found the following issue on:
>> > >
>> > > HEAD commit:    59126901 Merge tag 'perf-tools-fixes-for-v5.9-2020-09-03' ..
>> >
>> > Looks like d70cec898324 ("mm: mmap: merge vma after call_mmap() if 
>> > possible") added an extra fput.
>> 
>> Can you please help me to understand how do you figure out this commit ?
>
>Feel free to correct Hillf if I missed any thing.
>Failing to reproduce the gpf without the commit may tell us more about it than I could.
>> >
>> > --- a/mm/mmap.c
>> > +++ b/mm/mmap.c
>> > @@ -1781,7 +1781,6 @@ unsigned long mmap_region(struct file *f
>> >                         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;
>> >                                 /* Update vm_flags and possible addr 
>> > to pick up the change. We don't

Yes, It seems vma_merge() could fput the vm_file via remove_next case in __vma_adjust(). So the fput vm_file here do the
extra one.

But we may not remove the fput here directly as vma_merge() do not always fput the vm_file. I'am not really familiar with
the vma merge yet, but I would try my best to fix this as soon as possible.

Many thanks for point this out. And sorry for my careless.