linux-kernel - Re: [RFC PATCH v2] mm: initialize struct pages reserved by ZONE

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: <c0dbddc7-6d43-bc6f-b4fd-e413af292857@redhat.com>
Date:   Tue, 10 Sep 2019 12:10:00 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     Toshiki Fukasawa <t-fukasawa@...jp.nec.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "mhocko@...nel.org" <mhocko@...nel.org>,
        "adobriyan@...il.com" <adobriyan@...il.com>,
        "hch@....de" <hch@....de>,
        "longman@...hat.com" <longman@...hat.com>,
        "sfr@...b.auug.org.au" <sfr@...b.auug.org.au>,
        "mst@...hat.com" <mst@...hat.com>,
        Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
        Junichi Nomura <j-nomura@...jp.nec.com>
Subject: Re: [RFC PATCH v2] mm: initialize struct pages reserved by
 ZONE_DEVICE driver.

On 10.09.19 11:21, Dan Williams wrote:
> On Mon, Sep 9, 2019 at 5:06 AM David Hildenbrand <david@...hat.com> wrote:
>>
>> On 09.09.19 13:53, Dan Williams wrote:
>>> On Mon, Sep 9, 2019 at 1:11 AM David Hildenbrand <david@...hat.com> wrote:
>>> [..]
>>>>>> It seems that SECTION_IS_ONLINE and SECTION_MARKED_PRESENT can be used to
>>>>>> distinguish uninitialized struct pages if we can apply them to ZONE_DEVICE,
>>>>>> but that is no longer necessary with this approach.
>>>>>
>>>>> Let's take a step back here to understand the issues I am aware of. I
>>>>> think we should solve this for good now:
>>>>>
>>>>> A PFN walker takes a look at a random PFN at a random point in time. It
>>>>> finds a PFN with SECTION_MARKED_PRESENT && !SECTION_IS_ONLINE. The
>>>>> options are:
>>>>>
>>>>> 1. It is buddy memory (add_memory()) that has not been online yet. The
>>>>> memmap contains garbage. Don't access.
>>>>>
>>>>> 2. It is ZONE_DEVICE memory with a valid memmap. Access it.
>>>>>
>>>>> 3. It is ZONE_DEVICE memory with an invalid memmap, because the section
>>>>> is only partially present: E.g., device starts at offset 64MB within a
>>>>> section or the device ends at offset 64MB within a section. Don't access it.
>>>>>
>>>>> 4. It is ZONE_DEVICE memory with an invalid memmap, because the memmap
>>>>> was not initialized yet. memmap_init_zone_device() did not yet succeed
>>>>> after dropping the mem_hotplug lock in mm/memremap.c. Don't access it.
>>>>>
>>>>> 5. It is reserved ZONE_DEVICE memory ("pages mapped, but reserved for
>>>>> driver") with an invalid memmap. Don't access it.
>>>>>
>>>>> I can see that your patch tries to make #5 vanish by initializing the
>>>>> memmap, fair enough. #3 and #4 can't be detected. The PFN walker could
>>>>> still stumble over uninitialized memmaps.
>>>>>
>>>>
>>>> FWIW, I thinkg having something like pfn_zone_device(), similarly
>>>> implemented like pfn_zone_device_reserved() could be one solution to
>>>> most issues.
>>>
>>> I've been thinking of a replacement for PTE_DEVMAP with section-level,
>>> or sub-section level flags. The section-level flag would still require
>>> a call to get_dev_pagemap() to validate that the pfn is not section in
>>> the subsection case which seems to be entirely too much overhead. If
>>> ZONE_DEVICE is to be a first class citizen in pfn walkers I think it
>>> would be worth the cost to double the size of subsection_map and to
>>> identify whether a sub-section is ZONE_DEVICE, or not.
>>>
>>> Thoughts?
>>>
>>
>> I thought about this last week and came up with something like
>>
>> 1. Convert SECTION_IS_ONLINE to SECTION IS_ACTIVE
>>
>> 2. Make pfn_to_online_page() also check that it's not ZONE_DEVICE.
>> Online pfns are limited to !ZONE_DEVICE.
>>
>> 3. Extend subsection_map to an additional active_map
>>
>> 4. Set SECTION IS_ACTIVE *iff* the whole active_map is set. This keeps
>> most accesses of pfn_to_online_page() fast. If !SECTION IS_ACTIVE, check
>> the active_map.
>>
>> 5. Set sub-sections active/unactive in
>> move_pfn_range_to_zone()/remove_pfn_range_from_zone() - see "[PATCH v4
>> 0/8] mm/memory_hotplug: Shrink zones before removing memory" for the
>> latter.
>>
>> 6. Set boot memory properly active (this is a tricky bit :/ ).
>>
>> However, it turned out too complex for my taste (and limited time to
>> spend on this), so I abandoned that idea for now. If somebody wants to
>> pick that up, fine.
>>
> 
> That seems to solve the pfn walk case but it would not address the
> need for PTE_DEVMAP or speed up the other places that want an
> efficient way to determine if it's worthwhile to call
> get_dev_pagemap().
> 

Then I probably didn't get how your suggestion would look like :)

Would you suggest - instead of reusing SECTION_IS_ONLINE - something
like SECTION_IS_DEVMAP / SECTION_IS_DEVICE, to decide whether to call
get_dev_pagemap()?

How to deal with subsections? I would like to avoid storing subsection
information only useful for ZONE_DEVICE for any kind of sections (or
could we then simply not store the information per subsection but
instead check the device as initially suggested by me?)

-- 

Thanks,

David / dhildenb