linux-kernel - Re: [PATCH v8 06/23] mm/shmem: Handle uffd-wp special pte in page fault handler

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] [day] [month] [year] [list]

Message-ID: <Yn03HkVrzSzYkpn8@xz-m1.local>
Date:   Thu, 12 May 2022 12:34:38 -0400
From:   Peter Xu <peterx@...hat.com>
To:     David Hildenbrand <david@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Nadav Amit <nadav.amit@...il.com>,
        Matthew Wilcox <willy@...radead.org>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        Hugh Dickins <hughd@...gle.com>,
        Jerome Glisse <jglisse@...hat.com>,
        "Kirill A . Shutemov" <kirill@...temov.name>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        Alistair Popple <apopple@...dia.com>
Subject: Re: [PATCH v8 06/23] mm/shmem: Handle uffd-wp special pte in page
 fault handler

On Wed, May 11, 2022 at 06:30:59PM +0200, David Hildenbrand wrote:
> > +/*
> > + * This is actually a page-missing access, but with uffd-wp special pte
> > + * installed.  It means this pte was wr-protected before being unmapped.
> > + */
> > +static vm_fault_t pte_marker_handle_uffd_wp(struct vm_fault *vmf)
> > +{
> > +	/*
> > +	 * Just in case there're leftover special ptes even after the region
> > +	 * got unregistered - we can simply clear them.  We can also do that
> > +	 * proactively when e.g. when we do UFFDIO_UNREGISTER upon some uffd-wp
> > +	 * ranges, but it should be more efficient to be done lazily here.
> > +	 */
> > +	if (unlikely(!userfaultfd_wp(vmf->vma) || vma_is_anonymous(vmf->vma)))
> > +		return pte_marker_clear(vmf);
> 
> What would happen if we do a unregister followed by a register? IMHO we
> should start with a clean uffd-wp slate then. Your comment makes ma
> assume that we could receive stale WP events, which would be wrong?

I'd say it's not wrong, but it's true and actually expected.

Firstly, userfaultfd (by design) always allows false positives (getting
same message multiple times) but no tolerance on false negatives (missing
event, which is data corrupt).

The latter should be obvious.  For the former, the simplest example is when
two threads access the same missing page the same time, two same messages
will be generated.  Same applies to wr-protect faults.  And it'll be
non-trivial (or say, impossible.. IMHO) to avoid those.

In this specific case, it's about when to drop the uffd-wp bits when
unregister.  Two obvious options: (1) during unregister, or (2) lazy.

Here I chose the lazy way because unregister could be slowed down by this,
and that's when program quits.  In short with current approach we quit
fast.  We could have leftovers, but we'll take care of them when needed.

One important thing is leftover ptes should not be the major way uffd-wp
should be used by the normal register -> wr-protect -> unprotect ->
unregister sequence.  Normally the process won't unregister probably until
it quits, so the leftover does no harm to anyone.

Meanwhile, any user who wants to avoid the lazy way can simply do a
whole-round unprotect before unregister.  So we leave more choice for the
user and by default we make sure no syscall will be easily slowed down.

Hope that answers, thanks!

-- 
Peter Xu