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

[Tal Zussman]

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