Re: general protection fault in unlink_file_vma

linmiaohe
2020-09-10T06:27:03Z

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.