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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] COW (Copy-On-Write) bypass in fork() due to PTE permission upgrade on RWP markers.
- [Critical] COW (Copy-On-Write) bypass in mremap() due to unconditional PTE/PMD permission upgrade.
- [High] restore_exclusive_pte() incorrectly applies write permissions to a PAGE_NONE RWP marker.
- [High] Kernel Samepage Merging (KSM) silently disarms RWP markers when replacing pages.
--

commit 774184e482d558cfd9ea45101fb7b151ec350b6b
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>

mm: preserve RWP marker across PTE rewrites

This commit wires the preservation of the userfaultfd RWP marker across various
kernel paths that rewrite PTEs. It ensures the uffd PTE bit and PAGE_NONE
semantics survive operations like swap, device-exclusive restore, migration,
fork, and mprotect.

> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index b352b0dfe1ba2..ddf8d2e49c127 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c

[ ... ]

> @@ -2539,8 +2545,19 @@ 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 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);

[Severity: Critical]
Does this code inadvertently bypass Copy-On-Write protections?

In move_huge_pmd(), when mremap() is called on a VM_UFFD_RWP VMA without
EVENT_REMAP, the destination VMA drops the uffd context. By normalizing the
marker via pmd_modify(pmd, vma->vm_page_prot), any pre-existing COW read-only
restrictions (such as from a prior fork or KSM) seem to be overwritten by the
VMA's default writable permissions. Can this result in a shared page being
mapped writable?

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

[ ... ]

> diff --git a/mm/memory.c b/mm/memory.c
> index 6b0304caf1a4a..c1f79a3dfd6a0 100644
> --- 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))
>  			pte = pte_mkdirty(pte);
>  		pte = pte_mkwrite(pte, vma);
>  	}

[Severity: High]
Does this block create an invalid clean+writable PTE and bypass RWP tracking?

In restore_exclusive_pte(), the RWP marker gets its PAGE_NONE protection
restored. However, if the VMA has VM_WRITE, the code proceeds to
unconditionally apply pte_mkwrite() to the PTE. Can this hardware write bit
added to a PAGE_NONE PTE cause hardware writes to succeed without trapping,
permanently bypassing the VMM's access tracking?

[ ... ]

> @@ -1100,9 +1108,31 @@ static __always_inline void __copy_present_ptes(struct vm_area_struct *dst_vma,
>  		pte_t pte, unsigned long addr, int nr)
>  {
>  	struct mm_struct *src_mm = src_vma->vm_mm;
> +	bool writable;
> +
> +	/*
> +	 * Snapshot writability before the RWP-disarm rewrite below: when the
> +	 * child is not RWP-armed, pte_modify(pte, dst_vma->vm_page_prot) can
> +	 * silently drop _PAGE_RW from a resolved (no-marker) writable PTE,
> +	 * so a later pte_write(pte) check would skip the COW wrprotect and
> +	 * leave the parent writable over a folio shared with the child.
> +	 */
> +	writable = pte_write(pte);
> +
> +	/*
> +	 * Child is not RWP-armed: restore accessible protection so the
> +	 * inherited PAGE_NONE does not cost a fault on first read. Gate on
> +	 * pte_uffd(pte) so unrelated PAGE_NONE markers (e.g. NUMA balancing)
> +	 * are not normalised away.
> +	 */
> +	if (!userfaultfd_protected(dst_vma)) {
> +		if (userfaultfd_rwp(src_vma) && pte_uffd(pte))
> +			pte = pte_modify(pte, dst_vma->vm_page_prot);

[Severity: Critical]
Does this normalization branch inadvertently upgrade a COW-shared page to
writable?

In __copy_present_ptes(), the "writable" variable is evaluated before this
block. For an RWP marker (which is PAGE_NONE), pte_write() evaluates to false.
If the child VMA drops VM_UFFD_RWP and the RWP normalization branch executes,
the PTE may be made writable based on dst_vma->vm_page_prot.

Because "writable" was originally false, the subsequent COW write-protect
block (is_cow_mapping() && writable) is skipped. Does this result in a fully
writable PTE being installed for a shared Copy-On-Write page?

> +		pte = pte_clear_uffd(pte);
> +	}
>  
>  	/* If it's a COW mapping, write protect it both processes. */
> -	if (is_cow_mapping(src_vma->vm_flags) && pte_write(pte)) {
> +	if (is_cow_mapping(src_vma->vm_flags) && writable) {
>  		wrprotect_ptes(src_mm, addr, src_pte, nr);

[ ... ]

> diff --git a/mm/mremap.c b/mm/mremap.c
> index 12732a5c547e9..8a46ec5831c84 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c

[ ... ]

> @@ -296,10 +296,19 @@ static int move_ptes(struct pagetable_move_control *pmc,
>  			pte_clear(mm, new_addr, new_ptep);
>  		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_uffd(pte))
> +						pte = pte_modify(pte, vma->vm_page_prot);

[Severity: Critical]
Could this unconditionally upgrade PTE permissions and destroy Copy-On-Write
protection during mremap()?

Similar to move_huge_pmd(), when move_ptes() encounters an RWP marker on a VMA
dropping its uffd context, it overwrites the PTE protection with the VMA's
default via pte_modify(). If the page was previously shared and COW-protected,
its read-only status appears to be lost.

>  					pte = pte_clear_uffd(pte);
> -				else
> +				} else {
>  					pte = pte_swp_clear_uffd(pte);
> +				}

[ ... ]

[Severity: High]
Does Kernel Samepage Merging (KSM) silently disarm RWP markers when replacing
pages?

Looking at replace_page() in mm/ksm.c:

	newpte = mk_pte(kpage, vma->vm_page_prot);

When KSM merges an anonymous page in a VM_UFFD_RWP VMA, it constructs the new
PTE using the VMA's default protection. This appears to entirely drop the
PAGE_NONE marker and the _PAGE_UFFD bit required for RWP tracking. Will reads
to the page bypass the RWP trap entirely because it is now mapped accessible?

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