linux-kernel - Re: [PATCH v6 04/23] mm/uffd: PTE_MARKER_UFFD

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:   Thu, 16 Dec 2021 13:45:28 +0800
From:   Peter Xu <peterx@...hat.com>
To:     Alistair Popple <apopple@...dia.com>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        Nadav Amit <nadav.amit@...il.com>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        Hugh Dickins <hughd@...gle.com>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        "Kirill A . Shutemov" <kirill@...temov.name>,
        Jerome Glisse <jglisse@...hat.com>,
        Matthew Wilcox <willy@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Hildenbrand <david@...hat.com>,
        Andrea Arcangeli <aarcange@...hat.com>
Subject: Re: [PATCH v6 04/23] mm/uffd: PTE_MARKER_UFFD_WP

On Thu, Dec 16, 2021 at 04:18:50PM +1100, Alistair Popple wrote:
> On Monday, 15 November 2021 6:55:03 PM AEDT Peter Xu wrote:
> 
> [...]
> 
> > +/*
> > + * Returns true if this is a swap pte and was uffd-wp wr-protected in either
> > + * forms (pte marker or a normal swap pte), false otherwise.
> > + */
> > +static inline bool pte_swp_uffd_wp_any(pte_t pte)
> > +{
> > +#ifdef CONFIG_PTE_MARKER_UFFD_WP
> > +	if (!is_swap_pte(pte))
> > +		return false;
> > +
> > +	if (pte_swp_uffd_wp(pte))
> > +		return true;
> 
> If I'm not mistaken normal swap uffd-wp ptes can still exist when
> CONFIG_PTE_MARKER_UFFD_WP=n so shouldn't this be outside the #ifdef protection?
> 
> In fact we could drop the #ifdef entirely here as it is safe to call
> is_pte_marker_uffd_wp() without CONFIG_PTE_MARKER_UFFD_WP.

But if CONFIG_PTE_MARKER_UFFD_WP=n then it means we don't support file-backed
uffd-wp.  The thing is pte_swp_uffd_wp_any() is only needed for file-backed..
Anonymous uffd-wp code never uses it.

In other words, pte_swp_uffd_wp() is indeed still a valid helper, however this
specific function (pte_swp_uffd_wp_any) may not really be necessary anymore.

Keeping the "#ifdef" could still help, imho, to generate clean code for direct
calls to pte_swp_uffd_wp_any() if someone wants to turn pte markers off,
e.g. we could still have chance to optimize below:

        if (pte_swp_uffd_wp_any(pte) &&
                !(zap_flags & ZAP_FLAG_DROP_MARKER))
                set_huge_pte_at(mm, address, ptep,
                                make_pte_marker(PTE_MARKER_UFFD_WP));
        else
                huge_pte_clear(mm, address, ptep, sz);

Into:

        huge_pte_clear(mm, address, ptep, sz);

with any decent compiler.

That's majorly why I still slightly prefer to keep it, and that's also one of
the major reason to have the config knob, imho.

Thanks,

-- 
Peter Xu