Re: [PATCH 09/12] mm/madvise: let a guard marker carry the uffd-wp bit

[Lorenzo Stoakes (ARM)]

On Wed, Nov 11, 2026 at 12:55:17PM -0500, Tal Zussman wrote:
> MADV_GUARD_INSTALL can only install guard markers in a range without
> populated entries. It zaps the range and retries, restarting the syscall
> if the range still isn't clear. On a file-backed VMA registered for
> uffd-wp, the zap keeps uffd-wp markers and converts write-protected PTEs
> into new markers, as the pages are still in the page cache and must stay
> write-protected. The range never becomes clear, and installing a guard
> region in write-protected shmem keeps restarting the madvise() call.
> Signal handlers still run, but the call does not return until the task
> is killed.
>
> An entry that is write-protected must stay write-protected while a guard
> is installed on it and after the guard is removed. PTE markers are a bit
> mask, so a marker can be a guard marker and a uffd-wp marker at the same
> time. To fix it, let a guard marker carry the uffd-wp bit:
>
>  - guard install adds the guard bit to an existing uffd-wp marker
>  - guard removal clears only the guard bit
>  - unprotecting or unregistering clears the uffd-wp bit of a guard
>    marker
>
> The fault path, zap, fork, and pagemap already handle a marker with both
> bits set. However, pte_is_uffd_marker() would let UFFDIO_COPY overwrite
> the guard, so make it ignore guard markers. Anonymous VMAs drop uffd-wp
> markers on zap and never get the combined marker.
>
> MADV_GUARD_INSTALL on a uffd-wp shmem range now succeeds instead of
> hanging. When the guard region is removed again, its pages are still
> write-protected. While the guard is in place, pagemap reports both
> PM_GUARD_REGION and PM_UFFD_WP for them.
>
> There are two simpler alternatives, but neither works. Zapping with
> ZAP_FLAG_DROP_MARKER fixes the hang but drops the write protection: the
> page is still in the page cache once the guard is removed, and writes to
> it are not reported. Refusing guard install on uffd-wp file VMAs in
> is_valid_guard_vma() would break a combination that works on anonymous
> memory, by adding a new -EINVAL for it.
>
> Fixes: f807123d578d ("mm: allow guard regions in file-backed and read-only mappings")
> Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>

Please drop this patch too.

With my patch disallowing guard marker install on uffd-armed VMAs this goes
away (and doesn't make sense anyway.)

I really feel these guard region ones should have been sent separately, btw.

You're establishing a bunch of interlocking dependencies of different changes in
this series that makes life hard for reviewers, especially given backports etc.

And now I'm having to essentially inverse cherry pick stuff :)

> ---
>  include/linux/leafops.h |  4 ++++
>  mm/madvise.c            | 28 ++++++++++++++++++++++++++--
>  mm/mprotect.c           | 23 +++++++++++++++++++++--
>  3 files changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/leafops.h b/include/linux/leafops.h
> index 4c1476ae3234..add4968ded44 100644
> --- a/include/linux/leafops.h
> +++ b/include/linux/leafops.h
> @@ -578,6 +578,10 @@ static inline bool pte_is_uffd_marker(pte_t pte)
>  	if (!softleaf_is_marker(entry))
>  		return false;
>
> +	/* A guard marker stays a guard, even with the uffd-wp bit set. */
> +	if (softleaf_is_guard_marker(entry))
> +		return false;
> +
>  	/* UFFD WP, poisoned swap entries are UFFD-handled. */
>  	if (softleaf_is_uffd_wp_marker(entry))
>  		return true;
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 0214efd2a014..e2c6b0f26672 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1123,6 +1123,23 @@ static int guard_install_pte_entry(pte_t *pte, unsigned long addr,
>  		return 0;
>  	}
>
> +	/*
> +	 * Zapping keeps uffd-wp markers on file-backed VMAs, so add the guard
> +	 * to the marker instead. The slot stays write-protected once the guard
> +	 * is removed.
> +	 */
> +	if (!vma_is_anonymous(walk->vma) && pte_is_uffd_wp_marker(pteval)) {
> +		const softleaf_t entry = softleaf_from_pte(pteval);
> +
> +		set_pte_at(walk->mm, addr, pte,
> +			   make_pte_marker(softleaf_to_marker(entry) |
> +					   PTE_MARKER_GUARD));
> +		update_mmu_cache(walk->vma, addr, pte);
> +		(*nr_pages)++;
> +
> +		return 0;
> +	}
> +
>  	/* If populated return >0 so we abort the operation + zap. */
>  	return 1;
>  }
> @@ -1259,8 +1276,15 @@ static int guard_remove_pte_entry(pte_t *pte, unsigned long addr,
>  	pte_t ptent = ptep_get(pte);
>
>  	if (is_guard_pte_marker(ptent)) {
> -		/* Simply clear the PTE marker. */
> -		pte_clear(walk->mm, addr, pte);
> +		const softleaf_t entry = softleaf_from_pte(ptent);
> +		const pte_marker marker = softleaf_to_marker(entry) &
> +					  ~PTE_MARKER_GUARD;
> +
> +		/* Clear the guard bit and keep the uffd-wp bit, if set. */
> +		if (marker)
> +			set_pte_at(walk->mm, addr, pte, make_pte_marker(marker));
> +		else
> +			pte_clear(walk->mm, addr, pte);
>  		update_mmu_cache(walk->vma, addr, pte);
>  	}
>
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index ef4e7188e24d..6cc5b99b4185 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -249,9 +249,28 @@ static long change_softleaf_pte(struct vm_area_struct *vma,
>  		 * because any access should sigbus/sigsegv
>  		 * anyway.
>  		 */
> -		if (softleaf_is_poison_marker(entry) ||
> -		    softleaf_is_guard_marker(entry))
> +		if (softleaf_is_poison_marker(entry))
>  			return 0;
> +
> +		/*
> +		 * A guard marker on a file-backed VMA carries the
> +		 * uffd-wp state of the slot for when the guard is
> +		 * removed.
> +		 */
> +		if (softleaf_is_guard_marker(entry)) {
> +			const pte_marker old = softleaf_to_marker(entry);
> +			pte_marker marker = old;
> +
> +			if (uffd_prot_resolve)
> +				marker &= ~PTE_MARKER_UFFD_WP;
> +
> +			if (marker == old)
> +				return 0;
> +			set_pte_at(vma->vm_mm, addr, pte,
> +				   make_pte_marker(marker));
> +			return 1;
> +		}
> +
>  		/*
>  		 * If this is uffd-wp pte marker and we'd like
>  		 * to unprotect it, drop it; the next page
>
> --
> 2.39.5
>

--
Cheers, Lorenzo