linux-kernel - Re: [RFC v9 PATCH 2/4] mm: mmap: zap pages with read mmap

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b69d3f7d-e9ba-b95c-45cd-44489950751b@linux.alibaba.com>
Date:   Tue, 11 Sep 2018 16:35:03 -0700
From:   Yang Shi <yang.shi@...ux.alibaba.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     mhocko@...nel.org, ldufour@...ux.vnet.ibm.com, vbabka@...e.cz,
        akpm@...ux-foundation.org, dave.hansen@...el.com, oleg@...hat.com,
        srikar@...ux.vnet.ibm.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC v9 PATCH 2/4] mm: mmap: zap pages with read mmap_sem in
 munmap

On 9/11/18 2:16 PM, Matthew Wilcox wrote:
> On Wed, Sep 12, 2018 at 04:58:11AM +0800, Yang Shi wrote:
>>   mm/mmap.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> I really think you're going about this the wrong way by duplicating
> vm_munmap().

If we don't duplicate vm_munmap() or do_munmap(), we need pass an extra 
parameter to them to tell when it is fine to downgrade write lock or if 
the lock has been acquired outside it (i.e. in mmap()/mremap()), right? 
But, vm_munmap() or do_munmap() is called not only by mmap-related, but 
also some other places, like arch-specific places, which don't need 
downgrade write lock or are not safe to do so.

Actually, I did this way in the v1 patches, but it got pushed back by 
tglx who suggested duplicate the code so that the change could be done 
in mm only without touching other files, i.e. arch-specific stuff. I 
didn't have strong argument to convince him.

And, Michal prefers have VM_HUGETLB and VM_PFNMAP handled separately for 
safe and bisectable sake, which needs call the regular do_munmap().

In addition to this, I just found mpx code may call do_munmap() 
recursively when I was looking into the mpx code.

We might be able to handle these by the extra parameter, but it sounds 
it make the code hard to understand and error prone.

Thanks,
Yang