Re: [PATCH v7 07/15] mm: preserve RWP marker across PTE rewrites

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] do_swap_page bypasses RWP tracking on non-exclusive private pages by resolving COW faults internally, dropping the RWP marker.
- [High] move_huge_pmd() and move_huge_pte() unconditionally overwrite the protection bits of ALL present huge entries in RWP-enabled VMAs, omitting marker validation.
- [High] RWP markers (PAGE_NONE PTEs) are incorrectly made writable by subsequent pte_mkwrite/pmd_mkwrite calls, violating PTE invariants.
--

commit 5e6bf53c7874736a0aaea6decc0c390ca2b40f26
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>

mm: preserve RWP marker across PTE rewrites

This commit wires the preservation of the uffd RWP marker (PAGE_NONE
semantics) through various paths that rewrite PTEs, PMDs, and huge PTEs
on a VM_UFFD_RWP VMA. This includes swap, migration, fork, and mprotect
paths to prevent tracking leaks on next access.

> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
[ ... ]
> @@ -2539,8 +2545,16 @@ 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))
> +		if (vma_has_uffd_without_event_remap(vma)) {
> +			/*
> +			 * See __copy_present_ptes(): normalise RWP PMDs so
> +			 * the destination starts accessible instead of taking
> +			 * a numa-hinting fault on first access.
> +			 */
> +			if (pmd_present(pmd) && userfaultfd_rwp(vma))
> +				pmd = pmd_modify(pmd, vma->vm_page_prot);

[Severity: High]
Does this unconditionally overwrite the protection bits of all present PMDs
in RWP-enabled VMAs? By omitting a pmd_uffd(pmd) check, this could strip
legitimate write permissions from private VMAs or inject write permissions
into clean shared pages when mremap() is called on a VM_UFFD_RWP VMA with
mixed RWP-protected and accessible huge pages.

>  			pmd = clear_uffd_wp_pmd(pmd);
> +		}
>  		set_pmd_at(mm, new_addr, new_pmd, pmd);

[ ... ]
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
[ ... ]
> @@ -5060,10 +5075,22 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,
>  		huge_pte_clear(mm, new_addr, dst_pte, sz);
>  	} else {
>  		if (need_clear_uffd_wp) {
> -			if (pte_present(pte))
> +			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 = huge_pte_modify(pte, vma->vm_page_prot);

[Severity: High]
Is a huge_pte_uffd(pte) check missing here as well? Without it, any mremap()
on an RWP-registered VMA could incorrectly modify all present huge PTEs
regardless of their actual marker status.

> +					pte = arch_make_huge_pte(pte, huge_page_shift(h),
> +								 vma->vm_flags);
> +				}
>  				pte = huge_pte_clear_uffd(pte);

[ ... ]
> diff --git a/mm/memory.c b/mm/memory.c
> --- a/mm/memory.c
> +++ b/mm/memory.c
[ ... ]
> @@ -896,6 +896,10 @@ static void restore_exclusive_pte(struct vm_area_struct *vma,
>  	if (pte_swp_uffd(orig_pte))
>  		pte = pte_mkuffd(pte);
>  
> +	/* See do_swap_page(): restore PAGE_NONE for RWP */
> +	if (pte_swp_uffd(orig_pte) && userfaultfd_rwp(vma))
> +		pte = pte_modify(pte, PAGE_NONE);
> +
>  	if ((vma->vm_flags & VM_WRITE) &&
>  	    can_change_pte_writable(vma, address, pte)) {
>  		if (folio_test_dirty(folio))

[Severity: High]
Could this create an invalid clean+writable PTE? If the folio is clean,
pte_mkdirty() is skipped but the subsequent code calls pte_mkwrite() on
the PAGE_NONE PTE. Writable PTEs must be dirty to avoid hardware faults on
architectures without hardware A/D bit management.

[ ... ]
> @@ -5041,6 +5068,14 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  	if (pte_swp_uffd(vmf->orig_pte))
>  		pte = pte_mkuffd(pte);
>  
> +	/*
> +	 * A page reclaimed while RWP-protected carries the uffd bit on
> +	 * its swap entry. Re-apply PAGE_NONE on swap-in so the first access
> +	 * still traps as an RWP fault. pte_modify() preserves _PAGE_UFFD.
> +	 */
> +	if (pte_swp_uffd(vmf->orig_pte) && userfaultfd_rwp(vma))
> +		pte = pte_modify(pte, PAGE_NONE);
> +
>  	/*
>  	 * Same logic as in do_wp_page(); however, optimize for pages that are
>  	 * certainly not shared either because we just allocated them without

[Severity: High]
Similar to restore_exclusive_pte() above, does this lead to a clean+writable
PAGE_NONE PTE during a read fault? In do_swap_page(), if it is a read fault,
pte_mkwrite() is called but pte_mkdirty() is skipped, which violates the
clean+writable PTE invariant.

[Severity: Critical]
What happens later in do_swap_page() for non-exclusive private pages on a
write fault?

Since the RWP marker is restored as PAGE_NONE, do_swap_page() will eventually
call do_wp_page() to resolve the COW fault. However, wp_page_copy() appears
to allocate a new page and map it with full accessible permissions, missing
the PAGE_NONE protection.

Does this completely bypass the userfaultfd RWP boundary for shared or
forked swapped-out pages?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629120749.566063-1-kirill@shutemov.name?part=7