Linux kernel -stable discussions
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] mm/hugetlb: unshare page tables during VMA split, not before" failed to apply to 6.1-stable tree
@ 2025-06-20 10:36 gregkh
  2025-06-20 21:33 ` [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before Jann Horn
  0 siblings, 1 reply; 9+ messages in thread
From: gregkh @ 2025-06-20 10:36 UTC (permalink / raw)
  To: jannh, akpm, liam.howlett, lorenzo.stoakes, osalvador, stable,
	vbabka
  Cc: stable


The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 081056dc00a27bccb55ccc3c6f230a3d5fd3f7e0
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2025062041-uplifted-cahoots-6c42@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..

Possible dependencies:



thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 081056dc00a27bccb55ccc3c6f230a3d5fd3f7e0 Mon Sep 17 00:00:00 2001
From: Jann Horn <jannh@google.com>
Date: Tue, 27 May 2025 23:23:53 +0200
Subject: [PATCH] mm/hugetlb: unshare page tables during VMA split, not before

Currently, __split_vma() triggers hugetlb page table unsharing through
vm_ops->may_split().  This happens before the VMA lock and rmap locks are
taken - which is too early, it allows racing VMA-locked page faults in our
process and racing rmap walks from other processes to cause page tables to
be shared again before we actually perform the split.

Fix it by explicitly calling into the hugetlb unshare logic from
__split_vma() in the same place where THP splitting also happens.  At that
point, both the VMA and the rmap(s) are write-locked.

An annoying detail is that we can now call into the helper
hugetlb_unshare_pmds() from two different locking contexts:

1. from hugetlb_split(), holding:
    - mmap lock (exclusively)
    - VMA lock
    - file rmap lock (exclusively)
2. hugetlb_unshare_all_pmds(), which I think is designed to be able to
   call us with only the mmap lock held (in shared mode), but currently
   only runs while holding mmap lock (exclusively) and VMA lock

Backporting note:
This commit fixes a racy protection that was introduced in commit
b30c14cd6102 ("hugetlb: unshare some PMDs when splitting VMAs"); that
commit claimed to fix an issue introduced in 5.13, but it should actually
also go all the way back.

[jannh@google.com: v2]
  Link: https://lkml.kernel.org/r/20250528-hugetlb-fixes-splitrace-v2-1-1329349bad1a@google.com
Link: https://lkml.kernel.org/r/20250528-hugetlb-fixes-splitrace-v2-0-1329349bad1a@google.com
Link: https://lkml.kernel.org/r/20250527-hugetlb-fixes-splitrace-v1-1-f4136f5ec58a@google.com
Fixes: 39dde65c9940 ("[PATCH] shared page table for hugetlb page")
Signed-off-by: Jann Horn <jannh@google.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: <stable@vger.kernel.org>	[b30c14cd6102: hugetlb: unshare some PMDs when splitting VMAs]
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 0598f36931de..42f374e828a2 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -279,6 +279,7 @@ bool is_hugetlb_entry_migration(pte_t pte);
 bool is_hugetlb_entry_hwpoisoned(pte_t pte);
 void hugetlb_unshare_all_pmds(struct vm_area_struct *vma);
 void fixup_hugetlb_reservations(struct vm_area_struct *vma);
+void hugetlb_split(struct vm_area_struct *vma, unsigned long addr);
 
 #else /* !CONFIG_HUGETLB_PAGE */
 
@@ -476,6 +477,8 @@ static inline void fixup_hugetlb_reservations(struct vm_area_struct *vma)
 {
 }
 
+static inline void hugetlb_split(struct vm_area_struct *vma, unsigned long addr) {}
+
 #endif /* !CONFIG_HUGETLB_PAGE */
 
 #ifndef pgd_write
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f0b1d53079f9..7ba020d489d4 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -121,7 +121,7 @@ static void hugetlb_vma_lock_free(struct vm_area_struct *vma);
 static void hugetlb_vma_lock_alloc(struct vm_area_struct *vma);
 static void __hugetlb_vma_unlock_write_free(struct vm_area_struct *vma);
 static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
-		unsigned long start, unsigned long end);
+		unsigned long start, unsigned long end, bool take_locks);
 static struct resv_map *vma_resv_map(struct vm_area_struct *vma);
 
 static void hugetlb_free_folio(struct folio *folio)
@@ -5426,26 +5426,40 @@ static int hugetlb_vm_op_split(struct vm_area_struct *vma, unsigned long addr)
 {
 	if (addr & ~(huge_page_mask(hstate_vma(vma))))
 		return -EINVAL;
+	return 0;
+}
 
+void hugetlb_split(struct vm_area_struct *vma, unsigned long addr)
+{
 	/*
 	 * PMD sharing is only possible for PUD_SIZE-aligned address ranges
 	 * in HugeTLB VMAs. If we will lose PUD_SIZE alignment due to this
 	 * split, unshare PMDs in the PUD_SIZE interval surrounding addr now.
+	 * This function is called in the middle of a VMA split operation, with
+	 * MM, VMA and rmap all write-locked to prevent concurrent page table
+	 * walks (except hardware and gup_fast()).
 	 */
+	vma_assert_write_locked(vma);
+	i_mmap_assert_write_locked(vma->vm_file->f_mapping);
+
 	if (addr & ~PUD_MASK) {
-		/*
-		 * hugetlb_vm_op_split is called right before we attempt to
-		 * split the VMA. We will need to unshare PMDs in the old and
-		 * new VMAs, so let's unshare before we split.
-		 */
 		unsigned long floor = addr & PUD_MASK;
 		unsigned long ceil = floor + PUD_SIZE;
 
-		if (floor >= vma->vm_start && ceil <= vma->vm_end)
-			hugetlb_unshare_pmds(vma, floor, ceil);
+		if (floor >= vma->vm_start && ceil <= vma->vm_end) {
+			/*
+			 * Locking:
+			 * Use take_locks=false here.
+			 * The file rmap lock is already held.
+			 * The hugetlb VMA lock can't be taken when we already
+			 * hold the file rmap lock, and we don't need it because
+			 * its purpose is to synchronize against concurrent page
+			 * table walks, which are not possible thanks to the
+			 * locks held by our caller.
+			 */
+			hugetlb_unshare_pmds(vma, floor, ceil, /* take_locks = */ false);
+		}
 	}
-
-	return 0;
 }
 
 static unsigned long hugetlb_vm_op_pagesize(struct vm_area_struct *vma)
@@ -7885,9 +7899,16 @@ void move_hugetlb_state(struct folio *old_folio, struct folio *new_folio, int re
 	spin_unlock_irq(&hugetlb_lock);
 }
 
+/*
+ * If @take_locks is false, the caller must ensure that no concurrent page table
+ * access can happen (except for gup_fast() and hardware page walks).
+ * If @take_locks is true, we take the hugetlb VMA lock (to lock out things like
+ * concurrent page fault handling) and the file rmap lock.
+ */
 static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
 				   unsigned long start,
-				   unsigned long end)
+				   unsigned long end,
+				   bool take_locks)
 {
 	struct hstate *h = hstate_vma(vma);
 	unsigned long sz = huge_page_size(h);
@@ -7911,8 +7932,12 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
 	mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm,
 				start, end);
 	mmu_notifier_invalidate_range_start(&range);
-	hugetlb_vma_lock_write(vma);
-	i_mmap_lock_write(vma->vm_file->f_mapping);
+	if (take_locks) {
+		hugetlb_vma_lock_write(vma);
+		i_mmap_lock_write(vma->vm_file->f_mapping);
+	} else {
+		i_mmap_assert_write_locked(vma->vm_file->f_mapping);
+	}
 	for (address = start; address < end; address += PUD_SIZE) {
 		ptep = hugetlb_walk(vma, address, sz);
 		if (!ptep)
@@ -7922,8 +7947,10 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
 		spin_unlock(ptl);
 	}
 	flush_hugetlb_tlb_range(vma, start, end);
-	i_mmap_unlock_write(vma->vm_file->f_mapping);
-	hugetlb_vma_unlock_write(vma);
+	if (take_locks) {
+		i_mmap_unlock_write(vma->vm_file->f_mapping);
+		hugetlb_vma_unlock_write(vma);
+	}
 	/*
 	 * No need to call mmu_notifier_arch_invalidate_secondary_tlbs(), see
 	 * Documentation/mm/mmu_notifier.rst.
@@ -7938,7 +7965,8 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
 void hugetlb_unshare_all_pmds(struct vm_area_struct *vma)
 {
 	hugetlb_unshare_pmds(vma, ALIGN(vma->vm_start, PUD_SIZE),
-			ALIGN_DOWN(vma->vm_end, PUD_SIZE));
+			ALIGN_DOWN(vma->vm_end, PUD_SIZE),
+			/* take_locks = */ true);
 }
 
 /*
diff --git a/mm/vma.c b/mm/vma.c
index 1c6595f282e5..7ebc9eb608f4 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -539,7 +539,14 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
 	init_vma_prep(&vp, vma);
 	vp.insert = new;
 	vma_prepare(&vp);
+
+	/*
+	 * Get rid of huge pages and shared page tables straddling the split
+	 * boundary.
+	 */
 	vma_adjust_trans_huge(vma, vma->vm_start, addr, NULL);
+	if (is_vm_hugetlb_page(vma))
+		hugetlb_split(vma, addr);
 
 	if (new_below) {
 		vma->vm_start = addr;
diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
index 441feb21aa5a..4505b1c31be1 100644
--- a/tools/testing/vma/vma_internal.h
+++ b/tools/testing/vma/vma_internal.h
@@ -932,6 +932,8 @@ static inline void vma_adjust_trans_huge(struct vm_area_struct *vma,
 	(void)next;
 }
 
+static inline void hugetlb_split(struct vm_area_struct *, unsigned long) {}
+
 static inline void vma_iter_free(struct vma_iterator *vmi)
 {
 	mas_destroy(&vmi->mas);


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before
  2025-06-20 10:36 FAILED: patch "[PATCH] mm/hugetlb: unshare page tables during VMA split, not before" failed to apply to 6.1-stable tree gregkh
@ 2025-06-20 21:33 ` Jann Horn
  2025-06-20 21:33   ` [PATCH 6.1.y 2/3] mm: hugetlb: independent PMD page table shared count Jann Horn
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Jann Horn @ 2025-06-20 21:33 UTC (permalink / raw)
  To: stable

Currently, __split_vma() triggers hugetlb page table unsharing through
vm_ops->may_split().  This happens before the VMA lock and rmap locks are
taken - which is too early, it allows racing VMA-locked page faults in our
process and racing rmap walks from other processes to cause page tables to
be shared again before we actually perform the split.

Fix it by explicitly calling into the hugetlb unshare logic from
__split_vma() in the same place where THP splitting also happens.  At that
point, both the VMA and the rmap(s) are write-locked.

An annoying detail is that we can now call into the helper
hugetlb_unshare_pmds() from two different locking contexts:

1. from hugetlb_split(), holding:
    - mmap lock (exclusively)
    - VMA lock
    - file rmap lock (exclusively)
2. hugetlb_unshare_all_pmds(), which I think is designed to be able to
   call us with only the mmap lock held (in shared mode), but currently
   only runs while holding mmap lock (exclusively) and VMA lock

Backporting note:
This commit fixes a racy protection that was introduced in commit
b30c14cd6102 ("hugetlb: unshare some PMDs when splitting VMAs"); that
commit claimed to fix an issue introduced in 5.13, but it should actually
also go all the way back.

[jannh@google.com: v2]
  Link: https://lkml.kernel.org/r/20250528-hugetlb-fixes-splitrace-v2-1-1329349bad1a@google.com
Link: https://lkml.kernel.org/r/20250528-hugetlb-fixes-splitrace-v2-0-1329349bad1a@google.com
Link: https://lkml.kernel.org/r/20250527-hugetlb-fixes-splitrace-v1-1-f4136f5ec58a@google.com
Fixes: 39dde65c9940 ("[PATCH] shared page table for hugetlb page")
Signed-off-by: Jann Horn <jannh@google.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: <stable@vger.kernel.org>	[b30c14cd6102: hugetlb: unshare some PMDs when splitting VMAs]
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
[stable backport: code got moved around, VMA splitting is in
__vma_adjust]
Signed-off-by: Jann Horn <jannh@google.com>
---
 include/linux/hugetlb.h |  3 +++
 mm/hugetlb.c            | 60 ++++++++++++++++++++++++++++++-----------
 mm/mmap.c               |  8 ++++++
 3 files changed, 55 insertions(+), 16 deletions(-)

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index cc555072940f..26f2947c399d 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -239,6 +239,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
 
 bool is_hugetlb_entry_migration(pte_t pte);
 void hugetlb_unshare_all_pmds(struct vm_area_struct *vma);
+void hugetlb_split(struct vm_area_struct *vma, unsigned long addr);
 
 #else /* !CONFIG_HUGETLB_PAGE */
 
@@ -472,6 +473,8 @@ static inline vm_fault_t hugetlb_fault(struct mm_struct *mm,
 
 static inline void hugetlb_unshare_all_pmds(struct vm_area_struct *vma) { }
 
+static inline void hugetlb_split(struct vm_area_struct *vma, unsigned long addr) {}
+
 #endif /* !CONFIG_HUGETLB_PAGE */
 /*
  * hugepages at page global directory. If arch support
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 14b9494c58ed..fc5d3d665266 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -95,7 +95,7 @@ static void hugetlb_vma_lock_free(struct vm_area_struct *vma);
 static void hugetlb_vma_lock_alloc(struct vm_area_struct *vma);
 static void __hugetlb_vma_unlock_write_free(struct vm_area_struct *vma);
 static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
-		unsigned long start, unsigned long end);
+		unsigned long start, unsigned long end, bool take_locks);
 static struct resv_map *vma_resv_map(struct vm_area_struct *vma);
 
 static inline bool subpool_is_free(struct hugepage_subpool *spool)
@@ -4900,26 +4900,40 @@ static int hugetlb_vm_op_split(struct vm_area_struct *vma, unsigned long addr)
 {
 	if (addr & ~(huge_page_mask(hstate_vma(vma))))
 		return -EINVAL;
+	return 0;
+}
 
+void hugetlb_split(struct vm_area_struct *vma, unsigned long addr)
+{
 	/*
 	 * PMD sharing is only possible for PUD_SIZE-aligned address ranges
 	 * in HugeTLB VMAs. If we will lose PUD_SIZE alignment due to this
 	 * split, unshare PMDs in the PUD_SIZE interval surrounding addr now.
+	 * This function is called in the middle of a VMA split operation, with
+	 * MM, VMA and rmap all write-locked to prevent concurrent page table
+	 * walks (except hardware and gup_fast()).
 	 */
+	mmap_assert_write_locked(vma->vm_mm);
+	i_mmap_assert_write_locked(vma->vm_file->f_mapping);
+
 	if (addr & ~PUD_MASK) {
-		/*
-		 * hugetlb_vm_op_split is called right before we attempt to
-		 * split the VMA. We will need to unshare PMDs in the old and
-		 * new VMAs, so let's unshare before we split.
-		 */
 		unsigned long floor = addr & PUD_MASK;
 		unsigned long ceil = floor + PUD_SIZE;
 
-		if (floor >= vma->vm_start && ceil <= vma->vm_end)
-			hugetlb_unshare_pmds(vma, floor, ceil);
+		if (floor >= vma->vm_start && ceil <= vma->vm_end) {
+			/*
+			 * Locking:
+			 * Use take_locks=false here.
+			 * The file rmap lock is already held.
+			 * The hugetlb VMA lock can't be taken when we already
+			 * hold the file rmap lock, and we don't need it because
+			 * its purpose is to synchronize against concurrent page
+			 * table walks, which are not possible thanks to the
+			 * locks held by our caller.
+			 */
+			hugetlb_unshare_pmds(vma, floor, ceil, /* take_locks = */ false);
+		}
 	}
-
-	return 0;
 }
 
 static unsigned long hugetlb_vm_op_pagesize(struct vm_area_struct *vma)
@@ -7495,9 +7509,16 @@ void move_hugetlb_state(struct page *oldpage, struct page *newpage, int reason)
 	}
 }
 
+/*
+ * If @take_locks is false, the caller must ensure that no concurrent page table
+ * access can happen (except for gup_fast() and hardware page walks).
+ * If @take_locks is true, we take the hugetlb VMA lock (to lock out things like
+ * concurrent page fault handling) and the file rmap lock.
+ */
 static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
 				   unsigned long start,
-				   unsigned long end)
+				   unsigned long end,
+				   bool take_locks)
 {
 	struct hstate *h = hstate_vma(vma);
 	unsigned long sz = huge_page_size(h);
@@ -7521,8 +7542,12 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
 	mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, mm,
 				start, end);
 	mmu_notifier_invalidate_range_start(&range);
-	hugetlb_vma_lock_write(vma);
-	i_mmap_lock_write(vma->vm_file->f_mapping);
+	if (take_locks) {
+		hugetlb_vma_lock_write(vma);
+		i_mmap_lock_write(vma->vm_file->f_mapping);
+	} else {
+		i_mmap_assert_write_locked(vma->vm_file->f_mapping);
+	}
 	for (address = start; address < end; address += PUD_SIZE) {
 		ptep = huge_pte_offset(mm, address, sz);
 		if (!ptep)
@@ -7532,8 +7557,10 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
 		spin_unlock(ptl);
 	}
 	flush_hugetlb_tlb_range(vma, start, end);
-	i_mmap_unlock_write(vma->vm_file->f_mapping);
-	hugetlb_vma_unlock_write(vma);
+	if (take_locks) {
+		i_mmap_unlock_write(vma->vm_file->f_mapping);
+		hugetlb_vma_unlock_write(vma);
+	}
 	/*
 	 * No need to call mmu_notifier_invalidate_range(), see
 	 * Documentation/mm/mmu_notifier.rst.
@@ -7548,7 +7575,8 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
 void hugetlb_unshare_all_pmds(struct vm_area_struct *vma)
 {
 	hugetlb_unshare_pmds(vma, ALIGN(vma->vm_start, PUD_SIZE),
-			ALIGN_DOWN(vma->vm_end, PUD_SIZE));
+			ALIGN_DOWN(vma->vm_end, PUD_SIZE),
+			/* take_locks = */ true);
 }
 
 #ifdef CONFIG_CMA
diff --git a/mm/mmap.c b/mm/mmap.c
index ebc3583fa612..0f303dc8425a 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -727,7 +727,15 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 		return -ENOMEM;
 	}
 
+	/*
+	 * Get rid of huge pages and shared page tables straddling the split
+	 * boundary.
+	 */
 	vma_adjust_trans_huge(orig_vma, start, end, adjust_next);
+	if (is_vm_hugetlb_page(orig_vma)) {
+		hugetlb_split(orig_vma, start);
+		hugetlb_split(orig_vma, end);
+	}
 	if (file) {
 		mapping = file->f_mapping;
 		root = &mapping->i_mmap;
-- 
2.50.0.rc2.701.gf1e915cc24-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6.1.y 2/3] mm: hugetlb: independent PMD page table shared count
  2025-06-20 21:33 ` [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before Jann Horn
@ 2025-06-20 21:33   ` Jann Horn
  2025-06-29 13:00     ` Vitaly Chikunov
  2025-06-20 21:33   ` [PATCH 6.1.y 3/3] mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race Jann Horn
  2025-07-07 10:39   ` [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before Fedor Pchelkin
  2 siblings, 1 reply; 9+ messages in thread
From: Jann Horn @ 2025-06-20 21:33 UTC (permalink / raw)
  To: stable

From: Liu Shixin <liushixin2@huawei.com>

[ Upstream commit 59d9094df3d79443937add8700b2ef1a866b1081 ]

The folio refcount may be increased unexpectly through try_get_folio() by
caller such as split_huge_pages.  In huge_pmd_unshare(), we use refcount
to check whether a pmd page table is shared.  The check is incorrect if
the refcount is increased by the above caller, and this can cause the page
table leaked:

 BUG: Bad page state in process sh  pfn:109324
 page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x66 pfn:0x109324
 flags: 0x17ffff800000000(node=0|zone=2|lastcpupid=0xfffff)
 page_type: f2(table)
 raw: 017ffff800000000 0000000000000000 0000000000000000 0000000000000000
 raw: 0000000000000066 0000000000000000 00000000f2000000 0000000000000000
 page dumped because: nonzero mapcount
 ...
 CPU: 31 UID: 0 PID: 7515 Comm: sh Kdump: loaded Tainted: G    B              6.13.0-rc2master+ #7
 Tainted: [B]=BAD_PAGE
 Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
 Call trace:
  show_stack+0x20/0x38 (C)
  dump_stack_lvl+0x80/0xf8
  dump_stack+0x18/0x28
  bad_page+0x8c/0x130
  free_page_is_bad_report+0xa4/0xb0
  free_unref_page+0x3cc/0x620
  __folio_put+0xf4/0x158
  split_huge_pages_all+0x1e0/0x3e8
  split_huge_pages_write+0x25c/0x2d8
  full_proxy_write+0x64/0xd8
  vfs_write+0xcc/0x280
  ksys_write+0x70/0x110
  __arm64_sys_write+0x24/0x38
  invoke_syscall+0x50/0x120
  el0_svc_common.constprop.0+0xc8/0xf0
  do_el0_svc+0x24/0x38
  el0_svc+0x34/0x128
  el0t_64_sync_handler+0xc8/0xd0
  el0t_64_sync+0x190/0x198

The issue may be triggered by damon, offline_page, page_idle, etc, which
will increase the refcount of page table.

1. The page table itself will be discarded after reporting the
   "nonzero mapcount".

2. The HugeTLB page mapped by the page table miss freeing since we
   treat the page table as shared and a shared page table will not be
   unmapped.

Fix it by introducing independent PMD page table shared count.  As
described by comment, pt_index/pt_mm/pt_frag_refcount are used for s390
gmap, x86 pgds and powerpc, pt_share_count is used for x86/arm64/riscv
pmds, so we can reuse the field as pt_share_count.

Link: https://lkml.kernel.org/r/20241216071147.3984217-1-liushixin2@huawei.com
Fixes: 39dde65c9940 ("[PATCH] shared page table for hugetlb page")
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: Ken Chen <kenneth.w.chen@intel.com>
Cc: Muchun Song <muchun.song@linux.dev>
Cc: Nanyong Sun <sunnanyong@huawei.com>
Cc: Jane Chu <jane.chu@oracle.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[backport note: struct ptdesc did not exist yet, stuff it equivalently
into struct page instead]
Signed-off-by: Jann Horn <jannh@google.com>
---
 include/linux/mm.h       |  3 +++
 include/linux/mm_types.h |  3 +++
 mm/hugetlb.c             | 16 +++++++---------
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 03357c196e0b..b36dffbfbe69 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2537,6 +2537,9 @@ static inline bool pgtable_pmd_page_ctor(struct page *page)
 	if (!pmd_ptlock_init(page))
 		return false;
 	__SetPageTable(page);
+#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
+	atomic_set(&page->pt_share_count, 0);
+#endif
 	inc_lruvec_page_state(page, NR_PAGETABLE);
 	return true;
 }
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index a9c1d611029d..9b64610eddcc 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -160,6 +160,9 @@ struct page {
 			union {
 				struct mm_struct *pt_mm; /* x86 pgds only */
 				atomic_t pt_frag_refcount; /* powerpc */
+#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
+				atomic_t pt_share_count;
+#endif
 			};
 #if ALLOC_SPLIT_PTLOCKS
 			spinlock_t *ptl;
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index fc5d3d665266..a3907edf2909 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -7114,7 +7114,7 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
 			spte = huge_pte_offset(svma->vm_mm, saddr,
 					       vma_mmu_pagesize(svma));
 			if (spte) {
-				get_page(virt_to_page(spte));
+				atomic_inc(&virt_to_page(spte)->pt_share_count);
 				break;
 			}
 		}
@@ -7129,7 +7129,7 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
 		mm_inc_nr_pmds(mm);
 	} else {
-		put_page(virt_to_page(spte));
+		atomic_dec(&virt_to_page(spte)->pt_share_count);
 	}
 	spin_unlock(ptl);
 out:
@@ -7141,10 +7141,6 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
 /*
  * unmap huge page backed by shared pte.
  *
- * Hugetlb pte page is ref counted at the time of mapping.  If pte is shared
- * indicated by page_count > 1, unmap is achieved by clearing pud and
- * decrementing the ref count. If count == 1, the pte page is not shared.
- *
  * Called with page table lock held.
  *
  * returns: 1 successfully unmapped a shared pte page
@@ -7153,18 +7149,20 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
 int huge_pmd_unshare(struct mm_struct *mm, struct vm_area_struct *vma,
 					unsigned long addr, pte_t *ptep)
 {
+	unsigned long sz = huge_page_size(hstate_vma(vma));
 	pgd_t *pgd = pgd_offset(mm, addr);
 	p4d_t *p4d = p4d_offset(pgd, addr);
 	pud_t *pud = pud_offset(p4d, addr);
 
 	i_mmap_assert_write_locked(vma->vm_file->f_mapping);
 	hugetlb_vma_assert_locked(vma);
-	BUG_ON(page_count(virt_to_page(ptep)) == 0);
-	if (page_count(virt_to_page(ptep)) == 1)
+	if (sz != PMD_SIZE)
+		return 0;
+	if (!atomic_read(&virt_to_page(ptep)->pt_share_count))
 		return 0;
 
 	pud_clear(pud);
-	put_page(virt_to_page(ptep));
+	atomic_dec(&virt_to_page(ptep)->pt_share_count);
 	mm_dec_nr_pmds(mm);
 	return 1;
 }
-- 
2.50.0.rc2.701.gf1e915cc24-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6.1.y 3/3] mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race
  2025-06-20 21:33 ` [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before Jann Horn
  2025-06-20 21:33   ` [PATCH 6.1.y 2/3] mm: hugetlb: independent PMD page table shared count Jann Horn
@ 2025-06-20 21:33   ` Jann Horn
  2025-07-07 10:39   ` [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before Fedor Pchelkin
  2 siblings, 0 replies; 9+ messages in thread
From: Jann Horn @ 2025-06-20 21:33 UTC (permalink / raw)
  To: stable

huge_pmd_unshare() drops a reference on a page table that may have
previously been shared across processes, potentially turning it into a
normal page table used in another process in which unrelated VMAs can
afterwards be installed.

If this happens in the middle of a concurrent gup_fast(), gup_fast() could
end up walking the page tables of another process.  While I don't see any
way in which that immediately leads to kernel memory corruption, it is
really weird and unexpected.

Fix it with an explicit broadcast IPI through tlb_remove_table_sync_one(),
just like we do in khugepaged when removing page tables for a THP
collapse.

Link: https://lkml.kernel.org/r/20250528-hugetlb-fixes-splitrace-v2-2-1329349bad1a@google.com
Link: https://lkml.kernel.org/r/20250527-hugetlb-fixes-splitrace-v1-2-f4136f5ec58a@google.com
Fixes: 39dde65c9940 ("[PATCH] shared page table for hugetlb page")
Signed-off-by: Jann Horn <jannh@google.com>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Cc: Muchun Song <muchun.song@linux.dev>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
[backport]
Signed-off-by: Jann Horn <jannh@google.com>
---
 mm/hugetlb.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index a3907edf2909..2dee7dcb3b18 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -7162,6 +7162,13 @@ int huge_pmd_unshare(struct mm_struct *mm, struct vm_area_struct *vma,
 		return 0;
 
 	pud_clear(pud);
+	/*
+	 * Once our caller drops the rmap lock, some other process might be
+	 * using this page table as a normal, non-hugetlb page table.
+	 * Wait for pending gup_fast() in other threads to finish before letting
+	 * that happen.
+	 */
+	tlb_remove_table_sync_one();
 	atomic_dec(&virt_to_page(ptep)->pt_share_count);
 	mm_dec_nr_pmds(mm);
 	return 1;
-- 
2.50.0.rc2.701.gf1e915cc24-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 6.1.y 2/3] mm: hugetlb: independent PMD page table shared count
  2025-06-20 21:33   ` [PATCH 6.1.y 2/3] mm: hugetlb: independent PMD page table shared count Jann Horn
@ 2025-06-29 13:00     ` Vitaly Chikunov
  2025-06-30 17:12       ` Jann Horn
  0 siblings, 1 reply; 9+ messages in thread
From: Vitaly Chikunov @ 2025-06-29 13:00 UTC (permalink / raw)
  To: Jann Horn, Sasha Levin, Andrew Morton, gregkh, stable
  Cc: Jane Chu, Nanyong Sun, Muchun Song, Ken Chen, Kefeng Wang,
	Liu Shixin, linux-mm

Hi,

LTP tests failure with the following commit described below:

On Fri, Jun 20, 2025 at 11:33:32PM +0200, Jann Horn wrote:
> From: Liu Shixin <liushixin2@huawei.com>
> 
> [ Upstream commit 59d9094df3d79443937add8700b2ef1a866b1081 ]
> 
> The folio refcount may be increased unexpectly through try_get_folio() by
> caller such as split_huge_pages.  In huge_pmd_unshare(), we use refcount
> to check whether a pmd page table is shared.  The check is incorrect if
> the refcount is increased by the above caller, and this can cause the page
> table leaked:
> 
>  BUG: Bad page state in process sh  pfn:109324
>  page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x66 pfn:0x109324
>  flags: 0x17ffff800000000(node=0|zone=2|lastcpupid=0xfffff)
>  page_type: f2(table)
>  raw: 017ffff800000000 0000000000000000 0000000000000000 0000000000000000
>  raw: 0000000000000066 0000000000000000 00000000f2000000 0000000000000000
>  page dumped because: nonzero mapcount
>  ...
>  CPU: 31 UID: 0 PID: 7515 Comm: sh Kdump: loaded Tainted: G    B              6.13.0-rc2master+ #7
>  Tainted: [B]=BAD_PAGE
>  Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
>  Call trace:
>   show_stack+0x20/0x38 (C)
>   dump_stack_lvl+0x80/0xf8
>   dump_stack+0x18/0x28
>   bad_page+0x8c/0x130
>   free_page_is_bad_report+0xa4/0xb0
>   free_unref_page+0x3cc/0x620
>   __folio_put+0xf4/0x158
>   split_huge_pages_all+0x1e0/0x3e8
>   split_huge_pages_write+0x25c/0x2d8
>   full_proxy_write+0x64/0xd8
>   vfs_write+0xcc/0x280
>   ksys_write+0x70/0x110
>   __arm64_sys_write+0x24/0x38
>   invoke_syscall+0x50/0x120
>   el0_svc_common.constprop.0+0xc8/0xf0
>   do_el0_svc+0x24/0x38
>   el0_svc+0x34/0x128
>   el0t_64_sync_handler+0xc8/0xd0
>   el0t_64_sync+0x190/0x198
> 
> The issue may be triggered by damon, offline_page, page_idle, etc, which
> will increase the refcount of page table.
> 
> 1. The page table itself will be discarded after reporting the
>    "nonzero mapcount".
> 
> 2. The HugeTLB page mapped by the page table miss freeing since we
>    treat the page table as shared and a shared page table will not be
>    unmapped.
> 
> Fix it by introducing independent PMD page table shared count.  As
> described by comment, pt_index/pt_mm/pt_frag_refcount are used for s390
> gmap, x86 pgds and powerpc, pt_share_count is used for x86/arm64/riscv
> pmds, so we can reuse the field as pt_share_count.

The commit causes LTP test memfd_create03 to fail on i586 architecture
on v6.1.142 stable release, the test was passing on v6.1.141. Found the
commit with git bisect.

The failure:

  root@i586:~# /usr/lib/ltp/testcases/bin/memfd_create03
  tst_hugepage.c:78: TINFO: 2 hugepage(s) reserved
  tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
  memfd_create03.c:171: TINFO: --TESTING WRITE CALL IN HUGEPAGES--
  memfd_create03.c:176: TINFO: memfd_create() succeeded
  memfd_create03.c:70: TPASS: write(3, "LTP", 3) failed as expected

  memfd_create03.c:171: TINFO: --TESTING PAGE SIZE OF CREATED FILE--
  memfd_create03.c:176: TINFO: memfd_create() succeeded
  memfd_create03.c:43: TINFO: mmap((nil), 4194304, 2, 2, 3, 0) succeeded
  memfd_create03.c:92: TINFO: munmap(0xb7800000, 1024kB) failed as expected
  memfd_create03.c:92: TINFO: munmap(0xb7800000, 2048kB) failed as expected
  memfd_create03.c:92: TINFO: munmap(0xb7800000, 3072kB) failed as expected
  memfd_create03.c:111: TPASS: munmap() fails for page sizes less than 4096kB

  memfd_create03.c:171: TINFO: --TESTING HUGEPAGE ALLOCATION LIMIT--
  memfd_create03.c:176: TINFO: memfd_create() succeeded
  memfd_create03.c:39: TBROK: mmap((nil),0,2,2,3,0) failed: EINVAL (22)

  Summary:
  passed   2
  failed   0
  broken   1
  skipped  0
  warnings 0

dmesg while the test run:

  [   16.072078] memfd_create03 (203): drop_caches: 3
  [   16.075298] mm/pgtable-generic.c:51: bad pgd 7d4000e7

The same error occurs for v5.10.239. There is no test failure on v6.12.35 nor
v6.15.4 even though they contain the same commit.

Thanks,

> 
> Link: https://lkml.kernel.org/r/20241216071147.3984217-1-liushixin2@huawei.com
> Fixes: 39dde65c9940 ("[PATCH] shared page table for hugetlb page")
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
> Cc: Ken Chen <kenneth.w.chen@intel.com>
> Cc: Muchun Song <muchun.song@linux.dev>
> Cc: Nanyong Sun <sunnanyong@huawei.com>
> Cc: Jane Chu <jane.chu@oracle.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> [backport note: struct ptdesc did not exist yet, stuff it equivalently
> into struct page instead]
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
>  include/linux/mm.h       |  3 +++
>  include/linux/mm_types.h |  3 +++
>  mm/hugetlb.c             | 16 +++++++---------
>  3 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 03357c196e0b..b36dffbfbe69 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2537,6 +2537,9 @@ static inline bool pgtable_pmd_page_ctor(struct page *page)
>  	if (!pmd_ptlock_init(page))
>  		return false;
>  	__SetPageTable(page);
> +#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
> +	atomic_set(&page->pt_share_count, 0);
> +#endif
>  	inc_lruvec_page_state(page, NR_PAGETABLE);
>  	return true;
>  }
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index a9c1d611029d..9b64610eddcc 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -160,6 +160,9 @@ struct page {
>  			union {
>  				struct mm_struct *pt_mm; /* x86 pgds only */
>  				atomic_t pt_frag_refcount; /* powerpc */
> +#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
> +				atomic_t pt_share_count;
> +#endif
>  			};
>  #if ALLOC_SPLIT_PTLOCKS
>  			spinlock_t *ptl;
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index fc5d3d665266..a3907edf2909 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -7114,7 +7114,7 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
>  			spte = huge_pte_offset(svma->vm_mm, saddr,
>  					       vma_mmu_pagesize(svma));
>  			if (spte) {
> -				get_page(virt_to_page(spte));
> +				atomic_inc(&virt_to_page(spte)->pt_share_count);
>  				break;
>  			}
>  		}
> @@ -7129,7 +7129,7 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
>  				(pmd_t *)((unsigned long)spte & PAGE_MASK));
>  		mm_inc_nr_pmds(mm);
>  	} else {
> -		put_page(virt_to_page(spte));
> +		atomic_dec(&virt_to_page(spte)->pt_share_count);
>  	}
>  	spin_unlock(ptl);
>  out:
> @@ -7141,10 +7141,6 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
>  /*
>   * unmap huge page backed by shared pte.
>   *
> - * Hugetlb pte page is ref counted at the time of mapping.  If pte is shared
> - * indicated by page_count > 1, unmap is achieved by clearing pud and
> - * decrementing the ref count. If count == 1, the pte page is not shared.
> - *
>   * Called with page table lock held.
>   *
>   * returns: 1 successfully unmapped a shared pte page
> @@ -7153,18 +7149,20 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
>  int huge_pmd_unshare(struct mm_struct *mm, struct vm_area_struct *vma,
>  					unsigned long addr, pte_t *ptep)
>  {
> +	unsigned long sz = huge_page_size(hstate_vma(vma));
>  	pgd_t *pgd = pgd_offset(mm, addr);
>  	p4d_t *p4d = p4d_offset(pgd, addr);
>  	pud_t *pud = pud_offset(p4d, addr);
>  
>  	i_mmap_assert_write_locked(vma->vm_file->f_mapping);
>  	hugetlb_vma_assert_locked(vma);
> -	BUG_ON(page_count(virt_to_page(ptep)) == 0);
> -	if (page_count(virt_to_page(ptep)) == 1)
> +	if (sz != PMD_SIZE)
> +		return 0;
> +	if (!atomic_read(&virt_to_page(ptep)->pt_share_count))
>  		return 0;
>  
>  	pud_clear(pud);
> -	put_page(virt_to_page(ptep));
> +	atomic_dec(&virt_to_page(ptep)->pt_share_count);
>  	mm_dec_nr_pmds(mm);
>  	return 1;
>  }
> -- 
> 2.50.0.rc2.701.gf1e915cc24-goog
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 6.1.y 2/3] mm: hugetlb: independent PMD page table shared count
  2025-06-29 13:00     ` Vitaly Chikunov
@ 2025-06-30 17:12       ` Jann Horn
  2025-06-30 19:17         ` Jann Horn
  0 siblings, 1 reply; 9+ messages in thread
From: Jann Horn @ 2025-06-30 17:12 UTC (permalink / raw)
  To: Vitaly Chikunov, Muchun Song, Oscar Salvador, Dave Hansen,
	Andy Lutomirski, Peter Zijlstra
  Cc: Sasha Levin, Andrew Morton, gregkh, stable, Jane Chu, Nanyong Sun,
	Ken Chen, Kefeng Wang, Liu Shixin, linux-mm

tl;dr: 32-bit x86 without PAE opts into hugetlb page table sharing
despite only having 2-level paging, which means the "sharable" page
tables are PGDs, and then stuff breaks

On Sun, Jun 29, 2025 at 3:00 PM Vitaly Chikunov <vt@altlinux.org> wrote:
> LTP tests failure with the following commit described below:

Uuugh... thanks for letting me know.

> On Fri, Jun 20, 2025 at 11:33:32PM +0200, Jann Horn wrote:
> > From: Liu Shixin <liushixin2@huawei.com>
> >
> > [ Upstream commit 59d9094df3d79443937add8700b2ef1a866b1081 ]
> >
> > The folio refcount may be increased unexpectly through try_get_folio() by
> > caller such as split_huge_pages.  In huge_pmd_unshare(), we use refcount
> > to check whether a pmd page table is shared.  The check is incorrect if
> > the refcount is increased by the above caller, and this can cause the page
> > table leaked:
[...]
> The commit causes LTP test memfd_create03 to fail on i586 architecture
> on v6.1.142 stable release, the test was passing on v6.1.141. Found the
> commit with git bisect.

Ah, yes, I can reproduce this; specifically it reproduces on a 32-bit
X86 builds without X86_PAE. If I enable X86_PAE, the tests pass.

Okay, I don't know precisely why this is breaking, but at a high
level: x86 unconditionally selects ARCH_WANT_HUGE_PMD_SHARE (and still
does in mainline). That flag means "when we have PMD entries pointing
to hugetlb pages, we want to share the PMD table across processes".

32-bit X86 with PAE has 3 page table levels (pgd, pmd, pte); so with
this sharing mechanism, we'd have multiple PGD entries pointing to the
same PMD. I guess that seems fine.

But 32-bit X86 with PAE only has 2 page table levels (pgd, pte). So a
hugepage is referenced by a PGD entry, and it makes no sense to try to
share PGDs. PGDs not being shared page tables is also baked into
(looking at the mainline version) "struct ptdesc", which puts "struct
mm_struct *pt_mm;" (for x86 PGDs) and "atomic_t pt_share_count;" (for
hugetlb page table sharing) into the same union.

I guess I'll send a patch later to disable page table sharing in
non-PAE 32-bit x86... or maybe we should disable it entirely for
32-bit x86...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 6.1.y 2/3] mm: hugetlb: independent PMD page table shared count
  2025-06-30 17:12       ` Jann Horn
@ 2025-06-30 19:17         ` Jann Horn
  0 siblings, 0 replies; 9+ messages in thread
From: Jann Horn @ 2025-06-30 19:17 UTC (permalink / raw)
  To: Vitaly Chikunov, Muchun Song, Oscar Salvador, Dave Hansen,
	Andy Lutomirski, Peter Zijlstra, gregkh
  Cc: Sasha Levin, Andrew Morton, stable, Jane Chu, Nanyong Sun,
	Kefeng Wang, Liu Shixin, linux-mm

On Mon, Jun 30, 2025 at 7:12 PM Jann Horn <jannh@google.com> wrote:
> tl;dr: 32-bit x86 without PAE opts into hugetlb page table sharing
> despite only having 2-level paging, which means the "sharable" page
> tables are PGDs, and then stuff breaks
>
> On Sun, Jun 29, 2025 at 3:00 PM Vitaly Chikunov <vt@altlinux.org> wrote:
> > LTP tests failure with the following commit described below:
>
> Uuugh... thanks for letting me know.
>
> > On Fri, Jun 20, 2025 at 11:33:32PM +0200, Jann Horn wrote:
> > > From: Liu Shixin <liushixin2@huawei.com>
> > >
> > > [ Upstream commit 59d9094df3d79443937add8700b2ef1a866b1081 ]
> > >
> > > The folio refcount may be increased unexpectly through try_get_folio() by
> > > caller such as split_huge_pages.  In huge_pmd_unshare(), we use refcount
> > > to check whether a pmd page table is shared.  The check is incorrect if
> > > the refcount is increased by the above caller, and this can cause the page
> > > table leaked:
> [...]
> > The commit causes LTP test memfd_create03 to fail on i586 architecture
> > on v6.1.142 stable release, the test was passing on v6.1.141. Found the
> > commit with git bisect.
>
> Ah, yes, I can reproduce this; specifically it reproduces on a 32-bit
> X86 builds without X86_PAE. If I enable X86_PAE, the tests pass.
[...]
> I guess I'll send a patch later to disable page table sharing in
> non-PAE 32-bit x86... or maybe we should disable it entirely for
> 32-bit x86...

This follow-up patch should address that:
<https://lore.kernel.org/r/20250630-x86-2level-hugetlb-v1-1-077cd53d8255@google.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before
  2025-06-20 21:33 ` [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before Jann Horn
  2025-06-20 21:33   ` [PATCH 6.1.y 2/3] mm: hugetlb: independent PMD page table shared count Jann Horn
  2025-06-20 21:33   ` [PATCH 6.1.y 3/3] mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race Jann Horn
@ 2025-07-07 10:39   ` Fedor Pchelkin
  2025-07-07 13:19     ` Jann Horn
  2 siblings, 1 reply; 9+ messages in thread
From: Fedor Pchelkin @ 2025-07-07 10:39 UTC (permalink / raw)
  To: Jann Horn; +Cc: stable, lvc-project

Hello Jann,

On Fri, 20 Jun 2025 23:33:31 +0200, Jann Horn wrote:
> Currently, __split_vma() triggers hugetlb page table unsharing through
> vm_ops->may_split().  This happens before the VMA lock and rmap locks are
> taken - which is too early, it allows racing VMA-locked page faults in our
> process and racing rmap walks from other processes to cause page tables to
> be shared again before we actually perform the split.
> 
> Fix it by explicitly calling into the hugetlb unshare logic from
> __split_vma() in the same place where THP splitting also happens.  At that
> point, both the VMA and the rmap(s) are write-locked.
> 
> An annoying detail is that we can now call into the helper
> hugetlb_unshare_pmds() from two different locking contexts:
> 
> 1. from hugetlb_split(), holding:
>     - mmap lock (exclusively)
>     - VMA lock
>     - file rmap lock (exclusively)
> 2. hugetlb_unshare_all_pmds(), which I think is designed to be able to
>    call us with only the mmap lock held (in shared mode), but currently
>    only runs while holding mmap lock (exclusively) and VMA lock
> 
> Backporting note:
> This commit fixes a racy protection that was introduced in commit
> b30c14cd6102 ("hugetlb: unshare some PMDs when splitting VMAs"); that
> commit claimed to fix an issue introduced in 5.13, but it should actually
> also go all the way back.
> 
> [jannh@google.com: v2]
>   Link: https://lkml.kernel.org/r/20250528-hugetlb-fixes-splitrace-v2-1-1329349bad1a@google.com
> Link: https://lkml.kernel.org/r/20250528-hugetlb-fixes-splitrace-v2-0-1329349bad1a@google.com
> Link: https://lkml.kernel.org/r/20250527-hugetlb-fixes-splitrace-v1-1-f4136f5ec58a@google.com
> Fixes: 39dde65c9940 ("[PATCH] shared page table for hugetlb page")
> Signed-off-by: Jann Horn <jannh@google.com>
> Cc: Liam Howlett <liam.howlett@oracle.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Reviewed-by: Oscar Salvador <osalvador@suse.de>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: <stable@vger.kernel.org>	[b30c14cd6102: hugetlb: unshare some PMDs when splitting VMAs]
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> [stable backport: code got moved around, VMA splitting is in
> __vma_adjust]
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
>  include/linux/hugetlb.h |  3 +++
>  mm/hugetlb.c            | 60 ++++++++++++++++++++++++++++++-----------
>  mm/mmap.c               |  8 ++++++
>  3 files changed, 55 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index cc555072940f..26f2947c399d 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -239,6 +239,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
>  
>  bool is_hugetlb_entry_migration(pte_t pte);
>  void hugetlb_unshare_all_pmds(struct vm_area_struct *vma);
> +void hugetlb_split(struct vm_area_struct *vma, unsigned long addr);
>  
>  #else /* !CONFIG_HUGETLB_PAGE */
>  
> @@ -472,6 +473,8 @@ static inline vm_fault_t hugetlb_fault(struct mm_struct *mm,
>  
>  static inline void hugetlb_unshare_all_pmds(struct vm_area_struct *vma) { }
>  
> +static inline void hugetlb_split(struct vm_area_struct *vma, unsigned long addr) {}
> +
>  #endif /* !CONFIG_HUGETLB_PAGE */
>  /*
>   * hugepages at page global directory. If arch support
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 14b9494c58ed..fc5d3d665266 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -95,7 +95,7 @@ static void hugetlb_vma_lock_free(struct vm_area_struct *vma);
>  static void hugetlb_vma_lock_alloc(struct vm_area_struct *vma);
>  static void __hugetlb_vma_unlock_write_free(struct vm_area_struct *vma);
>  static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
> -		unsigned long start, unsigned long end);
> +		unsigned long start, unsigned long end, bool take_locks);
>  static struct resv_map *vma_resv_map(struct vm_area_struct *vma);
>  
>  static inline bool subpool_is_free(struct hugepage_subpool *spool)
> @@ -4900,26 +4900,40 @@ static int hugetlb_vm_op_split(struct vm_area_struct *vma, unsigned long addr)
>  {
>  	if (addr & ~(huge_page_mask(hstate_vma(vma))))
>  		return -EINVAL;
> +	return 0;
> +}
>  
> +void hugetlb_split(struct vm_area_struct *vma, unsigned long addr)
> +{
>  	/*
>  	 * PMD sharing is only possible for PUD_SIZE-aligned address ranges
>  	 * in HugeTLB VMAs. If we will lose PUD_SIZE alignment due to this
>  	 * split, unshare PMDs in the PUD_SIZE interval surrounding addr now.
> +	 * This function is called in the middle of a VMA split operation, with
> +	 * MM, VMA and rmap all write-locked to prevent concurrent page table
> +	 * walks (except hardware and gup_fast()).
>  	 */
> +	mmap_assert_write_locked(vma->vm_mm);
> +	i_mmap_assert_write_locked(vma->vm_file->f_mapping);


The above i_mmap lock assertion is firing on stable kernels from 5.10 to 6.1
included.

------------[ cut here ]------------
WARNING: CPU: 0 PID: 11489 at include/linux/fs.h:503 i_mmap_assert_write_locked include/linux/fs.h:503 [inline]
WARNING: CPU: 0 PID: 11489 at include/linux/fs.h:503 hugetlb_split+0x267/0x300 mm/hugetlb.c:4917
Modules linked in:
CPU: 0 PID: 11489 Comm: syz-executor.4 Not tainted 6.1.142-syzkaller-00296-gfd0df5221577 #0
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
RIP: 0010:i_mmap_assert_write_locked include/linux/fs.h:503 [inline]
RIP: 0010:hugetlb_split+0x267/0x300 mm/hugetlb.c:4917
Call Trace:
 <TASK>
 __vma_adjust+0xd73/0x1c10 mm/mmap.c:736
 vma_adjust include/linux/mm.h:2745 [inline]
 __split_vma+0x459/0x540 mm/mmap.c:2385
 do_mas_align_munmap+0x5f2/0xf10 mm/mmap.c:2497
 do_mas_munmap+0x26c/0x2c0 mm/mmap.c:2646
 __mmap_region mm/mmap.c:2694 [inline]
 mmap_region+0x19f/0x1770 mm/mmap.c:2912
 do_mmap+0x84b/0xf20 mm/mmap.c:1432
 vm_mmap_pgoff+0x1af/0x280 mm/util.c:520
 ksys_mmap_pgoff+0x41f/0x5a0 mm/mmap.c:1478
 do_syscall_x64 arch/x86/entry/common.c:51 [inline]
 do_syscall_64+0x35/0x80 arch/x86/entry/common.c:81
 entry_SYSCALL_64_after_hwframe+0x6e/0xd8
RIP: 0033:0x46a269
 </TASK>

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.


The main reason is that those branches lack the following

  commit ccf1d78d8b86e28502fa1b575a459a402177def4
  Author: Suren Baghdasaryan <surenb@google.com>
  Date:   Mon Feb 27 09:36:13 2023 -0800
  
      mm/mmap: move vma_prepare before vma_adjust_trans_huge
      
      vma_prepare() acquires all locks required before VMA modifications.  Move
      vma_prepare() before vma_adjust_trans_huge() so that VMA is locked before
      any modification.
      
      Link: https://lkml.kernel.org/r/20230227173632.3292573-15-surenb@google.com
      Signed-off-by: Suren Baghdasaryan <surenb@google.com>
      Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

thus the needed lock is acquired just after vma_adjust_trans_huge() and
the newly added hugetlb_split().

Please have a look at a straightforward write-up which comes to my mind.
It does something like the ccf1d78d8b86 ("mm/mmap: move vma_prepare before
vma_adjust_trans_huge"), but in context of an old stable branch.

If looks okay, I'll be glad to prepare it as a formal patch and send it
out for the 5.10-5.15, too.

against 6.1.y
-------------
diff --git a/mm/mmap.c b/mm/mmap.c
index 0f303dc8425a..941880ed62d7 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -543,8 +543,6 @@ inline int vma_expand(struct ma_state *mas, struct vm_area_struct *vma,
        if (mas_preallocate(mas, vma, GFP_KERNEL))
                goto nomem;
 
-       vma_adjust_trans_huge(vma, start, end, 0);
-
        if (file) {
                mapping = file->f_mapping;
                root = &mapping->i_mmap;
@@ -562,6 +560,8 @@ inline int vma_expand(struct ma_state *mas, struct vm_area_struct *vma,
                vma_interval_tree_remove(vma, root);
        }
 
+       vma_adjust_trans_huge(vma, start, end, 0);
+
        vma->vm_start = start;
        vma->vm_end = end;
        vma->vm_pgoff = pgoff;
@@ -727,15 +727,6 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
                return -ENOMEM;
        }
 
-       /*
-        * Get rid of huge pages and shared page tables straddling the split
-        * boundary.
-        */
-       vma_adjust_trans_huge(orig_vma, start, end, adjust_next);
-       if (is_vm_hugetlb_page(orig_vma)) {
-               hugetlb_split(orig_vma, start);
-               hugetlb_split(orig_vma, end);
-       }
        if (file) {
                mapping = file->f_mapping;
                root = &mapping->i_mmap;
@@ -775,6 +766,16 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
                        vma_interval_tree_remove(next, root);
        }
 
+       /*
+        * Get rid of huge pages and shared page tables straddling the split
+        * boundary.
+        */
+       vma_adjust_trans_huge(orig_vma, start, end, adjust_next);
+       if (is_vm_hugetlb_page(orig_vma)) {
+               hugetlb_split(orig_vma, start);
+               hugetlb_split(orig_vma, end);
+       }
+
        if (start != vma->vm_start) {
                if ((vma->vm_start < start) &&
                    (!insert || (insert->vm_end != start))) {


--
Fedor

> +
>  	if (addr & ~PUD_MASK) {
> -		/*
> -		 * hugetlb_vm_op_split is called right before we attempt to
> -		 * split the VMA. We will need to unshare PMDs in the old and
> -		 * new VMAs, so let's unshare before we split.
> -		 */
>  		unsigned long floor = addr & PUD_MASK;
>  		unsigned long ceil = floor + PUD_SIZE;
>  
> -		if (floor >= vma->vm_start && ceil <= vma->vm_end)
> -			hugetlb_unshare_pmds(vma, floor, ceil);
> +		if (floor >= vma->vm_start && ceil <= vma->vm_end) {
> +			/*
> +			 * Locking:
> +			 * Use take_locks=false here.
> +			 * The file rmap lock is already held.
> +			 * The hugetlb VMA lock can't be taken when we already
> +			 * hold the file rmap lock, and we don't need it because
> +			 * its purpose is to synchronize against concurrent page
> +			 * table walks, which are not possible thanks to the
> +			 * locks held by our caller.
> +			 */
> +			hugetlb_unshare_pmds(vma, floor, ceil, /* take_locks = */ false);
> +		}
>  	}
> -
> -	return 0;
>  }
>  
>  static unsigned long hugetlb_vm_op_pagesize(struct vm_area_struct *vma)
> @@ -7495,9 +7509,16 @@ void move_hugetlb_state(struct page *oldpage, struct page *newpage, int reason)
>  	}
>  }
>  
> +/*
> + * If @take_locks is false, the caller must ensure that no concurrent page table
> + * access can happen (except for gup_fast() and hardware page walks).
> + * If @take_locks is true, we take the hugetlb VMA lock (to lock out things like
> + * concurrent page fault handling) and the file rmap lock.
> + */
>  static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
>  				   unsigned long start,
> -				   unsigned long end)
> +				   unsigned long end,
> +				   bool take_locks)
>  {
>  	struct hstate *h = hstate_vma(vma);
>  	unsigned long sz = huge_page_size(h);
> @@ -7521,8 +7542,12 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
>  	mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, mm,
>  				start, end);
>  	mmu_notifier_invalidate_range_start(&range);
> -	hugetlb_vma_lock_write(vma);
> -	i_mmap_lock_write(vma->vm_file->f_mapping);
> +	if (take_locks) {
> +		hugetlb_vma_lock_write(vma);
> +		i_mmap_lock_write(vma->vm_file->f_mapping);
> +	} else {
> +		i_mmap_assert_write_locked(vma->vm_file->f_mapping);
> +	}
>  	for (address = start; address < end; address += PUD_SIZE) {
>  		ptep = huge_pte_offset(mm, address, sz);
>  		if (!ptep)
> @@ -7532,8 +7557,10 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
>  		spin_unlock(ptl);
>  	}
>  	flush_hugetlb_tlb_range(vma, start, end);
> -	i_mmap_unlock_write(vma->vm_file->f_mapping);
> -	hugetlb_vma_unlock_write(vma);
> +	if (take_locks) {
> +		i_mmap_unlock_write(vma->vm_file->f_mapping);
> +		hugetlb_vma_unlock_write(vma);
> +	}
>  	/*
>  	 * No need to call mmu_notifier_invalidate_range(), see
>  	 * Documentation/mm/mmu_notifier.rst.
> @@ -7548,7 +7575,8 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
>  void hugetlb_unshare_all_pmds(struct vm_area_struct *vma)
>  {
>  	hugetlb_unshare_pmds(vma, ALIGN(vma->vm_start, PUD_SIZE),
> -			ALIGN_DOWN(vma->vm_end, PUD_SIZE));
> +			ALIGN_DOWN(vma->vm_end, PUD_SIZE),
> +			/* take_locks = */ true);
>  }
>  
>  #ifdef CONFIG_CMA
> diff --git a/mm/mmap.c b/mm/mmap.c
> index ebc3583fa612..0f303dc8425a 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -727,7 +727,15 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>  		return -ENOMEM;
>  	}
>  
> +	/*
> +	 * Get rid of huge pages and shared page tables straddling the split
> +	 * boundary.
> +	 */
>  	vma_adjust_trans_huge(orig_vma, start, end, adjust_next);
> +	if (is_vm_hugetlb_page(orig_vma)) {
> +		hugetlb_split(orig_vma, start);
> +		hugetlb_split(orig_vma, end);
> +	}
>  	if (file) {
>  		mapping = file->f_mapping;
>  		root = &mapping->i_mmap;
> -- 
> 2.50.0.rc2.701.gf1e915cc24-goog

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before
  2025-07-07 10:39   ` [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before Fedor Pchelkin
@ 2025-07-07 13:19     ` Jann Horn
  0 siblings, 0 replies; 9+ messages in thread
From: Jann Horn @ 2025-07-07 13:19 UTC (permalink / raw)
  To: Fedor Pchelkin
  Cc: stable, lvc-project, Muchun Song, Oscar Salvador, Andrew Morton,
	Liam R. Howlett, Lorenzo Stoakes, Vlastimil Babka, Pedro Falcato

+cc relevant maintainers for the relevant upstream code so they're
aware of how I broke stable with my backports of hugetlb fixes. Looks
like I wasn't being careful enough...

On Mon, Jul 7, 2025 at 12:39 PM Fedor Pchelkin <pchelkin@ispras.ru> wrote:
> On Fri, 20 Jun 2025 23:33:31 +0200, Jann Horn wrote:
> > Currently, __split_vma() triggers hugetlb page table unsharing through
> > vm_ops->may_split().  This happens before the VMA lock and rmap locks are
> > taken - which is too early, it allows racing VMA-locked page faults in our
> > process and racing rmap walks from other processes to cause page tables to
> > be shared again before we actually perform the split.
> >
> > Fix it by explicitly calling into the hugetlb unshare logic from
> > __split_vma() in the same place where THP splitting also happens.  At that
> > point, both the VMA and the rmap(s) are write-locked.
> >
> > An annoying detail is that we can now call into the helper
> > hugetlb_unshare_pmds() from two different locking contexts:
> >
> > 1. from hugetlb_split(), holding:
> >     - mmap lock (exclusively)
> >     - VMA lock
> >     - file rmap lock (exclusively)
> > 2. hugetlb_unshare_all_pmds(), which I think is designed to be able to
> >    call us with only the mmap lock held (in shared mode), but currently
> >    only runs while holding mmap lock (exclusively) and VMA lock
> >
> > Backporting note:
> > This commit fixes a racy protection that was introduced in commit
> > b30c14cd6102 ("hugetlb: unshare some PMDs when splitting VMAs"); that
> > commit claimed to fix an issue introduced in 5.13, but it should actually
> > also go all the way back.
> >
> > [jannh@google.com: v2]
> >   Link: https://lkml.kernel.org/r/20250528-hugetlb-fixes-splitrace-v2-1-1329349bad1a@google.com
> > Link: https://lkml.kernel.org/r/20250528-hugetlb-fixes-splitrace-v2-0-1329349bad1a@google.com
> > Link: https://lkml.kernel.org/r/20250527-hugetlb-fixes-splitrace-v1-1-f4136f5ec58a@google.com
> > Fixes: 39dde65c9940 ("[PATCH] shared page table for hugetlb page")
> > Signed-off-by: Jann Horn <jannh@google.com>
> > Cc: Liam Howlett <liam.howlett@oracle.com>
> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > Reviewed-by: Oscar Salvador <osalvador@suse.de>
> > Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: <stable@vger.kernel.org>  [b30c14cd6102: hugetlb: unshare some PMDs when splitting VMAs]
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > [stable backport: code got moved around, VMA splitting is in
> > __vma_adjust]
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> >  include/linux/hugetlb.h |  3 +++
> >  mm/hugetlb.c            | 60 ++++++++++++++++++++++++++++++-----------
> >  mm/mmap.c               |  8 ++++++
> >  3 files changed, 55 insertions(+), 16 deletions(-)
> >
> > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> > index cc555072940f..26f2947c399d 100644
> > --- a/include/linux/hugetlb.h
> > +++ b/include/linux/hugetlb.h
> > @@ -239,6 +239,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
> >
> >  bool is_hugetlb_entry_migration(pte_t pte);
> >  void hugetlb_unshare_all_pmds(struct vm_area_struct *vma);
> > +void hugetlb_split(struct vm_area_struct *vma, unsigned long addr);
> >
> >  #else /* !CONFIG_HUGETLB_PAGE */
> >
> > @@ -472,6 +473,8 @@ static inline vm_fault_t hugetlb_fault(struct mm_struct *mm,
> >
> >  static inline void hugetlb_unshare_all_pmds(struct vm_area_struct *vma) { }
> >
> > +static inline void hugetlb_split(struct vm_area_struct *vma, unsigned long addr) {}
> > +
> >  #endif /* !CONFIG_HUGETLB_PAGE */
> >  /*
> >   * hugepages at page global directory. If arch support
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 14b9494c58ed..fc5d3d665266 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -95,7 +95,7 @@ static void hugetlb_vma_lock_free(struct vm_area_struct *vma);
> >  static void hugetlb_vma_lock_alloc(struct vm_area_struct *vma);
> >  static void __hugetlb_vma_unlock_write_free(struct vm_area_struct *vma);
> >  static void hugetlb_unshare_pmds(struct vm_area_struct *vma,
> > -             unsigned long start, unsigned long end);
> > +             unsigned long start, unsigned long end, bool take_locks);
> >  static struct resv_map *vma_resv_map(struct vm_area_struct *vma);
> >
> >  static inline bool subpool_is_free(struct hugepage_subpool *spool)
> > @@ -4900,26 +4900,40 @@ static int hugetlb_vm_op_split(struct vm_area_struct *vma, unsigned long addr)
> >  {
> >       if (addr & ~(huge_page_mask(hstate_vma(vma))))
> >               return -EINVAL;
> > +     return 0;
> > +}
> >
> > +void hugetlb_split(struct vm_area_struct *vma, unsigned long addr)
> > +{
> >       /*
> >        * PMD sharing is only possible for PUD_SIZE-aligned address ranges
> >        * in HugeTLB VMAs. If we will lose PUD_SIZE alignment due to this
> >        * split, unshare PMDs in the PUD_SIZE interval surrounding addr now.
> > +      * This function is called in the middle of a VMA split operation, with
> > +      * MM, VMA and rmap all write-locked to prevent concurrent page table
> > +      * walks (except hardware and gup_fast()).
> >        */
> > +     mmap_assert_write_locked(vma->vm_mm);
> > +     i_mmap_assert_write_locked(vma->vm_file->f_mapping);
>
>
> The above i_mmap lock assertion is firing on stable kernels from 5.10 to 6.1
> included.
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 11489 at include/linux/fs.h:503 i_mmap_assert_write_locked include/linux/fs.h:503 [inline]
> WARNING: CPU: 0 PID: 11489 at include/linux/fs.h:503 hugetlb_split+0x267/0x300 mm/hugetlb.c:4917
> Modules linked in:
> CPU: 0 PID: 11489 Comm: syz-executor.4 Not tainted 6.1.142-syzkaller-00296-gfd0df5221577 #0
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
> RIP: 0010:i_mmap_assert_write_locked include/linux/fs.h:503 [inline]
> RIP: 0010:hugetlb_split+0x267/0x300 mm/hugetlb.c:4917
> Call Trace:
>  <TASK>
>  __vma_adjust+0xd73/0x1c10 mm/mmap.c:736
>  vma_adjust include/linux/mm.h:2745 [inline]
>  __split_vma+0x459/0x540 mm/mmap.c:2385
>  do_mas_align_munmap+0x5f2/0xf10 mm/mmap.c:2497
>  do_mas_munmap+0x26c/0x2c0 mm/mmap.c:2646
>  __mmap_region mm/mmap.c:2694 [inline]
>  mmap_region+0x19f/0x1770 mm/mmap.c:2912
>  do_mmap+0x84b/0xf20 mm/mmap.c:1432
>  vm_mmap_pgoff+0x1af/0x280 mm/util.c:520
>  ksys_mmap_pgoff+0x41f/0x5a0 mm/mmap.c:1478
>  do_syscall_x64 arch/x86/entry/common.c:51 [inline]
>  do_syscall_64+0x35/0x80 arch/x86/entry/common.c:81
>  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
> RIP: 0033:0x46a269
>  </TASK>
>
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
>
>
> The main reason is that those branches lack the following
>
>   commit ccf1d78d8b86e28502fa1b575a459a402177def4
>   Author: Suren Baghdasaryan <surenb@google.com>
>   Date:   Mon Feb 27 09:36:13 2023 -0800
>
>       mm/mmap: move vma_prepare before vma_adjust_trans_huge
>
>       vma_prepare() acquires all locks required before VMA modifications.  Move
>       vma_prepare() before vma_adjust_trans_huge() so that VMA is locked before
>       any modification.
>
>       Link: https://lkml.kernel.org/r/20230227173632.3292573-15-surenb@google.com
>       Signed-off-by: Suren Baghdasaryan <surenb@google.com>
>       Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>
> thus the needed lock is acquired just after vma_adjust_trans_huge() and
> the newly added hugetlb_split().

Oh, yuck. Indeed. Thanks for finding this.

> Please have a look at a straightforward write-up which comes to my mind.
> It does something like the ccf1d78d8b86 ("mm/mmap: move vma_prepare before
> vma_adjust_trans_huge"), but in context of an old stable branch.
>
> If looks okay, I'll be glad to prepare it as a formal patch and send it
> out for the 5.10-5.15, too.

Thanks, that looks good to me.

> against 6.1.y
> -------------
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 0f303dc8425a..941880ed62d7 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -543,8 +543,6 @@ inline int vma_expand(struct ma_state *mas, struct vm_area_struct *vma,
>         if (mas_preallocate(mas, vma, GFP_KERNEL))
>                 goto nomem;
>
> -       vma_adjust_trans_huge(vma, start, end, 0);
> -
>         if (file) {
>                 mapping = file->f_mapping;
>                 root = &mapping->i_mmap;
> @@ -562,6 +560,8 @@ inline int vma_expand(struct ma_state *mas, struct vm_area_struct *vma,
>                 vma_interval_tree_remove(vma, root);
>         }
>
> +       vma_adjust_trans_huge(vma, start, end, 0);
> +
>         vma->vm_start = start;
>         vma->vm_end = end;
>         vma->vm_pgoff = pgoff;
> @@ -727,15 +727,6 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>                 return -ENOMEM;
>         }
>
> -       /*
> -        * Get rid of huge pages and shared page tables straddling the split
> -        * boundary.
> -        */
> -       vma_adjust_trans_huge(orig_vma, start, end, adjust_next);
> -       if (is_vm_hugetlb_page(orig_vma)) {
> -               hugetlb_split(orig_vma, start);
> -               hugetlb_split(orig_vma, end);
> -       }
>         if (file) {
>                 mapping = file->f_mapping;
>                 root = &mapping->i_mmap;
> @@ -775,6 +766,16 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>                         vma_interval_tree_remove(next, root);
>         }
>
> +       /*
> +        * Get rid of huge pages and shared page tables straddling the split
> +        * boundary.
> +        */
> +       vma_adjust_trans_huge(orig_vma, start, end, adjust_next);
> +       if (is_vm_hugetlb_page(orig_vma)) {
> +               hugetlb_split(orig_vma, start);
> +               hugetlb_split(orig_vma, end);
> +       }
> +
>         if (start != vma->vm_start) {
>                 if ((vma->vm_start < start) &&
>                     (!insert || (insert->vm_end != start))) {

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-07-07 13:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-20 10:36 FAILED: patch "[PATCH] mm/hugetlb: unshare page tables during VMA split, not before" failed to apply to 6.1-stable tree gregkh
2025-06-20 21:33 ` [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before Jann Horn
2025-06-20 21:33   ` [PATCH 6.1.y 2/3] mm: hugetlb: independent PMD page table shared count Jann Horn
2025-06-29 13:00     ` Vitaly Chikunov
2025-06-30 17:12       ` Jann Horn
2025-06-30 19:17         ` Jann Horn
2025-06-20 21:33   ` [PATCH 6.1.y 3/3] mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race Jann Horn
2025-07-07 10:39   ` [PATCH 6.1.y 1/3] mm/hugetlb: unshare page tables during VMA split, not before Fedor Pchelkin
2025-07-07 13:19     ` Jann Horn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox