Re: [PATCH v3 2/3] mm/hugetlb: disable PMD sharing for UFFD missing mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/hugetlb: uffd_disable_huge_pmd_share() leaks poison markers on unregister
--

commit 5da87cbb6b8f2271fb75acc59e14e2d5860cb54c
Author: David Hildenbrand (Arm) <david@kernel.org>

mm/hugetlb: disable PMD sharing for UFFD missing mode

This commit disables hugetlb pmd page table sharing for all userfaultfd
registered vmas by checking vma->vm_userfaultfd_ctx.ctx in
uffd_disable_huge_pmd_share(), preventing poison markers installed under
missing mode from affecting shared pmd tables. It also adds a debug check
in hugetlb_mfill_atomic_pte() when installing a poison marker.

> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> index a14b8a9ffb7b1..ebd86677b052d 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -168,22 +168,12 @@ static inline bool is_mergeable_vm_userfaultfd_ctx(struct vm_area_struct *vma,
>  }
>  
>  /*
> - * Never enable huge pmd sharing on some uffd registered vmas:
> - *
> - * - VM_UFFD_WP and VM_UFFD_RWP VMAs, because the write protect / access
> - *   tracking information is per pgtable entry.
> - *
> - * - VM_UFFD_MINOR VMAs, because otherwise we would never get minor faults for
> - *   VMAs which share huge pmds. (If you have two mappings to the same
> - *   underlying pages, and fault in the non-UFFD-registered one with a write,
> - *   with huge pmd sharing this would *also* setup the second UFFD-registered
> - *   mapping, and we'd not get minor faults.)
> + * Never enable huge pmd sharing on userfaultfd registered VMAs, because
> + * userfaultfd state can be tracked per VMA or per page table entry.
>   */
>  static inline bool uffd_disable_huge_pmd_share(struct vm_area_struct *vma)
>  {
> -	return vma_test_any_mask(vma,
> -		mk_vma_flags_from_masks(VMA_UFFD_WP, VMA_UFFD_RWP,
> -					VMA_UFFD_MINOR));
> +	return vma->vm_userfaultfd_ctx.ctx;
>  }
>  
>  /*
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 966d249469454..7566f2eed50b0 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6399,6 +6399,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
>  			return -EEXIST;
>  		}
>  
> +		VM_WARN_ON_ONCE(size == PMD_SIZE && hugetlb_pmd_shared(dst_pte));
>  		_dst_pte = make_pte_marker(PTE_MARKER_POISONED);
>  		set_huge_pte_at(dst_mm, dst_addr, dst_pte, _dst_pte, size);
>  

[Severity: High]
Can a pmd table containing a poison marker still become shared across
processes after the vma is unregistered from userfaultfd?

When a process registers a shared hugetlbfs vma in missing mode and installs
a poison marker via hugetlb_mfill_atomic_pte(), the marker is written into the
process's private pmd table.

If the process later unregisters the vma or closes the userfaultfd descriptor
(via userfaultfd_unregister() -> userfaultfd_clear_vma() or
userfaultfd_release_all() -> userfaultfd_clear_vma(), or remaps via
mremap_userfaultfd_prep()), the userfaultfd flags and context are cleared
while the poison marker remains in the page table:

mm/userfaultfd.c:userfaultfd_clear_vma() {
	...
	ret = vma_modify_flags_uffd(vmi, prev, vma, start, end,
				    &new_vma_flags, NULL_VM_UFFD_CTX,
				    give_up_on_oom);

	if (!IS_ERR(ret))
		userfaultfd_reset_ctx(ret);
	...
}

Unlike write-protect markers, which are resolved by change_protection() on
unregistration, poison markers intentionally persist after the userfaultfd
context is reset.

Once vma->vm_userfaultfd_ctx.ctx is cleared and the userfaultfd flags are
removed from vma->vm_flags, uffd_disable_huge_pmd_share() returns false and
page_table_shareable() in mm/hugetlb.c considers the vma eligible for sharing
again:

mm/hugetlb.c:huge_pmd_share() {
	...
	mapping_rmap_tree_foreach(svma, mapping, idx, idx) {
		if (svma == vma)
			continue;

		saddr = page_table_shareable(svma, vma, addr, idx);
		if (saddr) {
			spte = hugetlb_walk(svma, saddr,
					    vma_mmu_pagesize(svma));
			if (spte) {
				ptdesc_pmd_pts_inc(virt_to_ptdesc(spte));
				break;
			}
		}
	}
	...
}

When another process mapping the same shared hugetlbfs file faults in that
range (hugetlb_fault() -> huge_pte_alloc() -> want_pmd_share() ->
huge_pmd_share()), huge_pmd_share() walks the first process's vma, finds its
populated pmd table containing the poison marker, and shares it into the
second process.

That exposes the second process to the first process's poison marker. In
addition, if the first process subsequently unshares the pmd table (for
example via mprotect(), hugetlb_split(), or re-registering userfaultfd),
__huge_pmd_unshare() clears the pud entry in the first process, causing the
originating process to lose its own poison marker while leaving it installed
in the second process.

Should the vma or ptdesc record that a poison marker was installed so pmd
sharing remains disabled for it even after userfaultfd unregistration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022034543.51226-1-lizhe.67@bytedance.com?part=2