Re: [PATCH v7 09/15] userfaultfd: add UFFDIO_REGISTER_MODE_RWP and UFFDIO_RWPROTECT plumbing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] userfaultfd_clear_vma() modifies page tables before write-locking the VMA, allowing concurrent page faults to leak stray UFFD markers.
- [High] UFFDIO_MOVE completely drops userfaultfd_wp protection markers on present PTEs, silently breaking write protection tracking.
--

commit 325a6bfce816d9aabb37133a65ed3422bc565fb2
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>

userfaultfd: add UFFDIO_REGISTER_MODE_RWP and UFFDIO_RWPROTECT plumbing

This commit adds the userspace interface and internal plumbing for
read-write protection tracking (RWP). It introduces UFFDIO_REGISTER_MODE_RWP
and UFFDIO_RWPROTECT, which allow tracking access to memory ranges.

> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
[ ... ]
> @@ -2204,9 +2282,22 @@ static struct vm_area_struct *userfaultfd_clear_vma(struct vma_iterator *vmi,
>  	if (start == vma->vm_start && end == vma->vm_end)
>  		give_up_on_oom = true;
>  
> -	/* Reset ptes for the whole vma range if wr-protected */
> -	if (userfaultfd_wp(vma))
> -		uffd_wp_range(vma, start, end - start, false);
> +	/* Clear the uffd bit and/or restore protnone PTEs */
> +	if (userfaultfd_protected(vma)) {
> +		unsigned int mm_cp_flags = 0;
> +		struct mmu_gather tlb;
> +
> +		if (userfaultfd_wp(vma))
> +			mm_cp_flags |= MM_CP_UFFD_WP_RESOLVE;
> +		if (userfaultfd_rwp(vma))
> +			mm_cp_flags |= MM_CP_UFFD_RWP_RESOLVE;
> +		if (vma_wants_manual_pte_write_upgrade(vma))
> +			mm_cp_flags |= MM_CP_TRY_CHANGE_WRITABLE;
> +
> +		tlb_gather_mmu(&tlb, vma->vm_mm);
> +		change_protection(&tlb, vma, start, end, mm_cp_flags);
> +		tlb_finish_mmu(&tlb);
> +	}
>  
>  	ret = vma_modify_flags_uffd(vmi, prev, vma, start, end,

[Severity: High]
This isn't a bug introduced by this patch, but userfaultfd_clear_vma() in
mm/userfaultfd.c appears to modify page tables before write-locking the VMA.

By calling change_protection() before vma_modify_flags_uffd() triggers
vma_start_write() to drain concurrent per-VMA readers, could a concurrent page
fault thread that already acquired the per-VMA lock see the old VM_UFFD_WP or
VM_UFFD_RWP flags?

If so, could it instantiate a new PTE with the _PAGE_UFFD marker after
change_protection() has already scanned that address, leaking a stray marker?

[ ... ]
> @@ -1358,7 +1358,7 @@ static long move_present_ptes(struct mm_struct *mm,
>  		orig_dst_pte = folio_mk_pte(src_folio, dst_vma->vm_page_prot);
>  		/* Set soft dirty bit so userspace can notice the pte was moved */
>  		if (pgtable_supports_soft_dirty())
>  			orig_dst_pte = pte_mksoft_dirty(orig_dst_pte);
>  		if (pte_dirty(orig_src_pte))
>  			orig_dst_pte = pte_mkdirty(orig_dst_pte);
>  		orig_dst_pte = pte_mkwrite(orig_dst_pte, dst_vma);

[Severity: High]
This is a pre-existing issue, but does UFFDIO_MOVE drop userfaultfd_wp
protection markers on present PTEs here in move_present_ptes() in
mm/userfaultfd.c?

When constructing the destination PTE with folio_mk_pte() and pte_mkwrite(),
it doesn't seem to check if the source PTE was write-protected
(e.g., via pte_uffd()), nor does it transfer the _PAGE_UFFD marker to the
destination PTE.

Could this silently break write protection tracking on pages moved via
UFFDIO_MOVE?

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