Re: [PATCH v9 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:

Pre-existing issues:
- [Critical] userfaultfd_clear_vma() accesses page tables under mmap_write_lock without calling vma_start_write(), creating a race condition with per-VMA lock readers.
- [High] Page table protections are irreversibly removed before VMA modification in userfaultfd_clear_vma(), leading to silent protection bypass if the VMA split fails.
- [High] UFFDIO_MOVE silently drops userfaultfd-wp state when moving present PTEs, allowing write-protection bypass.
--

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

userfaultfd: add UFFDIO_REGISTER_MODE_RWP and UFFDIO_RWPROTECT plumbing

This patch adds the userspace interface for read-write protection tracking
via userfaultfd. It introduces the UFFDIO_REGISTER_MODE_RWP register mode
and the UFFDIO_RWPROTECT ioctl.

> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -2204,9 +2273,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 this code race with per-VMA lock
readers?

When userfaultfd_unregister() iterates over VMAs under mmap_write_lock and
calls userfaultfd_clear_vma(), it accesses and modifies page tables via
change_protection() without first isolating the VMA using vma_start_write().

Because vma_start_write() is missing, the mmap_write_lock holder does not
wait for concurrent per-VMA lock readers, like MADV_DONTNEED, to finish.

This can result in a use-after-free if the reader frees the PTE page while
change_protection() is active.

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

[Severity: High]
This isn't a bug introduced by this patch, but could this sequence lead to
silent protection bypass if the VMA split fails?

Because change_protection() unconditionally strips the userfaultfd markers
from the PTEs before calling vma_modify_flags_uffd(), if the modification
fails, for example by returning -ENOMEM from hitting the max_map_count limit,
the ioctl will abort with an error.

The PTEs are left fully accessible while the VMA still retains its tracking
flags, silently breaking the memory tracking.

[ ... ]

> @@ ... @@ move_present_ptes(...)
> {
> [ ... ]
> 	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 silently drop the
userfaultfd-wp state here?

When moving a present PTE, the destination PTE is made writable via
pte_mkwrite() without checking if the source PTE was uffd_wp protected.

Because it does not apply pte_mkuffd() for WP ranges, the destination
page becomes fully writable and loses its tracking marker.

This allows subsequent writes to bypass the userfaultfd WP trap.

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