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

[Tal Zussman]

On 11/12/26 6:21 AM, Lorenzo Stoakes (ARM) wrote:
> 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 :)
> 

Sorry about that. These started off as one fix and then grew a couple more on
further rounds of AI review...

I'll drop this and the other two guard patches.

>> ---
>>  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
>