Re: [PATCH] mm/madvise: disallow guard region installation in uffd-armed VMAs

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 11/12/26 18:38, Lorenzo Stoakes (ARM) wrote:
> In order to establish guard regions, guard markers are installed in the
> page tables of a mapped VMA.
> 
> However, problems arise when one tries to combine this with registering a
> VMA for userfaultfd, as such operations may themselves need to install page
> table markers such as uffd-wp.
> 
> When this installation is attempted the guard marker prevents these markers
> being applied, which can lead to unexpected results (especially if the
> guard regions are later removed.)
> 
> This is especially problematic because guard region installation causes the
> regions to be zapped first, meaning that the removal of userfaultfd-tracked
> ranges will not be logged in this case.
> 
> Therefore, disallow this.
> 
> This makes sense as conceptually, guard regions mark a part of a VMA as
> effectively unmapped at the page table level, and removing the markers is
> like mapping the VMA again, over which operation uffd markers would in any
> case be lost.
> 
> Only check for this on installation, as a VMA can be registered for
> userfaultfd after guard regions are established.
> 
> In this case, the guard regions cannot have new markers installed, and no
> faults can occur in the guard regions, until they are removed, at which
> point everything should function normally.

So IIUC this means the other order of operations (first guard regions, then
uffd) already prevents the two co-existing. Makes sense to prevent the same
in the order of uffd and then guard regions then.

> A survey of open source users indicates nobody is relying on installing
> guard regions in uffd-armed VMAs. Software including glibc, android,
> tcmalloc and CRIU were checked.
> 
> Fix the selftest to reflect the change.
> 
> Fixes: 662df3e5c376 ("mm: madvise: implement lightweight guard page mechanism")
> Reported-by: Tal Zussman <tz2294@columbia.edu>
> Closes: https://lore.kernel.org/all/20261111-uffd-state-fixes-v1-9-00713d368976@columbia.edu/
> Closes: https://lore.kernel.org/all/20261111-uffd-state-fixes-v1-10-00713d368976@columbia.edu/
> Closes: https://lore.kernel.org/all/20261111-uffd-state-fixes-v1-11-00713d368976@columbia.edu/
> Cc: stable@vger.kernel.org
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/madvise.c                               |  4 ++++
>  tools/testing/selftests/mm/guard-regions.c | 12 ++++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 0214efd2a014..268731e90f12 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1156,6 +1156,10 @@ static long madvise_guard_install(struct madvise_behavior *madv_behavior)
>  	if (!is_valid_guard_vma(vma, /* allow_locked = */false))
>  		return -EINVAL;
>  
> +	/* Guard markers are incompatible with UFFD markers. */
> +	if (userfaultfd_armed(vma))
> +		return -EINVAL;
> +
>  	/*
>  	 * Set atomically under read lock. All pertinent readers will need to
>  	 * acquire an mmap/VMA write lock to read it. All remaining readers may
> diff --git a/tools/testing/selftests/mm/guard-regions.c b/tools/testing/selftests/mm/guard-regions.c
> index 02f69244f9a4..97da54dcf3a4 100644
> --- a/tools/testing/selftests/mm/guard-regions.c
> +++ b/tools/testing/selftests/mm/guard-regions.c
> @@ -1476,7 +1476,7 @@ TEST_F(guard_regions, cold_pageout)
>  	ASSERT_EQ(munmap(ptr, 10 * page_size), 0);
>  }
>  
> -/* Ensure that guard pages do not break userfaultd. */
> +/* Ensure that guard regions cannot be installed in uffd-armed VMAs. */
>  TEST_F(guard_regions, uffd)
>  {
>  	const unsigned long page_size = self->page_size;
> @@ -1526,10 +1526,15 @@ TEST_F(guard_regions, uffd)
>  	reg.mode = UFFDIO_REGISTER_MODE_MISSING;
>  	ASSERT_EQ(ioctl(uffd, UFFDIO_REGISTER, &reg), 0);
>  
> -	/* Guard the range. This should not trigger the uffd. */
> +	/* Now try to guard, should fail with EINVAL. */
> +	ASSERT_EQ(madvise(ptr, 10 * page_size, MADV_GUARD_INSTALL), -1);
> +	ASSERT_EQ(errno, EINVAL);
> +
> +	/* Once unregistered, we should be able to guard the range. */
> +	ASSERT_EQ(ioctl(uffd, UFFDIO_UNREGISTER, &range), 0);
>  	ASSERT_EQ(madvise(ptr, 10 * page_size, MADV_GUARD_INSTALL), 0);
>  
> -	/* The guarding should behave as usual with no uffd intervention. */
> +	/* The guarding should behave as usual. */
>  	for (i = 0; i < 10; i++) {
>  		char *curr = &ptr[i * page_size];
>  
> @@ -1537,7 +1542,6 @@ TEST_F(guard_regions, uffd)
>  	}
>  
>  	/* Cleanup. */
> -	ASSERT_EQ(ioctl(uffd, UFFDIO_UNREGISTER, &range), 0);
>  	close(uffd);
>  	ASSERT_EQ(munmap(ptr, 10 * page_size), 0);
>  }
> 
> ---
> base-commit: 4318887a3efa4c53d6f083ccfd7105b809a3ac65
> change-id: 20261112-disallow-uffd-guard-install-a5294129ad7b
> 
> Cheers,