Re: [PATCH 12/12] mm/userfaultfd: check hugetlb end alignment in UFFDIO_UNREGISTER pre-scan

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

hugetlb vs userfaultfd is a bit like the alien vs. predator of worst mm features
of all time (TM)

On Wed, Nov 11, 2026 at 12:55:20PM -0500, Tal Zussman wrote:
> UFFDIO_REGISTER and UFFDIO_UNREGISTER both scan the VMAs of the range
> before changing anything. For a hugetlb VMA, userfaultfd_register()
> rejects a range whose start or end is not huge page aligned there.
> However, userfaultfd_unregister() only checks the start.
>
> An unaligned end only fails later, when vma_modify_flags_uffd() cannot
> split the VMA there. At that point, userfaultfd_clear_vma() already
> cleared the WP or RWP protection. The ioctl returns -EINVAL and the VMA
> stays registered, but accesses to its pages are no longer reported. This
> requires a caller passing a misaligned range.
>
> To fix it, check the end of a registered VMA in the pre-scan as well.
> The ioctl still returns -EINVAL, but no longer unregisters the VMAs
> before the misaligned one first.
>
> Fixes: cab350afcbc9 ("userfaultfd: hugetlbfs: allow registration of ranges containing huge pages")
> Fixes: f369b07c8614 ("mm/uffd: reset write protection when unregister with wp-mode")
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
>  mm/userfaultfd.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index bf42fabb5afd..c0076c8b4e9d 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -3983,6 +3983,18 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
>  		if (!vma_can_userfault(cur, cur->vm_flags, wp_async))
>  			goto out_unlock;
>
> +		/*
> +		 * A range ending inside a registered hugetlb vma must end
> +		 * huge page aligned, as in userfaultfd_register().
> +		 */

Hmm I think this is more or less saying what the code is doing :) so I think
unnecessary. Maybe worth using the words 'cursed' and 'eldrich' here to describe
what's going on? Or possibly just e.g.:

	/* See check in userfaultfd_register(). */

> +		if (cur->vm_userfaultfd_ctx.ctx && vma_is_hugetlb(cur) &&

This first check is pretty horrid to spell out loud, maybe do something like:

	do {
		const struct userfaultfd_ctx *vma_ctx = cur->vm_userfaultfd_ctx.ctx;
		...
		if (vma_ctx && vma_is_hugetlb(cur) && end <= cur->vm_end &&
		    end > cur->vm_start) {
		    ...
		}

> +		    end <= cur->vm_end && end > cur->vm_start) {
> +			unsigned long vma_hpagesize = vma_kernel_pagesize(cur);

Could be const?

Also is vma_kernel_pagesize() right? If the VMA is hugetlb, then that'll end up
as hugetlb_vm_op_pagesize() which is huge_page_size(hstate_vma()), but it seems
userfaultfd is full of this crap and yeah ok. Fine :)

> +
> +			if (end & (vma_hpagesize - 1))

Could do:
			if (!IS_ALIGNED(end, vma_hpagesize))

Instead?


> +				goto out_unlock;
> +		}
> +
>  		found = true;
>  	} for_each_vma_range(vmi, cur, end);
>  	VM_WARN_ON_ONCE(!found);
>
> --
> 2.39.5
>

--
Cheers, Lorenzo