Re: [PATCH v8 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 3 potential issue(s) to consider:

New issues:
- [High] Unrestricted mprotect() on RWP-registered VMAs allows userspace to bypass accessibility constraints, leading to UFFDIO_UNREGISTER denial of service and FOLL_FORCE infinite loops.

Pre-existing issues:
- [Critical] userfaultfd_clear_vma() modifies page tables under mmap_write_lock without first calling vma_start_write(), allowing races with concurrent per-VMA read lock operations.
- [High] Incomplete rollback on OOM during partial userfaultfd_unregister leaves VMA and PTE state inconsistent.
--

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

userfaultfd: add UFFDIO_REGISTER_MODE_RWP and UFFDIO_RWPROTECT plumbing

Add the userspace interface for read-write protection tracking,
including UFFDIO_REGISTER_MODE_RWP, UFFD_FEATURE_RWP, and
UFFDIO_RWPROTECT. Introduces CONFIG_USERFAULTFD_RWP and integrates
RWP handling into userfaultfd core.

> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c

[ ... ]

> @@ -2152,6 +2221,15 @@ static bool vma_can_userfault(struct vm_area_struct *vma, vm_flags_t vm_flags,
>  	    !vma_is_anonymous(vma))
>  		return false;
>  
> +	/*
> +	 * RWP uses protnone as an access-tracking marker. PROT_NONE VMAs
> +	 * have vm_page_prot == PAGE_NONE, so RWP resolution can't make a
> +	 * page accessible -- the next access would fault again. Reject up
> +	 * front instead of letting FOLL_FORCE loop on protnone+uffd PTEs.
> +	 */
> +	if ((vm_flags & VM_UFFD_RWP) && !vma_is_accessible(vma))
> +		return false;
> +

[Severity: High]
Could this accessibility constraint be bypassed by mprotect() after the
VMA is already registered for RWP?

vma_can_userfault() correctly prevents initial RWP registration on
PROT_NONE VMAs. However, mprotect_fixup() preserves VM_UFFD_RWP when
changing a VMA to PROT_NONE later.

If userspace registers a readable VMA for RWP and then calls
mprotect(PROT_NONE), the VMA enters the exact state this check intended
to prevent.

Would a subsequent FOLL_FORCE access (e.g. via /proc/self/mem) skip RWP
resolution in handle_pte_fault(), fail to resolve the PROT_NONE state,
and retry infinitely? 

If FOLL_WRITE is used, could it trigger do_wp_page() to copy and allocate
new PROT_NONE pages infinitely?

Also, wouldn't this prevent the VMA from being unregistered, since
userfaultfd_unregister() calls vma_can_userfault() which would now reject
the inaccessible VMA?

[ ... ]

> @@ -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);

[Severity: Critical]
This is a pre-existing issue, but does userfaultfd_clear_vma() modify
page tables before draining per-VMA readers?

The kernel VMA locking guidelines require that mmap_write_lock() holders
call vma_start_write(vma) before modifying page tables. This excludes
per-VMA lock readers.

Here, change_protection() modifies PTEs before vma_modify_flags_uffd()
is called. Could this race with concurrent per-VMA operations like
MADV_DONTNEED that might modify or free the page tables while
change_protection() is walking them?

> +		tlb_finish_mmu(&tlb);
> +	}
>  
>  	ret = vma_modify_flags_uffd(vmi, prev, vma, start, end,
>  				    &new_vma_flags, NULL_VM_UFFD_CTX,
>  				    give_up_on_oom);

[Severity: High]
This is a pre-existing issue, but what happens if the subsequent VMA split
fails due to OOM?

change_protection() unconditionally resolves the UFFD protection on all
PTEs in the range before vma_modify_flags_uffd() executes.

If vma_modify_flags_uffd() returns -ENOMEM during a partial unregister, it
aborts and the error is returned to userspace.

Does this leave the VMA retaining the VM_UFFD_WP or VM_UFFD_RWP flag,
while the underlying PTEs have been permanently unprotected?

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