linux-kernel - Re: [PATCH 4/4] mm: Apply vm_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]

Message-ID: <aFxoSzdDGRkYLyr2@x1.local>
Date: Wed, 25 Jun 2025 17:21:15 -0400
From: Peter Xu <peterx@...hat.com>
To: James Houghton <jthoughton@...gle.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	Nikita Kalyazin <kalyazin@...zon.com>,
	Hugh Dickins <hughd@...gle.com>, Oscar Salvador <osalvador@...e.de>,
	Michal Hocko <mhocko@...e.com>,
	David Hildenbrand <david@...hat.com>,
	Muchun Song <muchun.song@...ux.dev>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Ujwal Kundur <ujwal.kundur@...il.com>,
	Suren Baghdasaryan <surenb@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Vlastimil Babka <vbabka@...e.cz>,
	"Liam R . Howlett" <Liam.Howlett@...cle.com>,
	Mike Rapoport <rppt@...nel.org>,
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
	Axel Rasmussen <axelrasmussen@...gle.com>
Subject: Re: [PATCH 4/4] mm: Apply vm_uffd_ops API to core mm

On Wed, Jun 25, 2025 at 01:31:49PM -0700, James Houghton wrote:
> > -static inline bool vma_can_userfault(struct vm_area_struct *vma,
> > -                                    unsigned long vm_flags,
> > -                                    bool wp_async)
> > +static inline const vm_uffd_ops *vma_get_uffd_ops(struct vm_area_struct *vma)
> >  {
> > -       vm_flags &= __VM_UFFD_FLAGS;
> > -
> > -       if (vma->vm_flags & VM_DROPPABLE)
> > -               return false;
> > -
> > -       if ((vm_flags & VM_UFFD_MINOR) &&
> > -           (!is_vm_hugetlb_page(vma) && !vma_is_shmem(vma)))
> > -               return false;
> > -
> > -       /*
> > -        * If wp async enabled, and WP is the only mode enabled, allow any
> > -        * memory type.
> > -        */
> > -       if (wp_async && (vm_flags == VM_UFFD_WP))
> > -               return true;
> > -
> > -#ifndef CONFIG_PTE_MARKER_UFFD_WP
> > -       /*
> > -        * If user requested uffd-wp but not enabled pte markers for
> > -        * uffd-wp, then shmem & hugetlbfs are not supported but only
> > -        * anonymous.
> > -        */
> > -       if ((vm_flags & VM_UFFD_WP) && !vma_is_anonymous(vma))
> > -               return false;
> > -#endif
> 
> Hi Peter,
> 
> Thanks for this cleanup!
> 
> It looks like the above two checks, the wp-async one and the PTE
> marker check, have been reordered in this patch. Does this result in a
> functional difference?
> 
> The rest of this series looks fine to me. :)

Thanks for the very careful review, James!

Yes that's a small tweak I did when moving. I don't expect to have any
functional change.  Maybe I should at least mention that in the commit log.

Here I did the movement because fundamentally wp_async depends on the pte
markers, so it may be slightly more intuitive to check pte markers first,
rejecting any form of file wr-protect traps.  Otherwise it may looks like
we could return the true for wp_async==true too early.  In reality IIUC it
can't happen.

For example, currently userfaultfd_api() has:

#ifndef CONFIG_PTE_MARKER_UFFD_WP
	uffdio_api.features &= ~UFFD_FEATURE_WP_HUGETLBFS_SHMEM;
	uffdio_api.features &= ~UFFD_FEATURE_WP_UNPOPULATED;
	uffdio_api.features &= ~UFFD_FEATURE_WP_ASYNC;
#endif

So when wp_async can be true above, pte markers must be compiled..  IOW,
above code clip should work identically with below lines:

    #ifdef CONFIG_PTE_MARKER_UFFD_WP
       if (wp_async && (vm_flags == VM_UFFD_WP))
            return true;
    #endif
    
    #ifndef CONFIG_PTE_MARKER_UFFD_WP
        if ((vm_flags & VM_UFFD_WP) && !vma_is_anonymous(vma))
            return false;
    #endif

Then it means both chunks of code cannot be compiled together.  The order
shouldn't matter.

But maybe I should just move it back as before, to save the explain and
confusions.  Let me know if you have any preference.

Thanks,

-- 
Peter Xu