Re: [PATCH 11/12] mm/madvise: send UFFD_EVENT_REMOVE when installing guard regions

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

On Wed, Nov 11, 2026 at 12:55:19PM -0500, Tal Zussman wrote:
> MADV_GUARD_INSTALL zaps whatever is mapped in the range before
> installing the guard markers. However, unlike MADV_DONTNEED and
> MADV_REMOVE, madvise_guard_install() does not call userfaultfd_remove(),
> so the userfaultfd manager is not told that the pages are gone.
>
> On an anonymous VMA, the zap drops uffd-wp and RWP protection along with
> the pages. Writes and accesses to the range after the guard is removed
> are then not reported, and the manager cannot know that it has to
> protect the range again.
>
> To fix it, send UFFD_EVENT_REMOVE before zapping. Guard install on a VMA
> registered with userfaultfd already takes the mmap lock instead of the
> VMA lock, so we can drop the lock for the event and look the VMA up
> again afterwards.
>
> As with MADV_DONTNEED, the call can now return -ENOMEM if the VMA goes
> away while the lock is dropped, or -EINVAL if it is replaced by a VMA
> that does not support guard regions.
>
> Fixes: 662df3e5c376 ("mm: madvise: implement lightweight guard page mechanism")
> Closes: https://lore.kernel.org/linux-mm/3f4b0d1b.bbf84.1a1c55cb666.Coremail.18670333770@163.com/

This report was super-synthetic AI slop

> Signed-off-by: Tal Zussman <tz2294@columbia.edu>

Sorry I hate this.

Since this was already silently broken, I think the right solution is to simply
disallow guard region installation for uffd-armed VMAs.

I had the AI go look for users, and it seems nobody is relying on this (glibc,
android, CRIU, etc.), and there's really no sensible world in which somebody's
combining the two. So I think fine to do that.

It was simply an oversight (reflex action is to disable new features for hugetlb
+ uffd)

I'll send a patch separate from this, please drop it from the series.

> ---
>  mm/madvise.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index e8b05dc899f1..6413f0e65381 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1166,6 +1166,29 @@ static long madvise_guard_install(struct madvise_behavior *madv_behavior)
>  	if (!is_valid_guard_vma(vma, /* allow_locked = */false))
>  		return -EINVAL;
>
> +	/*
> +	 * Installing guards zaps the range, so report it to a userfaultfd
> +	 * manager like MADV_DONTNEED does. The VMA can change while the mmap
> +	 * lock is dropped for the event, so look it up again.
> +	 */
> +	if (!userfaultfd_remove(vma, range->start, range->end)) {
> +		mark_mmap_lock_dropped(madv_behavior);
> +		mmap_read_lock(madv_behavior->mm);
> +		madv_behavior->vma = vma = vma_lookup(madv_behavior->mm,
> +						      range->start);
> +		if (!vma)
> +			return -ENOMEM;
> +		if (!is_valid_guard_vma(vma, /* allow_locked = */false))
> +			return -EINVAL;
> +		/*
> +		 * The VMA may have been split while the lock was dropped.
> +		 * Handle the part in this VMA. The remainder is handled by the
> +		 * next iteration of madvise_walk_vmas().
> +		 */
> +		if (range->end > vma->vm_end)
> +			range->end = vma->vm_end;
> +	}
> +

(separate rant - that we have to a. know to do it and b. write duplicative
open-coded crap like this is a perfect example of why uffd is such an utterly
broken mm feature that should never have been merged...)

>  	/*
>  	 * Set atomically under read lock. All pertinent readers will need to
>  	 * acquire an mmap/VMA write lock to read it. All remaining readers may
>
> --
> 2.39.5
>

--
Cheers, Lorenzo