linux-kernel - Re: general protection fault in unlink_file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <bc09fdb2ce24487b821bccc856c4c23b@huawei.com>
Date:   Thu, 10 Sep 2020 06:26:45 +0000
From:   linmiaohe <linmiaohe@...wei.com>
To:     Hillf Danton <hdanton@...a.com>
CC:     Souptick Joarder <jrdr.linux@...il.com>,
        syzbot <syzbot+c5d5a51dcbb558ca0cb5@...kaller.appspotmail.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>,
        "syzkaller-bugs@...glegroups.com" <syzkaller-bugs@...glegroups.com>
Subject: Re: general protection fault in unlink_file_vma

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.