linux-kernel - Re: [PATCH] mm: deferred_init

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:   Fri, 6 Oct 2017 09:55:49 -0400
From:   Pasha Tatashin <pasha.tatashin@...cle.com>
To:     Anshuman Khandual <khandual@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org, sparclinux@...r.kernel.org,
        linux-mm@...ck.org, linuxppc-dev@...ts.ozlabs.org,
        linux-s390@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        x86@...nel.org, kasan-dev@...glegroups.com, borntraeger@...ibm.com,
        heiko.carstens@...ibm.com, davem@...emloft.net,
        willy@...radead.org, mhocko@...nel.org, ard.biesheuvel@...aro.org,
        mark.rutland@....com, will.deacon@....com, catalin.marinas@....com,
        sam@...nborg.org, mgorman@...hsingularity.net,
        steven.sistare@...cle.com, daniel.m.jordan@...cle.com,
        bob.picco@...cle.com
Subject: Re: [PATCH] mm: deferred_init_memmap improvements

Hi Anshuman,

Thank you very much for looking at this. My reply below::

On 10/06/2017 02:48 AM, Anshuman Khandual wrote:
> On 10/04/2017 08:59 PM, Pavel Tatashin wrote:
>> This patch fixes another existing issue on systems that have holes in
>> zones i.e CONFIG_HOLES_IN_ZONE is defined.
>>
>> In for_each_mem_pfn_range() we have code like this:
>>
>> if (!pfn_valid_within(pfn)
>> 	goto free_range;
>>
>> Note: 'page' is not set to NULL and is not incremented but 'pfn' advances.
> 
> page is initialized to NULL at the beginning of the function.

Yes, it is initialized to NULL but at the beginning of 
for_each_mem_pfn_range() loop

> PFN advances but we dont proceed unless pfn_valid_within(pfn)
> holds true which basically should have checked with arch call
> back if the PFN is valid in presence of memory holes as well.
> Is not this correct ?

Correct, if pfn_valid_within() is false we jump to the "goto 
free_range;", which is at the end of for (; pfn < end_pfn; pfn++) loop, 
so we are not jumping outside of this loop.

> 
>> Thus means if deferred struct pages are enabled on systems with these kind
>> of holes, linux would get memory corruptions. I have fixed this issue by
>> defining a new macro that performs all the necessary operations when we
>> free the current set of pages.
> 
> If we bail out in case PFN is not valid, then how corruption
> can happen ?
> 

We are not bailing out. We continue next iteration with next pfn, but 
page is not incremented.

Please let me know if I am missing something.

Thank you,
Pasha