linux-kernel - Re: [PATCH RFC v2 1/2] mm: Don't skip swap entry even if zap

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]

Date:   Sun, 23 Jan 2022 22:51:57 -0800 (PST)
From:   Hugh Dickins <hughd@...gle.com>
To:     Peter Xu <peterx@...hat.com>
cc:     Hugh Dickins <hughd@...gle.com>, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, David Hildenbrand <david@...hat.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Yang Shi <shy828301@...il.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alistair Popple <apopple@...dia.com>,
        "Kirill A . Shutemov" <kirill@...temov.name>,
        Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH RFC v2 1/2] mm: Don't skip swap entry even if zap_details
 specified

On Fri, 21 Jan 2022, Peter Xu wrote:
> 
> Oh, one more thing..
> 
> When reading the history and also your explanations above, I figured one thing
> that may not be right for a long time, on zero page handling of zapping.
> 
> If to quote your comment above, we should keep the zero page entries too if
> zap_details.zap_mapping is specified.  However it's not true for a long time, I
> guess starting from when vm_normal_page() returns NULL for zero pfns.  I also
> have a very strong feeling that in the old world the "page*" is non-NULL for
> zero pages here.
> 
> So... I'm boldly thinking whether we should also need another fix upon the zero
> page handling here too, probably before this whole patchset (so it'll be the
> 1st patch, it should directly apply to master) because if I'm right on above it
> can be seen as another separate bug fix:
> 
> ---8<---
> diff --git a/mm/memory.c b/mm/memory.c
> index f306e698a1e3..9b8348746e0b 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1320,11 +1320,18 @@ struct zap_details {
>  static inline bool
>  zap_skip_check_mapping(struct zap_details *details, struct page *page)
>  {
> -       if (!details || !page)
> +       /* No detail pointer or no zap_mapping pointer, zap all */
> +       if (!details || !details->zap_mapping)
>                 return false;
>  
> -       return details->zap_mapping &&
> -               (details->zap_mapping != page_rmapping(page));
> +       /*
> +        * For example, the zero page.  If the user wants to keep the private
> +        * pages, zero page should also be in count.
> +        */
> +       if (!page)
> +               return true;
> +
> +       return details->zap_mapping != page_rmapping(page);
>  }
>  
>  static unsigned long zap_pte_range(struct mmu_gather *tlb,
> ---8<---
> 
> page can be NULL for e.g. PFNMAP and when error occured too above.  I assume we
> don't need to bother with them (e.g., I don't think PFNMAP or MIXED should
> specify even_cows=false at all, because there's no page cache layer), though.
> Mostly it's about how we should handle zero page right.

I have not understood the above.

I don't know of any problem that needs fixing with the zero page:
how do you suppose the zero page gets into a truncatable or hole-punchable
mapping?  We use it for read faults in anonymous mappings.  And I told the
story of how once-upon-a-time it could get inserted into any mapping by
reading from /dev/zero, but that odd case was dropped years ago.  And I
am open to (even encouraging) a change to make use of zero page for read
faults of holes in shmem: but that's potential future work, which would
require some changes elsewhere (though perhaps none here: the zero page
could never be used for the result of a COW).

Please explain the zero page problem you hope to fix here.

Hugh