linux-kernel - Re: [v6 05/15] mm: don't accessed uninitialized struct pages

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: <139b7d83-12a3-d584-0461-d01a79df5d2b@oracle.com>
Date:   Thu, 17 Aug 2017 11:28:23 -0400
From:   Pasha Tatashin <pasha.tatashin@...cle.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     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, ard.biesheuvel@...aro.org,
        will.deacon@....com, catalin.marinas@....com, sam@...nborg.org
Subject: Re: [v6 05/15] mm: don't accessed uninitialized struct pages

Hi Michal,

I've been looking through this code again, and I think your suggestion 
will work. I did not realize this iterator already exist:

for_each_free_mem_range() basically iterates through (memory && !reserved)

This is exactly what we need here. So, I will update this patch to use 
this iterator, which will simplify it.

Pasha

On 08/14/2017 09:51 AM, Pasha Tatashin wrote:
>>> mem_init()
>>>   free_all_bootmem()
>>>    free_low_memory_core_early()
>>>     for_each_reserved_mem_region()
>>>      reserve_bootmem_region()
>>>       init_reserved_page() <- if this is deferred reserved page
>>>        __init_single_pfn()
>>>         __init_single_page()
>>>
>>> So, currently, we are using the value of page->flags to figure out if 
>>> this
>>> page has been initialized while being part of deferred page, but this 
>>> is not
>>> going to work for this project, as we do not zero the memory that is 
>>> backing
>>> the struct pages, and size the value of page->flags can be anything.
>>
>> True, this is the initialization part I've missed in one of the previous
>> patches already. Would it be possible to only iterate over !reserved
>> memory blocks instead? Now that we discard all the metadata later it
>> should be quite easy to do for_each_memblock_type, no?
> 
> Hi Michal,
> 
> Clever suggestion to add a new iterator to go through unreserved 
> existing memory, I do not think there is this iterator available, so it 
> would need to be implemented, using similar approach to what I have done 
> with a call back.
> 
> However, there is a different reason, why I took this current approach.
> 
> Daniel Jordan is working on a ktask support:
> https://lkml.org/lkml/2017/7/14/666
> 
> He and I discussed on how to multi-thread struct pages initialization 
> within memory nodes using ktasks. Having this callback interface makes 
> that multi-threading quiet easy, improving the boot performance further, 
> with his prototype we saw x4-6 improvements (using 4-8 threads per 
> node). Reducing the total time it takes to initialize all struct pages 
> on machines with terabytes of memory to less than one second.
> 
> Pasha