[PATCH v3 4/9] mm: keep writable entries writable on protection changes

[Carlos López]

change_protection() applies vma->vm_page_prot, which lacks the write bit
for CoW and write-notified mappings. MM_CP_TRY_CHANGE_WRITABLE makes
entries writable again if a write fault would only set the write bit.
In shared mappings, it relies on the dirty bit for this, as a clean
entry might still need a write fault to notify the FS.

However, this misses clean entries that were already writable. For
example, a uffd-wp unprotect makes shmem PTEs read-only if they were
mapped by a read fault before uffd-wp was registered, so the next write
to each of them faults.

Entries that were writable before the change can stay writable, as
anything that relies on write faults write-protects the entry first, and
MM_CP_TRY_CHANGE_WRITABLE is only set if the VMA stays writable. So
check the old entry before the shared and private rules.

Note: writable anonymous entries are always exclusive and thus already
kept writable, so in private mappings this only affects PFNs that
drivers map writable into them.

Signed-off-by: Carlos López <clopez at suse.de>
---
 mm/huge_memory.c | 22 ++++++++++++++++------
 mm/mprotect.c    | 11 +++++++++++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 080b718eb7de..69f4485c277e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2269,7 +2269,8 @@ vm_fault_t do_huge_pmd_wp_page(struct vm_fault *vmf)
 }
 
 static inline bool can_change_pmd_writable(struct vm_area_struct *vma,
-					   unsigned long addr, pmd_t pmd)
+					   unsigned long addr, pmd_t oldpmd,
+					   pmd_t pmd)
 {
 	struct page *page;
 
@@ -2288,6 +2289,10 @@ static inline bool can_change_pmd_writable(struct vm_area_struct *vma,
 	if (userfaultfd_huge_pmd_wp(vma, pmd))
 		return false;
 
+	/* Writes did not need a fault before the change either. */
+	if (pmd_write(oldpmd))
+		return true;
+
 	if (!(vma->vm_flags & VM_SHARED)) {
 		/* See can_change_pte_writable(). */
 		page = vm_normal_page_pmd(vma, addr, pmd);
@@ -2317,7 +2322,7 @@ vm_fault_t do_huge_pmd_uffd_rwp(struct vm_fault *vmf)
 	pmd = pmd_mkyoung(pmd);
 	if (!pmd_write(pmd) &&
 	    vma_wants_manual_pte_write_upgrade(vma) &&
-	    can_change_pmd_writable(vma, vmf->address, pmd))
+	    can_change_pmd_writable(vma, vmf->address, vmf->orig_pmd, pmd))
 		pmd = pmd_mkwrite(pmd, vma);
 	set_pmd_at(vma->vm_mm, vmf->address & HPAGE_PMD_MASK,
 		   vmf->pmd, pmd);
@@ -2354,7 +2359,7 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf)
 	 */
 	writable = pmd_write(pmd);
 	if (!writable && vma_wants_manual_pte_write_upgrade(vma) &&
-	    can_change_pmd_writable(vma, vmf->address, pmd))
+	    can_change_pmd_writable(vma, vmf->address, old_pmd, pmd))
 		writable = true;
 
 	folio = vm_normal_folio_pmd(vma, haddr, pmd);
@@ -2820,7 +2825,7 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 
 	/* See change_pte_range(). */
 	if ((cp_flags & MM_CP_TRY_CHANGE_WRITABLE) && !pmd_write(entry) &&
-	    can_change_pmd_writable(vma, addr, entry))
+	    can_change_pmd_writable(vma, addr, oldpmd, entry))
 		entry = pmd_mkwrite(entry, vma);
 
 	ret = HPAGE_PMD_NR;
@@ -2835,7 +2840,8 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 
 #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
 static inline bool can_change_pud_writable(struct vm_area_struct *vma,
-					   unsigned long addr, pud_t pud)
+					   unsigned long addr, pud_t oldpud,
+					   pud_t pud)
 {
 	struct page *page;
 
@@ -2846,6 +2852,10 @@ static inline bool can_change_pud_writable(struct vm_area_struct *vma,
 	if (vma_soft_dirty_enabled(vma) && !pud_soft_dirty(pud))
 		return false;
 
+	/* Writes did not need a fault before the change either. */
+	if (pud_write(oldpud))
+		return true;
+
 	if (!(vma->vm_flags & VM_SHARED)) {
 		/* See can_change_pte_writable(). */
 		page = vm_normal_page_pud(vma, addr, pud);
@@ -2896,7 +2906,7 @@ int change_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma,
 	entry = pud_modify(oldpud, newprot);
 	/* See change_pte_range(). */
 	if ((cp_flags & MM_CP_TRY_CHANGE_WRITABLE) && !pud_write(entry) &&
-	    can_change_pud_writable(vma, addr, entry))
+	    can_change_pud_writable(vma, addr, oldpud, entry))
 		entry = pud_mkwrite(entry);
 	set_pud_at(mm, addr, pudp, entry);
 	tlb_flush_pud_range(tlb, addr, HPAGE_PUD_SIZE);
diff --git a/mm/mprotect.c b/mm/mprotect.c
index e18d4e12fc34..84bcfe0281d3 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -194,6 +194,17 @@ static __always_inline void set_write_prot_commit_flush_ptes(struct vm_area_stru
 {
 	bool set_write;
 
+	/*
+	 * Writes did not need a fault before the change either. All PTEs in
+	 * the batch share the write bit.
+	 */
+	if (pte_write(oldpte)) {
+		set_write = maybe_change_pte_writable(vma, ptent);
+		prot_commit_flush_ptes(vma, addr, ptep, oldpte, ptent, nr_ptes,
+				       /* idx = */ 0, set_write, tlb);
+		return;
+	}
+
 	if (vma_test(vma, VMA_SHARED_BIT)) {
 		set_write = can_change_shared_pte_writable(vma, ptent);
 		prot_commit_flush_ptes(vma, addr, ptep, oldpte, ptent, nr_ptes,
-- 
2.51.0