[PATCH] mm/userfaultfd: only allocate anon_vma in CoW mappings

[PATCH] mm/userfaultfd: only allocate anon_vma in CoW mappings

[David Hildenbrand (Arm)]

find_vma_and_prepare_anon() can end up allocating an anon_vma in VMAs that
are not CoW mappings, which is wrong.

If we have a MAP_SHARED R/O file mapped, we lack VM_SHARED (but do have
VM_MAYSHARE) and lack VM_MAYWRITE.

While uffd later rejects such mappings (uffd registration requires
VM_MAYWRITE, see comment in validate_dst_vma) find_vma_and_prepare_anon()
still wrongly allocates an anon vma before it validates the vma.

This in turn later confuses copy_page_range()->vma_needs_copy() such
that we end up calling copy_hugetlb_page_range() with ranges that
actually do not contain *any* per-pagetable information that would
require copying, which is the basis for PMD-table sharing in the first
place.

copy_hugetlb_page_range() should not have to worry about shared
PMD tables at all: it doesn't make any sense to copy page tables that are
shared, as they can be fully recreated later. Ending up in the function
with shareable page tables is the real issue.

If a destination PMD table is shared in copy_hugetlb_page_range(), it
might get concurrently unshared as we are not holding relevant
locks, resulting in us using a page table that is no longer mapped and
possibly freed after calling:

	dst_pte = huge_pte_alloc(dst, dst_vma, addr, sz);

To fix it, do not allocate an anon_vma in non-CoW mappings and add a
safety guard to copy_hugetlb_page_range(), making sure that we never
end up messing with shared PMD tables in there.

Before commit 867a43a34ff8a ("userfaultfd: use per-vma locks in userfaultfd
operations") we would always validate the VMA before allocating an
anon_vma, and consequently never wrongly allocate an anon_vma.

As uffd_lock_vma() is part of the same lookup machinery and
also contains a misleading VM_SHARED check, change that one as well
directly for consistency.

A follow-up cleanup will stop taking the hugetlb_vma_lock_read(src_vma)
lock in copy_hugetlb_page_range() entirely. Further, the usage of
VM_SHARED in userfaultfd code should be cleaned up later to focus on
CoW vs. non-CoW mappings.

Reported-by: Kyle Zeng <kylebot@openai.com>
Fixes: 867a43a34ff8a ("userfaultfd: use per-vma locks in userfaultfd operations")
Cc: stable@vger.kernel.org
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 mm/hugetlb.c     | 24 ++++++++++++------------
 mm/userfaultfd.c |  5 ++---
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 63dc836061c99..60f9b209c8db1 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4984,10 +4984,11 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
 		raw_write_seqcount_begin(&src->write_protect_seq);
 	} else {
 		/*
-		 * For shared mappings the vma lock must be held before
-		 * calling hugetlb_walk() in the src vma. Otherwise, the
-		 * returned ptep could go away if part of a shared pmd and
-		 * another thread calls huge_pmd_unshare.
+		 * Shared mappings only need copying when we could have
+		 * information stored in page tables (e.g., userfaultfd PTE
+		 * bits or markers), which is incompatible with PMD sharing.
+		 * Still take the hugetlb VMA lock for now to satisfy
+		 * hugetlb_walk()'s locking requirements.
 		 */
 		hugetlb_vma_lock_read(src_vma);
 	}
@@ -4995,6 +4996,13 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
 	last_addr_mask = hugetlb_mask_last_page(h);
 	for (addr = src_vma->vm_start; addr < src_vma->vm_end; addr += sz) {
 		spinlock_t *src_ptl, *dst_ptl;
+
+		if (WARN_ON_ONCE(want_pmd_share(src_vma, addr) ||
+				 want_pmd_share(dst_vma, addr))) {
+			ret = -EINVAL;
+			break;
+		}
+
 		src_pte = hugetlb_walk(src_vma, addr, sz);
 		if (!src_pte) {
 			addr |= last_addr_mask;
@@ -5006,14 +5014,6 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
 			break;
 		}
 
-#ifdef CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING
-		/* If the pagetables are shared, there is nothing to do */
-		if (ptdesc_pmd_is_shared(virt_to_ptdesc(dst_pte))) {
-			addr |= last_addr_mask;
-			continue;
-		}
-#endif
-
 		dst_ptl = huge_pte_lock(h, dst, dst_pte);
 		src_ptl = huge_pte_lockptr(h, src, src_pte);
 		spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 0e0fe9bfef239..82b56a58fc40d 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -115,8 +115,7 @@ struct vm_area_struct *find_vma_and_prepare_anon(struct mm_struct *mm,
 	vma = vma_lookup(mm, addr);
 	if (!vma)
 		vma = ERR_PTR(-ENOENT);
-	else if (!(vma->vm_flags & VM_SHARED) &&
-		 unlikely(anon_vma_prepare(vma)))
+	else if (vma_is_cow_mapping(vma) && unlikely(anon_vma_prepare(vma)))
 		vma = ERR_PTR(-ENOMEM);
 
 	return vma;
@@ -145,7 +144,7 @@ static struct vm_area_struct *uffd_lock_vma(struct mm_struct *mm,
 		 * We know we're going to need an anon rmap, so check that
 		 * early.
 		 */
-		if (!(vma->vm_flags & VM_SHARED) && unlikely(!vma_has_anon_rmap(vma)))
+		if (vma_is_cow_mapping(vma) && unlikely(!vma_has_anon_rmap(vma)))
 			vma_end_read(vma);
 		else
 			return vma;

---

base-commit: 1d0b04a60516de48250b04227dcf483c6c09aab1

change-id: 20261014-copy_hugetlb_page_range-743d71a2980e

--

Cheers,

David

Re: [PATCH] mm/userfaultfd: only allocate anon_vma in CoW mappings

[Lorenzo Stoakes (ARM)]

On Wed, Oct 14, 2026 at 01:53:11PM +0200, David Hildenbrand (Arm) wrote:
> As uffd_lock_vma() is part of the same lookup machinery and
> also contains a misleading VM_SHARED check, change that one as well
> directly for consistency.

OK, I think that one isn't _strictly necessary_, but while we're there,
worth doing too, clearly.

We should also go through and convert the remaining !VM_SHARED checks ->
vma_is_cow_mapping() too if possible, but that's a follow up, obviously!

>
> A follow-up cleanup will stop taking the hugetlb_vma_lock_read(src_vma)
> lock in copy_hugetlb_page_range() entirely. Further, the usage of

Hmm really? Is it possible to do that without the hugetlb lock?

It'd be good in general if we could get rid of that lock as a whole :) but maybe
dreaming.

> VM_SHARED in userfaultfd code should be cleaned up later to focus on
> CoW vs. non-CoW mappings.

I think also anywhere else in the kernel where this is done. An audit on that in
general would be useful!

>
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Fixes: 867a43a34ff8a ("userfaultfd: use per-vma locks in userfaultfd operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>

This LGTM, thanks for handling this!

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

--
Cheers, Lorenzo

Re: [PATCH] mm/userfaultfd: only allocate anon_vma in CoW mappings

[Kyle Zeng]

On Wed, Oct 14, 2026 at 03:51:22PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Wed, Oct 14, 2026 at 01:53:11PM +0200, David Hildenbrand (Arm) wrote:
> > As uffd_lock_vma() is part of the same lookup machinery and
> > also contains a misleading VM_SHARED check, change that one as well
> > directly for consistency.
> 
> OK, I think that one isn't _strictly necessary_, but while we're there,
> worth doing too, clearly.
> 
> We should also go through and convert the remaining !VM_SHARED checks ->
> vma_is_cow_mapping() too if possible, but that's a follow up, obviously!
> 
> >
> > A follow-up cleanup will stop taking the hugetlb_vma_lock_read(src_vma)
> > lock in copy_hugetlb_page_range() entirely. Further, the usage of
> 
> Hmm really? Is it possible to do that without the hugetlb lock?
> 
> It'd be good in general if we could get rid of that lock as a whole :) but maybe
> dreaming.
> 
> > VM_SHARED in userfaultfd code should be cleaned up later to focus on
> > CoW vs. non-CoW mappings.
> 
> I think also anywhere else in the kernel where this is done. An audit on that in
> general would be useful!
> 
> >
> > Reported-by: Kyle Zeng <kylebot@openai.com>
> > Fixes: 867a43a34ff8a ("userfaultfd: use per-vma locks in userfaultfd operations")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> 
> This LGTM, thanks for handling this!
> 
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> 
> --
> Cheers, Lorenzo

I ran the patch through my system and did not find any issues with this
patch. Thanks for addressing this!

Reviewed-by: Kyle Zeng <kylebot@openai.com>

Re: [PATCH] mm/userfaultfd: only allocate anon_vma in CoW mappings

[David Hildenbrand (Arm)]

On 10/14/26 20:04, Kyle Zeng wrote:
> On Wed, Oct 14, 2026 at 03:51:22PM +0100, Lorenzo Stoakes (ARM) wrote:
>> On Wed, Oct 14, 2026 at 01:53:11PM +0200, David Hildenbrand (Arm) wrote:
>>> As uffd_lock_vma() is part of the same lookup machinery and
>>> also contains a misleading VM_SHARED check, change that one as well
>>> directly for consistency.
>>
>> OK, I think that one isn't _strictly necessary_, but while we're there,
>> worth doing too, clearly.
>>
>> We should also go through and convert the remaining !VM_SHARED checks ->
>> vma_is_cow_mapping() too if possible, but that's a follow up, obviously!
>>
>>>
>>> A follow-up cleanup will stop taking the hugetlb_vma_lock_read(src_vma)
>>> lock in copy_hugetlb_page_range() entirely. Further, the usage of
>>
>> Hmm really? Is it possible to do that without the hugetlb lock?
>>
>> It'd be good in general if we could get rid of that lock as a whole :) but maybe
>> dreaming.
>>
>>> VM_SHARED in userfaultfd code should be cleaned up later to focus on
>>> CoW vs. non-CoW mappings.
>>
>> I think also anywhere else in the kernel where this is done. An audit on that in
>> general would be useful!
>>
>>>
>>> Reported-by: Kyle Zeng <kylebot@openai.com>
>>> Fixes: 867a43a34ff8a ("userfaultfd: use per-vma locks in userfaultfd operations")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
>>
>> This LGTM, thanks for handling this!
>>
>> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>>
>> --
>> Cheers, Lorenzo
> 
> I ran the patch through my system and did not find any issues with this
> patch. Thanks for addressing this!
> 
> Reviewed-by: Kyle Zeng <kylebot@openai.com>

Thanks a bunch, and thanks for reporting all these issues!

-- 
Cheers,

David

Re: [PATCH] mm/userfaultfd: only allocate anon_vma in CoW mappings

[David Hildenbrand (Arm)]

On 10/14/26 16:51, Lorenzo Stoakes (ARM) wrote:
> On Wed, Oct 14, 2026 at 01:53:11PM +0200, David Hildenbrand (Arm) wrote:
>> As uffd_lock_vma() is part of the same lookup machinery and
>> also contains a misleading VM_SHARED check, change that one as well
>> directly for consistency.
> 
> OK, I think that one isn't _strictly necessary_, but while we're there,
> worth doing too, clearly.
> 
> We should also go through and convert the remaining !VM_SHARED checks ->
> vma_is_cow_mapping() too if possible, but that's a follow up, obviously!
> 
>>
>> A follow-up cleanup will stop taking the hugetlb_vma_lock_read(src_vma)
>> lock in copy_hugetlb_page_range() entirely. Further, the usage of
> 
> Hmm really? Is it possible to do that without the hugetlb lock?
> 

Yes, we don't have to sync against PMD sharing if PMD sharing is impossible.
This is what I have laying around:

commit ad6a5c65bd5fa31d93a2de2d2e5e473b26ad6a3d
Author: David Hildenbrand (Arm) <david@kernel.org>
Date:   Sat Oct 10 13:00:13 2026 +0200

    mm/hugetlb: stop syncing against PMD table sharing in copy_hugetlb_page_range()
    
    We must never end up in copy_hugetlb_page_range() when PMD sharing
    applies, yet we still have to perform the hugetlb_vma_lock_read() so
    hugetlb_walk() is happy.
    
    Let's introduce a new vma_supports_pmd_sharing() and use that instead
    in hugetlb_walk(). Also convert want_pmd_share(), which now looks a lot
    cleaner, and document why the uffd checks cannot go into
    __vma_shareable_lock().
    
    We can now relocate our safety checks in copy_hugetlb_page_range() to
    the beginning of the function.
    
    No functional change intended.
    
    Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 5029c72418631..b2da5d5816944 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -1389,6 +1389,20 @@ static inline bool __vma_shareable_lock(struct vm_area_struct *vma)
 
 bool __vma_private_lock(struct vm_area_struct *vma);
 
+static inline bool vma_supports_pmd_sharing(struct vm_area_struct *vma)
+{
+#ifdef CONFIG_USERFAULTFD
+	/*
+	 * Keep the UFFD checks out of __vma_shareable_lock(): UFFD flags can
+	 * change at runtime while the hugetlb VMA lock is held. They must not
+	 * cause unlocking or freeing an existing lock to be skipped.
+	 */
+	if (uffd_disable_huge_pmd_share(vma))
+		return false;
+#endif
+	return __vma_shareable_lock(vma);
+}
+
 /*
  * Safe version of huge_pte_offset() to check the locks.  See comments
  * above huge_pte_offset().
@@ -1406,7 +1420,7 @@ hugetlb_walk(struct vm_area_struct *vma, unsigned long addr, unsigned long sz)
 	 *
 	 * NOTE: lockdep_is_held() is only defined with CONFIG_LOCKDEP.
 	 */
-	if (__vma_shareable_lock(vma))
+	if (vma_supports_pmd_sharing(vma))
 		WARN_ON_ONCE(!lockdep_is_held(&vma_lock->rw_sema) &&
 			     !lockdep_is_held(
 				 &vma->vm_file->f_mapping->i_mmap_rwsem));
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 60f9b209c8db1..606e93e32eefb 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4987,22 +4987,16 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
 		 * Shared mappings only need copying when we could have
 		 * information stored in page tables (e.g., userfaultfd PTE
 		 * bits or markers), which is incompatible with PMD sharing.
-		 * Still take the hugetlb VMA lock for now to satisfy
-		 * hugetlb_walk()'s locking requirements.
 		 */
-		hugetlb_vma_lock_read(src_vma);
+		if (WARN_ON_ONCE(vma_supports_pmd_sharing(src_vma) ||
+				 vma_supports_pmd_sharing(dst_vma)))
+			return -EINVAL;
 	}
 
 	last_addr_mask = hugetlb_mask_last_page(h);
 	for (addr = src_vma->vm_start; addr < src_vma->vm_end; addr += sz) {
 		spinlock_t *src_ptl, *dst_ptl;
 
-		if (WARN_ON_ONCE(want_pmd_share(src_vma, addr) ||
-				 want_pmd_share(dst_vma, addr))) {
-			ret = -EINVAL;
-			break;
-		}
-
 		src_pte = hugetlb_walk(src_vma, addr, sz);
 		if (!src_pte) {
 			addr |= last_addr_mask;
@@ -5147,8 +5141,6 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
 	if (cow) {
 		raw_write_seqcount_end(&src->write_protect_seq);
 		mmu_notifier_invalidate_range_end(&range);
-	} else {
-		hugetlb_vma_unlock_read(src_vma);
 	}
 
 	return ret;
@@ -7005,17 +6997,9 @@ bool want_pmd_share(struct vm_area_struct *vma, unsigned long addr)
 	unsigned long start = addr & PUD_MASK;
 	unsigned long end = start + PUD_SIZE;
 
-#ifdef CONFIG_USERFAULTFD
-	if (uffd_disable_huge_pmd_share(vma))
-		return false;
-#endif
-	/*
-	 * check on proper vm_flags and page table alignment
-	 */
-	if (!(vma->vm_flags & VM_MAYSHARE))
-		return false;
-	if (!vma->vm_private_data)	/* vma lock required for sharing */
+	if (!vma_supports_pmd_sharing(vma))
 		return false;
+
 	if (!range_in_vma(vma, start, end))
 		return false;
 	return true;


Will send that out for the next cycle.

> It'd be good in general if we could get rid of that lock as a whole :) but maybe
> dreaming.

Yeah :)

Thanks!

-- 
Cheers,

David