[PATCH 0/4] mm: fixes for huge PFN mappings

[PATCH 0/4] mm: fixes for huge PFN mappings

[Eliav Farber]

Four independent fixes to the existing huge PFN mapping code. They are
not dependent on each other and can be taken separately.

Patches 1-3 are three symptoms of one race. A VM_PFNMAP mapping that
provides both ->fault and ->huge_fault can take faults of different
orders on the same entry, because the mm's THP policy is read early in
each fault and is not held across it. Flipping it while faults are in
flight, for example with prctl(PR_SET_THP_DISABLE), is enough: one
fault takes the PTE path while another installs a PMD leaf over the
same address, or the reverse.

This is reachable today. vfio-pci has exactly that shape:
vfio_pci_mmap_ops provides .fault, which reaches insert_pfn() for
order 0, and .huge_fault, which reaches insert_pmd(); mshv_vtl_main and
nvgrace-gpu are built the same way.

  1. walk_to_pmd() asserts that the PMD is not a leaf. With
     CONFIG_DEBUG_VM=y the losing fault hits that assertion instead of
     recovering.
  2. insert_pfn() treats a NULL return from the PTE map as an
     allocation failure, so the losing fault reports VM_FAULT_OOM for a
     mapping that is healthy.
  3. insert_pmd() and insert_pud() compare the PFN of a non-empty entry
     without checking that it is a leaf. A page table installed by a
     concurrent order-0 fault makes pmd_pfn()/pud_pfn() report the
     table's own PFN, which never matches, and the mismatch is warned
     about. Under panic_on_warn that is fatal.

Patch 4 is unrelated to the race. insert_pmd() and insert_pud() build a
huge entry from a pgprot laid out for a PTE. On x86 the PAT bit sits at
bit 7 in a PTE and at bit 12 in a large entry, and bit 7 in a large
entry is _PAGE_PSE, so the attribute is silently dropped: write-through
degrades to UC and write-protect degrades to write-combining, which
loses the write protection. Run the pgprot through
pgprot_4k_2_large(), with a generic identity fallback for
architectures that encode the levels alike.

How these were observed, since it differs between them:

Patches 1-3 were reproduced on x86_64 under QEMU with four threads
colliding on one address of each PMD window of a VM_PFNMAP mapping
while another thread flipped PR_SET_THP_DISABLE. With patches 1 and 2
reverted and CONFIG_DEBUG_VM=y, that lands within three seconds:

  kernel BUG at mm/memory.c:2392!
  RIP: 0010:walk_to_pmd+0x188/0x190
  Call Trace:
   get_locked_pte+0x1b/0x70
   insert_pfn+0x40/0x230
   vmf_insert_pfn_prot+0x94/0x100
   __do_fault+0x30/0xb0
   __handle_mm_fault+0xbb2/0x1a90
   handle_mm_fault+0x9b/0x270
   do_user_addr_fault+0x15d/0x630

and BUG() leaves mmap_lock held, so the system then wedges. With those
two applied the race is survived, and patch 3's warning appears
instead:

  WARNING: mm/huge_memory.c:1649 at insert_pmd.isra.0+0x1d3/0x430
  Call Trace:
   vmf_insert_pfn_pmd+0xa6/0xf0
   __handle_mm_fault+0x45b/0x1a90

With all four applied the same test runs 51200 colliding faults with no
BUG, no warning, no OOM and no incorrect data. Debug counters placed at
each site in a throwaway tree confirmed the recovery path in patch 2 is
taken rather than merely not crashing.

Patch 4 was found by inspection rather than from a failure report. The
in-tree ->huge_fault providers map write-back or uncached memory, and
only write-through and write-protect set _PAGE_PAT, so I have not seen
the dropped attribute in practice. The entry is still built from the
wrong encoding, and a caller asking for write-protect would lose it
silently.

Found while adding huge_fault support for /dev/mem and UIO:
https://lore.kernel.org/all/20261111113045.35368-1-farbere@amazon.com/
That series is not a prerequisite for any of these.

Tested on x86_64 under QEMU with CONFIG_DEBUG_VM=y. The race above needs
a VM_PFNMAP mapping providing both ->fault and ->huge_fault, so the
reproducer ran on these four patches applied on top of the /dev/mem and
UIO series linked above, which gave it one to use; vfio-pci would serve
the same purpose. This series on its own boots clean and was
cross-built for riscv64, the other architecture selecting
ARCH_SUPPORTS_PUD_PFNMAP, so insert_pud() and pud_leaf() are compiled.
checkpatch --strict is clean on all four.

Based on v7.4-rc2 (067c5c24cef2).

Eliav Farber (4):
  mm: drop the huge PMD assertion from walk_to_pmd()
  mm: don't report OOM when a huge PMD race blocks a pfn insert
  mm/huge_memory: don't warn when a page table blocks a huge pfn insert
  mm/huge_memory: use the large-page PAT encoding for huge pfn mappings

 arch/x86/include/asm/pgtable_types.h |  2 ++
 include/linux/pgtable.h              |  9 +++++++++
 mm/huge_memory.c                     | 20 ++++++++++++++++++--
 mm/memory.c                          | 20 +++++++++++++++++---
 4 files changed, 46 insertions(+), 5 deletions(-)

-- 
2.47.3

[PATCH 1/4] mm: drop the huge PMD assertion from walk_to_pmd()

[Eliav Farber]

walk_to_pmd() asserts that the PMD it returns is not a huge entry. That
held when commit c2febafc6773 ("mm: convert generic code to 5-level
paging") added the assertion: nothing installed a huge PMD over the
mappings its callers walk.

ARCH_SUPPORTS_HUGE_PFNMAP changed that. A VM_PFNMAP mapping whose driver
provides both ->fault and ->huge_fault can have a PMD leaf installed by a
concurrent huge fault over the same address, so a caller can observe one
and BUG on CONFIG_DEBUG_VM=y.

The walk does not need the invariant. pmd_alloc() only allocates when the
PUD is empty and otherwise returns pmd_offset(), so a leaf is returned
rather than overwritten. Every caller already copes with one, because
they all reach pte_offset_map_lock(), which refuses to walk a leaf and
returns NULL: insert_pages() turns that into -EFAULT, get_locked_pte()
passes the NULL to its callers, and insert_pfn() returns a fault code.

Fixes: f9e54c3a2f5b ("vfio/pci: implement huge_fault support")
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 mm/memory.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index ec3d9d99ccee..0f54ee0a6d88 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2384,7 +2384,6 @@ static pmd_t *walk_to_pmd(struct mm_struct *mm, unsigned long addr)
 	if (!pmd)
 		return NULL;
 
-	VM_BUG_ON(pmd_trans_huge(*pmd));
 	return pmd;
 }
 
-- 
2.47.3

[PATCH 2/4] mm: don't report OOM when a huge PMD race blocks a pfn insert

[Eliav Farber]

insert_pfn() maps the PTE with get_locked_pte() and treats a NULL return
as an allocation failure. pte_offset_map_lock() also returns NULL when
the PMD is a leaf, which a VM_PFNMAP mapping with both ->fault and
->huge_fault can hit: an order-0 fault that passed handle_pte_fault()'s
pmd_none() check can reach insert_pfn() after a concurrent huge fault
installed a PMD leaf over the same address. The task then takes a
VM_FAULT_OOM for a mapping that is perfectly healthy. Changing an mm's
THP policy while another thread faults the same PMD, for example with
prctl(PR_SET_THP_DISABLE), is enough to produce the mixed orders.

Walk to the PMD first and tell the two cases apart: a failed PTE
allocation stays VM_FAULT_OOM, while a PMD leaf returns VM_FAULT_NOPAGE
so that the access refaults onto the huge entry.

Fixes: f9e54c3a2f5b ("vfio/pci: implement huge_fault support")
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 mm/memory.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 0f54ee0a6d88..b7b89d041f42 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2866,10 +2866,25 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr,
 	struct mm_struct *mm = vma->vm_mm;
 	pte_t *pte, entry;
 	spinlock_t *ptl;
+	pmd_t *pmd;
 
-	pte = get_locked_pte(mm, addr, &ptl);
-	if (!pte)
+	pmd = walk_to_pmd(mm, addr);
+	if (!pmd)
+		return VM_FAULT_OOM;
+
+	pte = pte_alloc_map_lock(mm, pmd, addr, &ptl);
+	if (!pte) {
+		/*
+		 * A concurrent ->huge_fault may have installed a PMD leaf over
+		 * addr, which pte_offset_map_lock() refuses to walk. The
+		 * mapping is fine, so let the access refault instead of
+		 * reporting a failed allocation.
+		 */
+		if (pmd_trans_huge(pmdp_get_lockless(pmd)))
+			return VM_FAULT_NOPAGE;
 		return VM_FAULT_OOM;
+	}
+
 	entry = ptep_get(pte);
 	if (!pte_none(entry)) {
 		if (mkwrite) {
-- 
2.47.3

[PATCH 3/4] mm/huge_memory: don't warn when a page table blocks a huge pfn insert

[Eliav Farber]

insert_pmd() and insert_pud() treat a non-empty entry at their own level
as a leaf and compare its PFN against the one being inserted. A page
table installed by a concurrent order-0 fault also makes the entry
non-empty, and pmd_pfn()/pud_pfn() then report the PFN of that table,
which never matches. insert_pmd() warns unless the entry is the huge
zero PMD, and insert_pud() warns on any mismatch.

A VM_PFNMAP mapping with both ->fault and ->huge_fault can produce the
mixed orders: an order-0 fault installs a page table over an address
while a concurrent huge fault on the same address reaches insert_pmd().
Changing an mm's THP policy while another thread faults the same PMD,
for example with prctl(PR_SET_THP_DISABLE), is enough to do it.

Compare the PFN only for a leaf entry. A page table is not an error, so
take the existing out_unlock path, which returns VM_FAULT_NOPAGE and
lets the access refault onto the smaller entry.

Fixes: f9e54c3a2f5b ("vfio/pci: implement huge_fault support")
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 mm/huge_memory.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index eb913acb03b7..052c50dadc48 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1643,6 +1643,14 @@ static vm_fault_t insert_pmd(struct vm_area_struct *vma, unsigned long addr,
 		const unsigned long pfn = fop.is_folio ? folio_pfn(fop.folio) :
 					  fop.pfn;
 
+		/*
+		 * A concurrent order-0 fault may have installed a PTE table
+		 * here, which pmd_pfn() would report as the PFN of the table
+		 * itself. Only a leaf entry can be compared.
+		 */
+		if (!pmd_leaf(*pmd))
+			goto out_unlock;
+
 		if (write) {
 			if (pmd_pfn(*pmd) != pfn) {
 				WARN_ON_ONCE(!is_huge_zero_pmd(*pmd));
@@ -1770,6 +1778,14 @@ static vm_fault_t insert_pud(struct vm_area_struct *vma, unsigned long addr,
 		const unsigned long pfn = fop.is_folio ? folio_pfn(fop.folio) :
 					  fop.pfn;
 
+		/*
+		 * A concurrent order-0 fault may have installed a PMD table
+		 * here, which pud_pfn() would report as the PFN of the table
+		 * itself. Only a leaf entry can be compared.
+		 */
+		if (!pud_leaf(*pud))
+			goto out_unlock;
+
 		if (write) {
 			if (WARN_ON_ONCE(pud_pfn(*pud) != pfn))
 				goto out_unlock;
-- 
2.47.3

[PATCH 4/4] mm/huge_memory: use the large-page PAT encoding for huge pfn mappings

[Eliav Farber]

insert_pmd() and insert_pud() build the entry from the pgprot that
vmf_insert_pfn_pmd()/_pud() took from vma->vm_page_prot and passed
through pfnmap_setup_cachemode_pfn(). That pgprot carries the PTE-level
cache mode encoding, but x86 keeps the high PAT bit at bit 7 for a PTE
(_PAGE_BIT_PAT) and at bit 12 for a PMD or PUD (_PAGE_BIT_PAT_LARGE),
where bit 7 is _PAGE_PSE.

A memtype that needs that bit, write-through or write-protected, loses
it: the cache mode index of the huge entry drops from 4-7 to 0-3, so a
write-through range is mapped write-back. The other x86 paths that build
a large entry from a PTE pgprot convert it first, see populate_pmd() and
populate_pud().

Convert in the two huge pfn paths. Add a generic pgprot_4k_2_large()
that returns its argument unchanged, so that only architectures which
encode the levels differently have to define it, and x86's existing
definition is picked up.

The folio paths keep using vma->vm_page_prot as is. They map
struct-page-backed memory, which x86 maps write-back.

Fixes: 6857be5fecae ("mm: introduce ARCH_SUPPORTS_HUGE_PFNMAP and special bits to pmd/pud")
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 arch/x86/include/asm/pgtable_types.h | 2 ++
 include/linux/pgtable.h              | 9 +++++++++
 mm/huge_memory.c                     | 4 ++--
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index b1a669e775f4..b1ffd72e47ef 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -497,6 +497,8 @@ static inline pgprotval_t protval_4k_2_large(pgprotval_t val)
 	return (val & ~(_PAGE_PAT | _PAGE_PAT_LARGE)) |
 		((val & _PAGE_PAT) << (_PAGE_BIT_PAT_LARGE - _PAGE_BIT_PAT));
 }
+
+#define pgprot_4k_2_large pgprot_4k_2_large
 static inline pgprot_t pgprot_4k_2_large(pgprot_t pgprot)
 {
 	return __pgprot(protval_4k_2_large(pgprot_val(pgprot)));
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index 780fe849ff8b..112814d31f0c 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -1787,6 +1787,15 @@ void arch_sync_kernel_mappings(unsigned long start, unsigned long end);
 #define pgprot_mhp(prot)	(prot)
 #endif
 
+/*
+ * Convert a pgprot built for a PTE so that it can be used in a PMD or PUD
+ * entry. Only needed on architectures that encode attributes differently
+ * between the levels; elsewhere the pgprot is used as is.
+ */
+#ifndef pgprot_4k_2_large
+#define pgprot_4k_2_large(prot)	(prot)
+#endif
+
 #ifdef CONFIG_MMU
 #ifndef pgprot_modify
 #define pgprot_modify pgprot_modify
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 052c50dadc48..945d480c1512 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1675,7 +1675,7 @@ static vm_fault_t insert_pmd(struct vm_area_struct *vma, unsigned long addr,
 			add_mm_counter(mm, mm_counter_file(fop.folio), HPAGE_PMD_NR);
 		}
 	} else {
-		entry = pmd_mkhuge(pfn_pmd(fop.pfn, prot));
+		entry = pmd_mkhuge(pfn_pmd(fop.pfn, pgprot_4k_2_large(prot)));
 		entry = pmd_mkspecial(entry);
 	}
 	if (write) {
@@ -1804,7 +1804,7 @@ static vm_fault_t insert_pud(struct vm_area_struct *vma, unsigned long addr,
 		folio_add_file_rmap_pud(fop.folio, &fop.folio->page, vma);
 		add_mm_counter(mm, mm_counter_file(fop.folio), HPAGE_PUD_NR);
 	} else {
-		entry = pud_mkhuge(pfn_pud(fop.pfn, prot));
+		entry = pud_mkhuge(pfn_pud(fop.pfn, pgprot_4k_2_large(prot)));
 		entry = pud_mkspecial(entry);
 	}
 	if (write) {
-- 
2.47.3