linux-kernel - Re: [RFC] mm/mremap: Remove redundant checks inside vma

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

Date:   Wed, 19 Jul 2017 12:19:04 +0530
From:   Anshuman Khandual <khandual@...ux.vnet.ibm.com>
To:     Michal Hocko <mhocko@...nel.org>,
        Anshuman Khandual <khandual@...ux.vnet.ibm.com>
Cc:     Vlastimil Babka <vbabka@...e.cz>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
        mike.kravetz@...cle.com
Subject: Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable()

On 07/11/2017 04:52 PM, Michal Hocko wrote:
> On Tue 11-07-17 16:38:46, Anshuman Khandual wrote:
>> On 07/11/2017 12:26 PM, Vlastimil Babka wrote:
>>> On 07/11/2017 08:50 AM, Michal Hocko wrote:
>>>> On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
>>>>> On 07/11/2017 08:03 AM, Michal Hocko wrote:
>>>>>> Are you telling me that two if conditions cause more than a second
>>>>>> difference? That sounds suspicious.
>>>>> It's removing also a call to get_unmapped_area(), AFAICS. That means a
>>>>> vma search?
>>>> Ohh, right. I have somehow missed that. Is this removal intentional?
>>> I think it is: "Checking for availability of virtual address range at
>>> the end of the VMA for the incremental size is also reduntant at this
>>> point."
>>>
>>>> The
>>>> changelog is silent about it.
>>> It doesn't explain why it's redundant, indeed. Unfortunately, the commit
>>> f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
>>> also doesn't explain why it's needed.
>> Its redundant because there are calls to get_unmapped_area() down the
>> line in the function whose failure will anyway fail the expansion of
>> the VMA.
> mremap code is quite complex and I am not sure you are right here.
> Anyway, please make sure you document why you believe those checks are
> not needed when resubmitting your patch.

vma_adjust() expands/contracts the given VMA and adjusts everything
around it like anon vma, rb tree, statistics etc but it never checks
whether the requested expansion/contraction is OK from arch point of
view by calling get_unmapped_area() some where. IIUC now the function
vma_expandable() does that check for it beforehand. Hence I think its
required. My tests happened to pass may be because of coincidence that
every thing (all the VA space) was up for grabs at that point of time.
So, will drop this patch.