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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm/madvise: missing can_madvise_modify() recheck in
  madvise_guard_install()
--

commit 8be4fedce36f0b45069f1f630f0d9241d579512f
Author: Tal Zussman <tz2294@columbia.edu>

mm/madvise: send UFFD_EVENT_REMOVE when installing guard regions

This commit updates madvise_guard_install() to call userfaultfd_remove()
before zapping the target range and re-look up the VMA if the mmap lock
was dropped.

> diff --git a/mm/madvise.c b/mm/madvise.c
> index e8b05dc899f10..6413f0e65381b 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;

[Severity: Medium]
Should this also recheck can_madvise_modify(madv_behavior) after looking
up the VMA again?

madvise_vma_behavior() checks can_madvise_modify() before calling
madvise_guard_install().  While mm->mmap_lock is dropped inside
userfaultfd_remove(), another thread can make the VMA read-only via
mprotect(PROT_READ) and seal it with mseal().  Because
is_valid_guard_vma() only checks VMA_LOCKED_BIT, vma_is_mm_backed() and
vma_is_hugetlb(), madvise_guard_install() will then proceed to zap pages
and install guard markers on the sealed read-only VMA instead of
returning -EPERM.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-uffd-state-fixes-v1-0-00713d368976@columbia.edu?part=11