Without UFFD_FEATURE_EVENT_REMAP, the new VMA of an mremap() is not registered with userfaultfd: mremap_userfaultfd_prep() drops the context from it once the move succeeded. The uffd-wp bits, uffd-wp markers, and RWP PAGE_NONE entries in the new VMA must be cleared as well. However, move_ptes(), move_huge_pmd(), and move_huge_pte() clear them from each entry while moving it, before we know whether the move will succeed. If a page table allocation fails halfway through, mremap() moves the entries back into the old VMA, which is still registered, without their uffd state. Writes to a WP range are then no longer reported, and an RWP range becomes accessible without a fault. Nothing restores the protection afterwards. This requires a page table allocation to fail in the middle of mremap(), so it is rare. The uffd state must only be cleared once the move can no longer fail. To fix it, leave the entries alone during the move and clear the uffd state of the new VMA in mremap_userfaultfd_prep(), right before dropping the context, using userfaultfd_clear_ptes() like unregistering does. A failed move then puts the entries back as they were. This adds a second walk over the page tables of the new VMA, for VMAs registered for WP or RWP without UFFD_FEATURE_EVENT_REMAP. Like unregistering, userfaultfd_clear_ptes() also asks for the PTEs to be made writable again where that is safe (MM_CP_TRY_CHANGE_WRITABLE). The old code only cleared the uffd bit and left the PTE read-only. For private anonymous memory this saves the minor fault on the first write to each page that was protected. However, this adds spurious minor faults on shmem and hugetlb. When resolving, change_protection() rewrites every present entry in the range with vma->vm_page_prot, so a page that was writable and not protected is made read-only, and its first write after mremap() faults. UFFDIO_UNREGISTER and UFFDIO_WRITEPROTECT already behave this way and lead to the same spurious faults. This needs to be fixed in change_protection() and will be done in a separate change. The entries no longer need per-entry handling during the move, so drop uffd_supports_page_table_move(). Where the old and new addresses are suitably aligned, move_normal_pmd() and move_normal_pud() can move whole page tables of these VMAs again. clear_uffd_wp_pmd() has no users left, so remove it. Fixes: 0cef0bb836e3 ("mm: clear uffd-wp PTE/PMD state on mremap()") Fixes: 763f43865fc4 ("mm: preserve RWP marker across PTE rewrites") Signed-off-by: Tal Zussman --- mm/huge_memory.c | 25 ------------------------- mm/hugetlb.c | 27 +-------------------------- mm/mremap.c | 46 +--------------------------------------------- mm/userfaultfd.c | 1 + 4 files changed, 3 insertions(+), 96 deletions(-) diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 09176b290f76..468a49041b95 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2616,18 +2616,6 @@ static pmd_t move_soft_dirty_pmd(pmd_t pmd) return pmd; } -static pmd_t clear_uffd_wp_pmd(pmd_t pmd) -{ - if (pmd_none(pmd)) - return pmd; - if (pmd_present(pmd)) - pmd = pmd_clear_uffd(pmd); - else - pmd = pmd_swp_clear_uffd(pmd); - - return pmd; -} - bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr, unsigned long new_addr, pmd_t *old_pmd, pmd_t *new_pmd) { @@ -2663,19 +2651,6 @@ bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr, pgtable_trans_huge_deposit(mm, new_pmd, pgtable); } pmd = move_soft_dirty_pmd(pmd); - if (vma_has_uffd_without_event_remap(vma)) { - /* - * See __copy_present_ptes(): normalise the RWP marker - * so the destination starts accessible instead of - * taking a numa-hinting fault on first access. Only the - * marker (protnone + uffd) needs it; leave other present - * PMDs in the VMA untouched. - */ - if (pmd_present(pmd) && userfaultfd_rwp(vma) && - pmd_uffd(pmd)) - pmd = pmd_modify(pmd, vma->vm_page_prot); - pmd = clear_uffd_wp_pmd(pmd); - } set_pmd_at(mm, new_addr, new_pmd, pmd); if (force_flush) flush_pmd_tlb_range(vma, old_addr, old_addr + PMD_SIZE); diff --git a/mm/hugetlb.c b/mm/hugetlb.c index cb2de24affdc..97bd29925476 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -5168,7 +5168,6 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr, unsigned long new_addr, pte_t *src_pte, pte_t *dst_pte, unsigned long sz) { - bool need_clear_uffd_wp = vma_has_uffd_without_event_remap(vma); struct hstate *h = hstate_vma(vma); struct mm_struct *mm = vma->vm_mm; spinlock_t *src_ptl, *dst_ptl; @@ -5179,31 +5178,7 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr, double_pt_lock(dst_ptl, src_ptl); pte = huge_ptep_get_and_clear(mm, old_addr, src_pte, sz); - - if (need_clear_uffd_wp && pte_is_uffd_wp_marker(pte)) { - huge_pte_clear(mm, new_addr, dst_pte, sz); - } else { - if (need_clear_uffd_wp) { - if (pte_present(pte)) { - /* - * See __copy_present_ptes(): normalise the RWP - * marker so the destination starts accessible - * instead of taking a numa-hinting fault on - * first access. Only the marker (protnone + uffd) - * needs it; leave other present PTEs untouched. - */ - if (userfaultfd_rwp(vma) && huge_pte_uffd(pte)) { - pte = huge_pte_modify(pte, vma->vm_page_prot); - pte = arch_make_huge_pte(pte, huge_page_shift(h), - vma->vm_flags); - } - pte = huge_pte_clear_uffd(pte); - } else { - pte = pte_swp_clear_uffd(pte); - } - } - set_huge_pte_at(mm, new_addr, dst_pte, pte, sz); - } + set_huge_pte_at(mm, new_addr, dst_pte, pte, sz); double_pt_unlock(dst_ptl, src_ptl); } diff --git a/mm/mremap.c b/mm/mremap.c index fff0465f5e06..4564416a6994 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -198,7 +198,6 @@ static int move_ptes(struct pagetable_move_control *pmc, unsigned long extent, pmd_t *old_pmd, pmd_t *new_pmd) { struct vm_area_struct *vma = pmc->old; - bool need_clear_uffd_wp = vma_has_uffd_without_event_remap(vma); struct mm_struct *mm = vma->vm_mm; pte_t *old_ptep, *new_ptep; pte_t old_pte, pte; @@ -290,27 +289,7 @@ static int move_ptes(struct pagetable_move_control *pmc, pte = get_and_clear_ptes(mm, old_addr, old_ptep, nr_ptes); pte = move_pte(pte, old_addr, new_addr); pte = move_soft_dirty_pte(pte); - - if (need_clear_uffd_wp && pte_is_uffd_wp_marker(pte)) - pte_clear(mm, new_addr, new_ptep); - else { - if (need_clear_uffd_wp) { - if (pte_present(pte)) { - /* - * See __copy_present_ptes(): normalise - * RWP PTEs so the destination starts - * accessible instead of taking a - * numa-hinting fault on first access. - */ - if (userfaultfd_rwp(vma) && pte_uffd(pte)) - pte = pte_modify(pte, vma->vm_page_prot); - pte = pte_clear_uffd(pte); - } else { - pte = pte_swp_clear_uffd(pte); - } - } - set_ptes(mm, new_addr, new_ptep, pte, nr_ptes); - } + set_ptes(mm, new_addr, new_ptep, pte, nr_ptes); } lazy_mmu_mode_disable(); @@ -334,25 +313,6 @@ static inline bool arch_supports_page_table_move(void) } #endif -static inline bool uffd_supports_page_table_move(struct pagetable_move_control *pmc) -{ - /* - * If we are moving a VMA that has uffd-wp registered but with - * remap events disabled (new VMA will not be registered with uffd), we - * need to ensure that the uffd-wp state is cleared from all pgtables. - * This means recursing into lower page tables in move_page_tables(). - * - * We might get called with VMAs reversed when recovering from a - * failed page table move. In that case, the - * "old"-but-actually-"originally new" VMA during recovery will not have - * a uffd context. Recursing into lower page tables during the original - * move but not during the recovery move will cause trouble, because we - * run into already-existing page tables. So check both VMAs. - */ - return !vma_has_uffd_without_event_remap(pmc->old) && - !vma_has_uffd_without_event_remap(pmc->new); -} - #ifdef CONFIG_HAVE_MOVE_PMD static bool move_normal_pmd(struct pagetable_move_control *pmc, pmd_t *old_pmd, pmd_t *new_pmd) @@ -365,8 +325,6 @@ static bool move_normal_pmd(struct pagetable_move_control *pmc, if (!arch_supports_page_table_move()) return false; - if (!uffd_supports_page_table_move(pmc)) - return false; /* * The destination pmd shouldn't be established, free_pgtables() * should have released it. @@ -434,8 +392,6 @@ static bool move_normal_pud(struct pagetable_move_control *pmc, if (!arch_supports_page_table_move()) return false; - if (!uffd_supports_page_table_move(pmc)) - return false; /* * The destination pud shouldn't be established, free_pgtables() * should have released it. diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c index ad9d243dc64b..bf42fabb5afd 100644 --- a/mm/userfaultfd.c +++ b/mm/userfaultfd.c @@ -3161,6 +3161,7 @@ void mremap_userfaultfd_prep(struct vm_area_struct *vma, up_write(&ctx->map_changing_lock); } else { /* Drop uffd context if remap feature not enabled */ + userfaultfd_clear_ptes(vma, vma->vm_start, vma->vm_end); userfaultfd_reset_ctx(vma); } } -- 2.39.5