hugetlb: Disable huge pmd unshare for uffd-wp

Huge pmd sharing for hugetlbfs is racy with userfaultfd-wp because userfaultfd-wp is always based on pgtable entries, so they cannot be shared.

Walk the hugetlb range and unshare all such mappings if there is, right before UFFDIO_REGISTER will succeed and return to userspace.

This will pair with want_pmd_share() in hugetlb code so that huge pmd sharing is completely disabled for userfaultfd-wp registered range.

Signed-off-by Peter Xu <[email protected]>

---
 fs/userfaultfd.c        |  4 ++++
 include/linux/hugetlb.h |  1 +
 mm/hugetlb.c            | 51 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)

fs/userfaultfd.c +4 -0

index 894cc28142e7..e259318fcae1 100644
@@ -15,6 +15,7 @@
 #include <linux/sched/signal.h>
 #include <linux/sched/mm.h>
 #include <linux/mm.h>
+#include <linux/mmu_notifier.h>
 #include <linux/poll.h>
 #include <linux/slab.h>
 #include <linux/seq_file.h>

@@ -1448,6 +1449,9 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
 		vma->vm_flags = new_flags;
 		vma->vm_userfaultfd_ctx.ctx = ctx;
 
+		if (is_vm_hugetlb_page(vma) && uffd_disable_huge_pmd_share(vma))
+			hugetlb_unshare_all_pmds(vma);
+
 	skip:
 		prev = vma;
 		start = vma->vm_end;

include/linux/hugetlb.h +1 -0

index 3b4104021dd3..97ecfd4c20b2 100644
@@ -188,6 +188,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
 		unsigned long address, unsigned long end, pgprot_t newprot);
 
 bool is_hugetlb_entry_migration(pte_t pte);
+void hugetlb_unshare_all_pmds(struct vm_area_struct *vma);
 
 #else /* !CONFIG_HUGETLB_PAGE */
 

mm/hugetlb.c +0 -0

index f53a0b852ed8..83c006ea3ff9 100644

mm/hugetlb.c +12 -0

+void hugetlb_unshare_all_pmds(struct vm_area_struct *vma)
+{
+	struct hstate *h = hstate_vma(vma);
+	unsigned long sz = huge_page_size(h);
+	struct mm_struct *mm = vma->vm_mm;
+	struct mmu_notifier_range range;
+	unsigned long address, start, end;
+	spinlock_t *ptl;
+	pte_t *ptep;
+
+	if (!(vma->vm_flags & VM_MAYSHARE))
+		return;

mm/hugetlb.c +4 -0

+	mmu_notifier_invalidate_range_start(&range);
+	i_mmap_lock_write(vma->vm_file->f_mapping);
+	for (address = start; address < end; address += PUD_SIZE) {
+		unsigned long tmp = address;

mm/hugetlb.c +8 -0

+	i_mmap_unlock_write(vma->vm_file->f_mapping);
+	/*
+	 * No need to call mmu_notifier_invalidate_range(), see
+	 * Documentation/vm/mmu_notifier.rst.
+	 */
+	mmu_notifier_invalidate_range_end(&range);
+}
+
 #endif /* CONFIG_CMA */

Quoted 55 lines · Peter Xu

Huge pmd sharing for hugetlbfs is racy with userfaultfd-wp because userfaultfd-wp is always based on pgtable entries, so they cannot be shared.

Walk the hugetlb range and unshare all such mappings if there is, right before UFFDIO_REGISTER will succeed and return to userspace.

This will pair with want_pmd_share() in hugetlb code so that huge pmd sharing is completely disabled for userfaultfd-wp registered range.

Signed-off-by Peter Xu <[email protected]>

---
 fs/userfaultfd.c        |  4 ++++
 include/linux/hugetlb.h |  1 +
 mm/hugetlb.c            | 51 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)

fs/userfaultfd.c +4 -0

index 894cc28142e7..e259318fcae1 100644
@@ -15,6 +15,7 @@
 #include <linux/sched/signal.h>
 #include <linux/sched/mm.h>
 #include <linux/mm.h>
+#include <linux/mmu_notifier.h>
 #include <linux/poll.h>
 #include <linux/slab.h>
 #include <linux/seq_file.h>

@@ -1448,6 +1449,9 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
 		vma->vm_flags = new_flags;
 		vma->vm_userfaultfd_ctx.ctx = ctx;
 
+		if (is_vm_hugetlb_page(vma) && uffd_disable_huge_pmd_share(vma))
+			hugetlb_unshare_all_pmds(vma);
+
 	skip:
 		prev = vma;
 		start = vma->vm_end;

include/linux/hugetlb.h +1 -0

index 3b4104021dd3..97ecfd4c20b2 100644
@@ -188,6 +188,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
 		unsigned long address, unsigned long end, pgprot_t newprot);
 
 bool is_hugetlb_entry_migration(pte_t pte);
+void hugetlb_unshare_all_pmds(struct vm_area_struct *vma);
 
 #else /* !CONFIG_HUGETLB_PAGE */
 

mm/hugetlb.c +0 -0

index f53a0b852ed8..83c006ea3ff9 100644

mm/hugetlb.c void __init hugetlb_cma_check(void) · line 5729

Quoted 7 lines

mm/hugetlb.c +4 -0

@@ -5723,4 +5723,55 @@ void __init hugetlb_cma_check(void)
 	pr_warn("hugetlb_cma: the option isn't supported by current arch\n");
 }
 
+/*
+ * This function will unconditionally remove all the shared pmd pgtable entries
+ * within the specific vma for a hugetlbfs memory range.
+ */

Thanks for updating this!

Quoted 12 lines

mm/hugetlb.c +12 -0

+void hugetlb_unshare_all_pmds(struct vm_area_struct *vma)
+{
+	struct hstate *h = hstate_vma(vma);
+	unsigned long sz = huge_page_size(h);
+	struct mm_struct *mm = vma->vm_mm;
+	struct mmu_notifier_range range;
+	unsigned long address, start, end;
+	spinlock_t *ptl;
+	pte_t *ptep;
+
+	if (!(vma->vm_flags & VM_MAYSHARE))
+		return;

mm/hugetlb.c void __init hugetlb_cma_check(void) · line 5751

Quoted 10 lines

mm/hugetlb.c +10 -0

+
+	start = ALIGN(vma->vm_start, PUD_SIZE);
+	end = ALIGN_DOWN(vma->vm_end, PUD_SIZE);
+
+	if (start >= end)
+		return;
+
+	/*
+	 * No need to call adjust_range_if_pmd_sharing_possible(), because
+	 * we're going to operate on the whole vma

not necessary, but perhaps change to:
* we're going to operate on ever PUD_SIZE aligned sized range
* within the vma.

mm/hugetlb.c void __init hugetlb_cma_check(void) · line 5754

Quoted 4 lines

mm/hugetlb.c +4 -0

+	 * we're going to operate on the whole vma
+	 */
+	mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, mm,
+				vma->vm_start, vma->vm_end);

Should we use start, end here instead of vma->vm_start, vma->vm_end ?

Quoted 4 lines

mm/hugetlb.c +4 -0

+	mmu_notifier_invalidate_range_start(&range);
+	i_mmap_lock_write(vma->vm_file->f_mapping);
+	for (address = start; address < end; address += PUD_SIZE) {
+		unsigned long tmp = address;

mm/hugetlb.c void __init hugetlb_cma_check(void) · line 5768

Quoted 10 lines

mm/hugetlb.c +10 -0

+
+		ptep = huge_pte_offset(mm, address, sz);
+		if (!ptep)
+			continue;
+		ptl = huge_pte_lock(h, mm, ptep);
+		/* We don't want 'address' to be changed */
+		huge_pmd_unshare(mm, vma, &tmp, ptep);
+		spin_unlock(ptl);
+	}
+	flush_hugetlb_tlb_range(vma, vma->vm_start, vma->vm_end);

start, end ?

Huge pmd sharing for hugetlbfs is racy with userfaultfd-wp because userfaultfd-wp is always based on pgtable entries, so they cannot be shared.

Walk the hugetlb range and unshare all such mappings if there is, right before UFFDIO_REGISTER will succeed and return to userspace.

This will pair with want_pmd_share() in hugetlb code so that huge pmd sharing is completely disabled for userfaultfd-wp registered range.

Signed-off-by Peter Xu <[email protected]>

---
 fs/userfaultfd.c        |  4 ++++
 include/linux/hugetlb.h |  1 +
 mm/hugetlb.c            | 51 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)

fs/userfaultfd.c +1 -0

index 894cc28142e7..e259318fcae1 100644
@@ -15,6 +15,7 @@
 #include <linux/sched/signal.h>
 #include <linux/sched/mm.h>
 #include <linux/mm.h>
+#include <linux/mmu_notifier.h>
 #include <linux/poll.h>
 #include <linux/slab.h>
 #include <linux/seq_file.h>

fs/userfaultfd.c +1 -0

+
        skip:
                prev = vma;
                start = vma->vm_end;

include/linux/hugetlb.h +1 -0

index 3b4104021dd3..97ecfd4c20b2 100644
@@ -188,6 +188,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
                unsigned long address, unsigned long end, pgprot_t newprot);
 
 bool is_hugetlb_entry_migration(pte_t pte);
+void hugetlb_unshare_all_pmds(struct vm_area_struct *vma);
 
 #else /* !CONFIG_HUGETLB_PAGE */
 

mm/hugetlb.c +51 -1

index f53a0b852ed8..83c006ea3ff9 100644
@@ -5723,4 +5723,55 @@ void __init hugetlb_cma_check(void)
        pr_warn("hugetlb_cma: the option isn't supported by current arch\n");
 }
 
+/*
+ * This function will unconditionally remove all the shared pmd pgtable entries
+ * within the specific vma for a hugetlbfs memory range.
+ */
+void hugetlb_unshare_all_pmds(struct vm_area_struct *vma)
+{
+       struct hstate *h = hstate_vma(vma);
+       unsigned long sz = huge_page_size(h);
+       struct mm_struct *mm = vma->vm_mm;
+       struct mmu_notifier_range range;
+       unsigned long address, start, end;
+       spinlock_t *ptl;
+       pte_t *ptep;
+
+       if (!(vma->vm_flags & VM_MAYSHARE))
+               return;
+
+       start = ALIGN(vma->vm_start, PUD_SIZE);
+       end = ALIGN_DOWN(vma->vm_end, PUD_SIZE);
+
+       if (start >= end)
+               return;
+
+       /*
+        * No need to call adjust_range_if_pmd_sharing_possible(), because
+        * we're going to operate on the whole vma
+        */
+       mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, mm,
+                               vma->vm_start, vma->vm_end);
+       mmu_notifier_invalidate_range_start(&range);
+       i_mmap_lock_write(vma->vm_file->f_mapping);
+       for (address = start; address < end; address += PUD_SIZE) {
+               unsigned long tmp = address;
+
+               ptep = huge_pte_offset(mm, address, sz);
+               if (!ptep)
+                       continue;
+               ptl = huge_pte_lock(h, mm, ptep);
+               /* We don't want 'address' to be changed */
+               huge_pmd_unshare(mm, vma, &tmp, ptep);
+               spin_unlock(ptl);
+       }
+       flush_hugetlb_tlb_range(vma, vma->vm_start, vma->vm_end);
+       i_mmap_unlock_write(vma->vm_file->f_mapping);
+       /*
+        * No need to call mmu_notifier_invalidate_range(), see
+        * Documentation/vm/mmu_notifier.rst.
+        */
+       mmu_notifier_invalidate_range_end(&range);
+}
+
 #endif /* CONFIG_CMA */
--

2.26.2

fs/userfaultfd.c static int userfaultfd_register(struct userfaultfd_ctx *ctx, · line 1453

Quoted 6 lines

fs/userfaultfd.c +2 -0

@@ -1448,6 +1449,9 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
                vma->vm_flags = new_flags;
                vma->vm_userfaultfd_ctx.ctx = ctx;
 
+               if (is_vm_hugetlb_page(vma) && uffd_disable_huge_pmd_share(vma))
+                       hugetlb_unshare_all_pmds(vma);

This line yields the following error, if building with:
# CONFIG_CMA is not set

./fs/userfaultfd.c:1459: undefined reference to `hugetlb_unshare_all_pmds'

Quoted 10 lines · Axel Rasmussen

fs/userfaultfd.c static int userfaultfd_register(struct userfaultfd_ctx *ctx, · line 1453

Quoted 6 lines

fs/userfaultfd.c +2 -0

@@ -1448,6 +1449,9 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
                vma->vm_flags = new_flags;
                vma->vm_userfaultfd_ctx.ctx = ctx;
 
+               if (is_vm_hugetlb_page(vma) && uffd_disable_huge_pmd_share(vma))
+                       hugetlb_unshare_all_pmds(vma);

This line yields the following error, if building with:
# CONFIG_CMA is not set

Quoted 1 line

./fs/userfaultfd.c:1459: undefined reference to `hugetlb_unshare_all_pmds'

Ouch.. Axel, you mean CONFIG_HUGETLBFS rather than CONFIG_CMA, am I right?

--
Peter Xu

Quoted 12 lines · Peter Xu

Quoted 10 lines · Axel Rasmussen

fs/userfaultfd.c static int userfaultfd_register(struct userfaultfd_ctx *ctx, · line 1453

Quoted 6 lines

fs/userfaultfd.c +2 -0

@@ -1448,6 +1449,9 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
                vma->vm_flags = new_flags;
                vma->vm_userfaultfd_ctx.ctx = ctx;
 
+               if (is_vm_hugetlb_page(vma) && uffd_disable_huge_pmd_share(vma))
+                       hugetlb_unshare_all_pmds(vma);

This line yields the following error, if building with:
# CONFIG_CMA is not set

Quoted 3 lines

Quoted 1 line

./fs/userfaultfd.c:1459: undefined reference to `hugetlb_unshare_all_pmds'

Ouch.. Axel, you mean CONFIG_HUGETLBFS rather than CONFIG_CMA, am I right?

Surprisingly no, there's a "#ifdef CONFIG_CMA" line ~100 lines above where hugetlb_unshare_all_pmds is defined in hugetlb.c which causes this. My guess is that putting the function inside that block was accidental and it can just be moved.

Huge pmd sharing could bring problem to userfaultfd. The thing is that userfaultfd is running its logic based on the special bits on page table entries, however the huge pmd sharing could potentially share page table entries for different address ranges. That could cause issues on either:

- When sharing huge pmd page tables for an uffd write protected range, the newly mapped huge pmd range will also be write protected unexpectedly, or,

- When we try to write protect a range of huge pmd shared range, we'll first do huge_pmd_unshare() in hugetlb_change_protection(), however that also means the UFFDIO_WRITEPROTECT could be silently skipped for the shared region, which could lead to data loss.

Since at it, a few other things are done altogether:

- Move want_pmd_share() from mm/hugetlb.c into linux/hugetlb.h, because that's definitely something that arch code would like to use too

- ARM64 currently directly check against CONFIG_ARCH_WANT_HUGE_PMD_SHARE when trying to share huge pmd. Switch to the want_pmd_share() helper.

Since at it, move vma_shareable() from huge_pmd_share() into want_pmd_share().

Signed-off-by Peter Xu <[email protected]>

---
 arch/arm64/mm/hugetlbpage.c   |  3 +--
 include/linux/hugetlb.h       |  2 ++
 include/linux/userfaultfd_k.h |  9 +++++++++
 mm/hugetlb.c                  | 20 ++++++++++++++------
 4 files changed, 26 insertions(+), 8 deletions(-)

v2:
- patch 4: move hugetlb_unshare_all_pmds() into mm/hugetlb.c, so it can be used even outside userfaultfd.c

This series tries to disable huge pmd unshare of hugetlbfs backed memory for uffd-wp. Although uffd-wp of hugetlbfs is still during rfc stage, the idea of this series may be needed for multiple tasks (Axel's uffd minor fault series, and Mike's soft dirty series), so I picked it out from the larger series.

References works:

Uffd shmem+hugetlbfs rfc:
https://lore.kernel.org/lkml/[email protected]/

Uffd minor mode for hugetlbfs:
https://lore.kernel.org/lkml/[email protected]/

Soft dirty for hugetlbfs:
https://lore.kernel.org/lkml/[email protected]/

Please review, thanks.

Peter Xu (4):
hugetlb: Pass vma into huge_pte_alloc() and huge_pmd_share() hugetlb/userfaultfd: Forbid huge pmd sharing when uffd enabled mm/hugetlb: Move flush_hugetlb_tlb_range() into hugetlb.h hugetlb/userfaultfd: Unshare all pmds for hugetlbfs when register wp

 arch/arm64/mm/hugetlbpage.c   |  7 ++-
 arch/ia64/mm/hugetlbpage.c    |  3 +-
 arch/mips/mm/hugetlbpage.c    |  4 +-
 arch/parisc/mm/hugetlbpage.c  |  2 +-
 arch/powerpc/mm/hugetlbpage.c |  3 +-
 arch/s390/mm/hugetlbpage.c    |  2 +-
 arch/sh/mm/hugetlbpage.c      |  2 +-
 arch/sparc/mm/hugetlbpage.c   |  1 +
 fs/userfaultfd.c              |  4 ++
 include/linux/hugetlb.h       | 16 +++++-
 include/linux/userfaultfd_k.h |  9 ++++
 mm/hugetlb.c                  | 94 +++++++++++++++++++++++++++--------
 mm/userfaultfd.c              |  2 +-
 13 files changed, 114 insertions(+), 35 deletions(-)

--
2.26.2

I reviewed these patches, rebased my minor fault handling series on top of this series, and then ran some stress tests of minor fault handling. Other than the one comment I left about !CONFIG_CMA, I didn't spot any issues. So:

Tested-by Axel Rasmussen <[email protected]>

(Or Reviewed-By: , if that makes more sense.)