Re: [PATCH 07/12] mm/mremap: resolve uffd state when the context is dropped, not during the move

[Tal Zussman <tz2294@columbia.edu>]

On 11/12/26 6:44 AM, Lorenzo Stoakes (ARM) wrote:
> On Wed, Nov 11, 2026 at 12:55:15PM -0500, Tal Zussman wrote:
>> Without UFFD_FEATURE_EVENT_REMAP, the new VMA of an mremap() is not
>> registered with userfaultfd: mremap_userfaultfd_prep() drops the context
>> from it once the move succeeded. The uffd-wp bits, uffd-wp markers, and
>> RWP PAGE_NONE entries in the new VMA must be cleared as well.
>>
>> However, move_ptes(), move_huge_pmd(), and move_huge_pte() clear them
>> from each entry while moving it, before we know whether the move will
>> succeed. If a page table allocation fails halfway through, mremap()
>> moves the entries back into the old VMA, which is still registered,
>> without their uffd state.
>>
>> Writes to a WP range are then no longer reported, and an RWP range
>> becomes accessible without a fault. Nothing restores the protection
>> afterwards. This requires a page table allocation to fail in the middle
>> of mremap(), so it is rare.
>>
>> The uffd state must only be cleared once the move can no longer fail. To
>> fix it, leave the entries alone during the move and clear the uffd state
>> of the new VMA in mremap_userfaultfd_prep(), right before dropping the
>> context, using userfaultfd_clear_ptes() like unregistering does. A
>> failed move then puts the entries back as they were.
>>
>> This adds a second walk over the page tables of the new VMA, for VMAs
>> registered for WP or RWP without UFFD_FEATURE_EVENT_REMAP.
>>
>> Like unregistering, userfaultfd_clear_ptes() also asks for the PTEs to
>> be made writable again where that is safe (MM_CP_TRY_CHANGE_WRITABLE).
>> The old code only cleared the uffd bit and left the PTE read-only. For
>> private anonymous memory this saves the minor fault on the first write
>> to each page that was protected.
>>
>> However, this adds spurious minor faults on shmem and hugetlb. When
>> resolving, change_protection() rewrites every present entry in the range
>> with vma->vm_page_prot, so a page that was writable and not protected is
>> made read-only, and its first write after mremap() faults.
>> UFFDIO_UNREGISTER and UFFDIO_WRITEPROTECT already behave this way and
>> lead to the same spurious faults. This needs to be fixed in
>> change_protection() and will be done in a separate change.
>>
>> The entries no longer need per-entry handling during the move, so drop
>> uffd_supports_page_table_move(). Where the old and new addresses are
>> suitably aligned, move_normal_pmd() and move_normal_pud() can move whole
>> page tables of these VMAs again.
>>
>> clear_uffd_wp_pmd() has no users left, so remove it.
> 
> This reads a little schloppy, I wonder if it coudl be cut down a bit?
> 

I'll trim it a little. I was trying to be precise about all the side
effects, and the spurious fault issue popped up along the way and it got
kind of long...

>>
>> Fixes: 0cef0bb836e3 ("mm: clear uffd-wp PTE/PMD state on mremap()")
>> Fixes: 763f43865fc4 ("mm: preserve RWP marker across PTE rewrites")
>> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> 
> This LGTM and is secretly a cleanup which I love :) So:
> 
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> 

Thanks!

>> ---
>>  mm/huge_memory.c | 25 -------------------------
>>  mm/hugetlb.c     | 27 +--------------------------
>>  mm/mremap.c      | 46 +---------------------------------------------
>>  mm/userfaultfd.c |  1 +
>>  4 files changed, 3 insertions(+), 96 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 09176b290f76..468a49041b95 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2616,18 +2616,6 @@ static pmd_t move_soft_dirty_pmd(pmd_t pmd)
>>  	return pmd;
>>  }
>>
>> -static pmd_t clear_uffd_wp_pmd(pmd_t pmd)
>> -{
>> -	if (pmd_none(pmd))
>> -		return pmd;
>> -	if (pmd_present(pmd))
>> -		pmd = pmd_clear_uffd(pmd);
>> -	else
>> -		pmd = pmd_swp_clear_uffd(pmd);
>> -
>> -	return pmd;
>> -}
>> -
>>  bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,
>>  		  unsigned long new_addr, pmd_t *old_pmd, pmd_t *new_pmd)
>>  {
>> @@ -2663,19 +2651,6 @@ bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,
>>  			pgtable_trans_huge_deposit(mm, new_pmd, pgtable);
>>  		}
>>  		pmd = move_soft_dirty_pmd(pmd);
>> -		if (vma_has_uffd_without_event_remap(vma)) {
>> -			/*
>> -			 * See __copy_present_ptes(): normalise the RWP marker
>> -			 * so the destination starts accessible instead of
>> -			 * taking a numa-hinting fault on first access. Only the
>> -			 * marker (protnone + uffd) needs it; leave other present
>> -			 * PMDs in the VMA untouched.
>> -			 */
>> -			if (pmd_present(pmd) && userfaultfd_rwp(vma) &&
>> -			    pmd_uffd(pmd))
>> -				pmd = pmd_modify(pmd, vma->vm_page_prot);
>> -			pmd = clear_uffd_wp_pmd(pmd);
>> -		}
>>  		set_pmd_at(mm, new_addr, new_pmd, pmd);
>>  		if (force_flush)
>>  			flush_pmd_tlb_range(vma, old_addr, old_addr + PMD_SIZE);
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index cb2de24affdc..97bd29925476 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -5168,7 +5168,6 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,
>>  			  unsigned long new_addr, pte_t *src_pte, pte_t *dst_pte,
>>  			  unsigned long sz)
>>  {
>> -	bool need_clear_uffd_wp = vma_has_uffd_without_event_remap(vma);
>>  	struct hstate *h = hstate_vma(vma);
>>  	struct mm_struct *mm = vma->vm_mm;
>>  	spinlock_t *src_ptl, *dst_ptl;
>> @@ -5179,31 +5178,7 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,
>>  	double_pt_lock(dst_ptl, src_ptl);
>>
>>  	pte = huge_ptep_get_and_clear(mm, old_addr, src_pte, sz);
>> -
>> -	if (need_clear_uffd_wp && pte_is_uffd_wp_marker(pte)) {
>> -		huge_pte_clear(mm, new_addr, dst_pte, sz);
>> -	} else {
>> -		if (need_clear_uffd_wp) {
>> -			if (pte_present(pte)) {
>> -				/*
>> -				 * See __copy_present_ptes(): normalise the RWP
>> -				 * marker so the destination starts accessible
>> -				 * instead of taking a numa-hinting fault on
>> -				 * first access. Only the marker (protnone + uffd)
>> -				 * needs it; leave other present PTEs untouched.
>> -				 */
>> -				if (userfaultfd_rwp(vma) && huge_pte_uffd(pte)) {
>> -					pte = huge_pte_modify(pte, vma->vm_page_prot);
>> -					pte = arch_make_huge_pte(pte, huge_page_shift(h),
>> -								 vma->vm_flags);
>> -				}
>> -				pte = huge_pte_clear_uffd(pte);
>> -			} else {
>> -				pte = pte_swp_clear_uffd(pte);
>> -			}
>> -		}
>> -		set_huge_pte_at(mm, new_addr, dst_pte, pte, sz);
>> -	}
>> +	set_huge_pte_at(mm, new_addr, dst_pte, pte, sz);
>>
>>  	double_pt_unlock(dst_ptl, src_ptl);
>>  }
>> diff --git a/mm/mremap.c b/mm/mremap.c
>> index fff0465f5e06..4564416a6994 100644
>> --- a/mm/mremap.c
>> +++ b/mm/mremap.c
>> @@ -198,7 +198,6 @@ static int move_ptes(struct pagetable_move_control *pmc,
>>  		unsigned long extent, pmd_t *old_pmd, pmd_t *new_pmd)
>>  {
>>  	struct vm_area_struct *vma = pmc->old;
>> -	bool need_clear_uffd_wp = vma_has_uffd_without_event_remap(vma);
>>  	struct mm_struct *mm = vma->vm_mm;
>>  	pte_t *old_ptep, *new_ptep;
>>  	pte_t old_pte, pte;
>> @@ -290,27 +289,7 @@ static int move_ptes(struct pagetable_move_control *pmc,
>>  		pte = get_and_clear_ptes(mm, old_addr, old_ptep, nr_ptes);
>>  		pte = move_pte(pte, old_addr, new_addr);
>>  		pte = move_soft_dirty_pte(pte);
>> -
>> -		if (need_clear_uffd_wp && pte_is_uffd_wp_marker(pte))
>> -			pte_clear(mm, new_addr, new_ptep);
>> -		else {
>> -			if (need_clear_uffd_wp) {
>> -				if (pte_present(pte)) {
>> -					/*
>> -					 * See __copy_present_ptes(): normalise
>> -					 * RWP PTEs so the destination starts
>> -					 * accessible instead of taking a
>> -					 * numa-hinting fault on first access.
>> -					 */
>> -					if (userfaultfd_rwp(vma) && pte_uffd(pte))
>> -						pte = pte_modify(pte, vma->vm_page_prot);
>> -					pte = pte_clear_uffd(pte);
>> -				} else {
>> -					pte = pte_swp_clear_uffd(pte);
>> -				}
>> -			}
>> -			set_ptes(mm, new_addr, new_ptep, pte, nr_ptes);
>> -		}
>> +		set_ptes(mm, new_addr, new_ptep, pte, nr_ptes);
>>  	}
>>
>>  	lazy_mmu_mode_disable();
>> @@ -334,25 +313,6 @@ static inline bool arch_supports_page_table_move(void)
>>  }
>>  #endif
>>
>> -static inline bool uffd_supports_page_table_move(struct pagetable_move_control *pmc)
>> -{
>> -	/*
>> -	 * If we are moving a VMA that has uffd-wp registered but with
>> -	 * remap events disabled (new VMA will not be registered with uffd), we
>> -	 * need to ensure that the uffd-wp state is cleared from all pgtables.
>> -	 * This means recursing into lower page tables in move_page_tables().
>> -	 *
>> -	 * We might get called with VMAs reversed when recovering from a
>> -	 * failed page table move. In that case, the
>> -	 * "old"-but-actually-"originally new" VMA during recovery will not have
>> -	 * a uffd context. Recursing into lower page tables during the original
>> -	 * move but not during the recovery move will cause trouble, because we
>> -	 * run into already-existing page tables. So check both VMAs.
>> -	 */
>> -	return !vma_has_uffd_without_event_remap(pmc->old) &&
>> -	       !vma_has_uffd_without_event_remap(pmc->new);
>> -}
>> -
>>  #ifdef CONFIG_HAVE_MOVE_PMD
>>  static bool move_normal_pmd(struct pagetable_move_control *pmc,
>>  			pmd_t *old_pmd, pmd_t *new_pmd)
>> @@ -365,8 +325,6 @@ static bool move_normal_pmd(struct pagetable_move_control *pmc,
>>
>>  	if (!arch_supports_page_table_move())
>>  		return false;
>> -	if (!uffd_supports_page_table_move(pmc))
>> -		return false;
>>  	/*
>>  	 * The destination pmd shouldn't be established, free_pgtables()
>>  	 * should have released it.
>> @@ -434,8 +392,6 @@ static bool move_normal_pud(struct pagetable_move_control *pmc,
>>
>>  	if (!arch_supports_page_table_move())
>>  		return false;
>> -	if (!uffd_supports_page_table_move(pmc))
>> -		return false;
>>  	/*
>>  	 * The destination pud shouldn't be established, free_pgtables()
>>  	 * should have released it.
>> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
>> index ad9d243dc64b..bf42fabb5afd 100644
>> --- a/mm/userfaultfd.c
>> +++ b/mm/userfaultfd.c
>> @@ -3161,6 +3161,7 @@ void mremap_userfaultfd_prep(struct vm_area_struct *vma,
>>  		up_write(&ctx->map_changing_lock);
>>  	} else {
>>  		/* Drop uffd context if remap feature not enabled */
>> +		userfaultfd_clear_ptes(vma, vma->vm_start, vma->vm_end);
>>  		userfaultfd_reset_ctx(vma);
>>  	}
>>  }
>>
>> --
>> 2.39.5
>>
> 
> --
> Cheers, Lorenzo
>