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