linux-kernel - Re: [PATCH v1 2/5] mm/madvise: introduce MADV_POPULATE

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: <26227fc6-3e7b-4e69-f69d-4dc2a67ecfe8@redhat.com>
Date:   Tue, 30 Mar 2021 18:30:48 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Jann Horn <jannh@...gle.com>
Cc:     kernel list <linux-kernel@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>, Michal Hocko <mhocko@...e.com>,
        Oscar Salvador <osalvador@...e.de>,
        Matthew Wilcox <willy@...radead.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Minchan Kim <minchan@...nel.org>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Dave Hansen <dave.hansen@...el.com>,
        Hugh Dickins <hughd@...gle.com>,
        Rik van Riel <riel@...riel.com>,
        "Michael S . Tsirkin" <mst@...hat.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Richard Henderson <rth@...ddle.net>,
        Ivan Kokshaysky <ink@...assic.park.msu.ru>,
        Matt Turner <mattst88@...il.com>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        "James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
        Helge Deller <deller@....de>, Chris Zankel <chris@...kel.net>,
        Max Filippov <jcmvbkbc@...il.com>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Peter Xu <peterx@...hat.com>,
        Rolf Eike Beer <eike-kernel@...tec.de>,
        linux-alpha@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-parisc@...r.kernel.org, linux-xtensa@...ux-xtensa.org,
        linux-arch <linux-arch@...r.kernel.org>,
        Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCH v1 2/5] mm/madvise: introduce MADV_POPULATE_(READ|WRITE)
 to prefault/prealloc memory

On 30.03.21 18:21, Jann Horn wrote:
> On Tue, Mar 30, 2021 at 5:01 PM David Hildenbrand <david@...hat.com> wrote:
>>>> +long faultin_vma_page_range(struct vm_area_struct *vma, unsigned long start,
>>>> +                           unsigned long end, bool write, int *locked)
>>>> +{
>>>> +       struct mm_struct *mm = vma->vm_mm;
>>>> +       unsigned long nr_pages = (end - start) / PAGE_SIZE;
>>>> +       int gup_flags;
>>>> +
>>>> +       VM_BUG_ON(!PAGE_ALIGNED(start));
>>>> +       VM_BUG_ON(!PAGE_ALIGNED(end));
>>>> +       VM_BUG_ON_VMA(start < vma->vm_start, vma);
>>>> +       VM_BUG_ON_VMA(end > vma->vm_end, vma);
>>>> +       mmap_assert_locked(mm);
>>>> +
>>>> +       /*
>>>> +        * FOLL_HWPOISON: Return -EHWPOISON instead of -EFAULT when we hit
>>>> +        *                a poisoned page.
>>>> +        * FOLL_POPULATE: Always populate memory with VM_LOCKONFAULT.
>>>> +        * !FOLL_FORCE: Require proper access permissions.
>>>> +        */
>>>> +       gup_flags = FOLL_TOUCH | FOLL_POPULATE | FOLL_MLOCK | FOLL_HWPOISON;
>>>> +       if (write)
>>>> +               gup_flags |= FOLL_WRITE;
>>>> +
>>>> +       /*
>>>> +        * See check_vma_flags(): Will return -EFAULT on incompatible mappings
>>>> +        * or with insufficient permissions.
>>>> +        */
>>>> +       return __get_user_pages(mm, start, nr_pages, gup_flags,
>>>> +                               NULL, NULL, locked);
>>>
>>> You mentioned in the commit message that you don't want to actually
>>> dirty all the file pages and force writeback; but doesn't
>>> POPULATE_WRITE still do exactly that? In follow_page_pte(), if
>>> FOLL_TOUCH and FOLL_WRITE are set, we mark the page as dirty:
>>
>> Well, I mention that POPULATE_READ explicitly doesn't do that. I
>> primarily set it because populate_vma_page_range() also sets it.
>>
>> Is it safe to *not* set it? IOW, fault something writable into a page
>> table (where the CPU could dirty it without additional page faults)
>> without marking it accessed? For me, this made logically sense. Thus I
>> also understood why populate_vma_page_range() set it.
> 
> FOLL_TOUCH doesn't have anything to do with installing the PTE - it
> essentially means "the caller of get_user_pages wants to read/write
> the contents of the returned page, so please do the same things you
> would do if userspace was accessing the page". So in particular, if
> you look up a page via get_user_pages() with FOLL_WRITE|FOLL_TOUCH,
> that tells the MM subsystem "I will be writing into this page directly
> from the kernel, bypassing the userspace page tables, so please mark
> it as dirty now so that it will be properly written back later". Part
> of that is that it marks the page as recently used, which has an
> effect on LRU pageout behavior, I think - as far as I understand, that
> is why populate_vma_page_range() uses FOLL_TOUCH.
> 
> If you look at __get_user_pages(), you can see that it is split up
> into two major parts: faultin_page() for creating PTEs, and
> follow_page_mask() for grabbing pages from PTEs. faultin_page()
> ignores FOLL_TOUCH completely; only follow_page_mask() uses it.
> 
> In a way I guess maybe you do want the "mark as recently accessed"
> part that FOLL_TOUCH would give you without FOLL_WRITE? But I think
> you very much don't want the dirtying that FOLL_TOUCH|FOLL_WRITE leads
> to. Maybe the ideal approach would be to add a new FOLL flag to say "I
> only want to mark as recently used, I don't want to dirty". Or maybe
> it's enough to just leave out the FOLL_TOUCH entirely, I don't know.

Any thoughts why populate_vma_page_range() does it?

-- 
Thanks,

David / dhildenb