[PATCH 0/5] x86/mm/pat: CPA fixes

[PATCH 0/5] x86/mm/pat: CPA fixes

[Mike Rapoport (Microsoft)]

There are a couple of CPA fixes floating around:

Denis Lunev fixed races between split and collapse of the large mappings:

https://lore.kernel.org/all/20260715183453.2381141-1-den@openvz.org

Lorenzo Stoakes fixed UAF caused by races between CPA and ptdump:

https://lore.kernel.org/all/20260723-series-vmap-race-fix-v6-0-8cc77dcc0018@kernel.org

and an issue with stale page tables in IOMMU:

https://lore.kernel.org/all/20260721-fix-cpa-kernel-pagetables-v2-1-2b255deed710@kernel.org

Mike Rapoport fixed a check of RW attribute in lookup_address_in_pgd_attr()
used for the verification of RWX:

https://lore.kernel.org/all/20260715144519.934289-1-rppt@kernel.org

Some of the fixes got merged into x86 tree, some of them got merged into mm
tree and some are still hanging in the air.

Beside the fixes there was a supposed simplification of cpa_lock locking 
that looked like removal of an optimization for DEBUG_PAGEALLOC, but it
turned out that it was not an optimization but rather a correctness
guard because with DEBUG_PAGEALLOC the locks could be taken in an atomic
context and couldn't use plain spin_lock()/spin_unlock().

The changes here are collected from all these fixes into a sinlge coherent
set on top of tip/x86/mm:
 
* update to cpa_lock handling with DEBUG_PAGEALLOC
* fix for races between CPA and ptdumpi causing UAF
* fix for stale page tables in IOMMU
* update to the fix of the race between split and collapse of large
  mappings
* fix for effective RW computation in lookup_address_in_pgd_attr()

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
Lorenzo Stoakes (ARM) (3):
      x86/mm/pat: acquire init_mm write lock on collapse to avoid UAF
      x86/mm/pat: acquire init_mm read lock on attribute change to avoid UAF
      x86/mm/pat: allocate split page tables as kernel page tables

Mike Rapoport (Microsoft) (2):
      x86/mm/pat: introcude cpa_lock() and cpa_unlock()
      x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()

 arch/x86/mm/pat/set_memory.c | 95 +++++++++++++++++++++++++++++++-------------
 include/linux/mmap_lock.h    |  2 +
 2 files changed, 70 insertions(+), 27 deletions(-)
---
base-commit: a5a162fe1ae130e3d2ceefef3f43afe3773c1d56
change-id: 20260727-cpa-fixes-d3c73c075672

--
Sincerely yours,
Mike.

[PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Mike Rapoport (Microsoft)]

The splitting and merging of kernel page table mappings between small and
large is protected by cpa_lock.

Commit 5fce67641a3e ("x86/mm/pat: Don't gate cpa_lock on
debug_pagealloc_enabled()") disabled gatig of cpa_lock on
debug_pagealloc_enabled() to simplify the code presuming that skipping
the lock when debug_pagealloc_enabled() was an optimization.

However Lorenzo Stoakes notes that:

  __kernel_map_pages() can be called from irq context:

  < GFP_ATOMIC context >
  kfree() or whatever
  -> ...
  -> __free_pages_prepare()
  -> debug_pagealloc_unmap_pages()
  -> __kernel_map_pages()
  -> __change_page_attr_set_clr()
  -> cpa_lock spins [irqs off]

  So you're spin locking in irq context here, which is probably not a
  good idea.

It would be possible to unconditionally use spin_lock_irqsave() and
spin_unlock_irqrestore() but that would complicate locking rules even
more.

Restore gating of cpa_lock of debug_pagealloc_enabled(), but instead of
putting the open-coded condition

	if (debug_pagealloc_enabled())

before every lock and unlock operation, wrap the condition and the
locking operation into cpa_lock() and cpa_unlock() helpers.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index b1e780a465b5..4c8922695fd3 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -65,8 +65,25 @@ static const int cpa_warn_level = CPA_PROTECT;
  * Serialize cpa() using cpa_lock so that we don't allow any other cpu, with
  * stale large tlb entries, to change the page attribute in parallel to some
  * other cpu splitting a large page entry along with changing the attribute.
+ *
+ * When debug_pagealloc_enabled(), page attributes could be changed in atomic
+ * context that would warrant disabling IRQs. But since debug_pagealloc always
+ * uses 4k pages in the direct map there are no races for splits and collapses
+ * and locking can be just skipped altogether.
  */
-static DEFINE_SPINLOCK(cpa_lock);
+static DEFINE_SPINLOCK(_cpa_lock);
+
+static inline void cpa_lock(void)
+{
+	if (!debug_pagealloc_enabled())
+		spin_lock(&_cpa_lock);
+}
+
+static inline void cpa_unlock(void)
+{
+	if (!debug_pagealloc_enabled())
+		spin_unlock(&_cpa_lock);
+}
 
 #define CPA_FLUSHTLB 1
 #define CPA_ARRAY 2
@@ -417,7 +434,7 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
 	int collapsed = 0;
 	int i;
 
-	spin_lock(&cpa_lock);
+	cpa_lock();
 
 	if (cpa->flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
 		for (i = 0; i < cpa->numpages; i++)
@@ -433,7 +450,7 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
 	}
 
 	if (!collapsed) {
-		spin_unlock(&cpa_lock);
+		cpa_unlock();
 		return;
 	}
 
@@ -444,7 +461,7 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
 		pagetable_free(ptdesc);
 	}
 
-	spin_unlock(&cpa_lock);
+	cpa_unlock();
 }
 
 static void cpa_flush(struct cpa_data *cpa, int cache)
@@ -1239,9 +1256,9 @@ static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
 {
 	struct ptdesc *ptdesc;
 
-	spin_unlock(&cpa_lock);
+	cpa_unlock();
 	ptdesc = pagetable_alloc(GFP_KERNEL, 0);
-	spin_lock(&cpa_lock);
+	cpa_lock();
 	if (!ptdesc)
 		return -ENOMEM;
 
@@ -2025,9 +2042,9 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int primary)
 		if (cpa->flags & (CPA_ARRAY | CPA_PAGES_ARRAY))
 			cpa->numpages = 1;
 
-		spin_lock(&cpa_lock);
+		cpa_lock();
 		ret = __change_page_attr(cpa, primary);
-		spin_unlock(&cpa_lock);
+		cpa_unlock();
 		if (ret)
 			goto out;
 

-- 
2.53.0

[PATCH 2/5] x86/mm/pat: acquire init_mm write lock on collapse to avoid UAF

[Mike Rapoport]

From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>

x86 implements page attribute modification using its Change Page
Attributes (CPA) mechanism.

This tracks properties of ranges such as cache mode through x86 page
attributes, and as part of that logic manipulates kernel page tables.

Since commit 41d88484c71c ("x86/mm/pat: restore large ROX pages after
fragmentation") ranges of kernel page table entries can be collapsed into
huge page table entries as part of this logic.

As part of this collapse, it frees the page tables which the collapsed
entries previously pointed to, and it does so without any relevant locks
being held to preclude concurrent kernel page table walkers.

The only way this code can be reached is if CPA_COLLAPSE is specified, and
this is only set in set_memory_rox() via:

set_memory_rox()
-> change_page_attr_set_clr()
-> cpa_flush()
-> cpa_collapse_large_pages()

Notable users of this are execmem and bpf when manipulating executable
mappings.

However, this is problematic for ptdump as it walks ranges it does not own
and thus runs the risk of a use-after-free on page tables freed underneath
it.

In addition, concurrent CPA collapse operations are possible which can also
cause races.

Resolve the issue by acquiring the mmap write lock on init_mm across the
whole operation.

It is safe to acquire a sleeping lock as all the callers invoke
set_memory_rox() from process context and in any case,
change_page_attr_set_clr() calls vm_unmap_alias() which ultimately takes a
mutex, disallowing atomic context here.

Fixes: 41d88484c71c ("x86/mm/pat: restore large ROX pages after fragmentation")
Cc: stable@vger.kernel.org
Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Will Deacon <will@kernel.org>
Reviewed-by: David Carlier <devnexen@gmail.com>
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 15 ++++++++++++++-
 include/linux/mmap_lock.h    |  2 ++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 4c8922695fd3..4ba16d72a535 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -22,6 +22,7 @@
 #include <linux/cc_platform.h>
 #include <linux/set_memory.h>
 #include <linux/memregion.h>
+#include <linux/cleanup.h>
 
 #include <asm/e820/api.h>
 #include <asm/processor.h>
@@ -426,7 +427,7 @@ static void __cpa_flush_tlb(void *data)
 
 static int collapse_large_pages(unsigned long addr, struct list_head *pgtables);
 
-static void cpa_collapse_large_pages(struct cpa_data *cpa)
+static void __cpa_collapse_large_pages(struct cpa_data *cpa)
 {
 	unsigned long start, addr, end;
 	struct ptdesc *ptdesc, *tmp;
@@ -464,6 +465,18 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
 	cpa_unlock();
 }
 
+static void cpa_collapse_large_pages(struct cpa_data *cpa)
+{
+	/*
+	 * Take the mmap write lock on init_mm to:
+	 * - Avoid a use-after-free if raced by ptdump (which takes its own
+	 *   write lock on init_mm).
+	 * - Serialise concurrent CPA walkers.
+	 */
+	scoped_guard(mmap_write_lock, &init_mm)
+		__cpa_collapse_large_pages(cpa);
+}
+
 static void cpa_flush(struct cpa_data *cpa, int cache)
 {
 	unsigned int i;
diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
index 04b8f61ece5d..f4ceb968aeb3 100644
--- a/include/linux/mmap_lock.h
+++ b/include/linux/mmap_lock.h
@@ -621,6 +621,8 @@ static inline void mmap_read_unlock(struct mm_struct *mm)
 
 DEFINE_GUARD(mmap_read_lock, struct mm_struct *,
 	     mmap_read_lock(_T), mmap_read_unlock(_T))
+DEFINE_GUARD(mmap_write_lock, struct mm_struct *,
+	     mmap_write_lock(_T), mmap_write_unlock(_T))
 
 static inline void mmap_read_unlock_non_owner(struct mm_struct *mm)
 {

-- 
2.53.0

[PATCH 3/5] x86/mm/pat: acquire init_mm read lock on attribute change to avoid UAF

[Mike Rapoport]

From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>

A previous commit protected us against races between ptdump and CPA
collapse, however one still exists between attribute changes and collapse
as reported by Denis V. Lunev (linked).

When an attribute change arises, a lockless page table walker obtains a PTE
entry, which is later written to via set_pte_atomic():

...
-> change_page_attr_set_clr()
-> __change_page_attr_set_clr()
-> __change_page_attr()
	-> _lookup_address_cpa()
	-> lookup_address_in_pgd_attr()
	-> [ lockless page table walker ]
-> set_pte_atomic()

There is nothing preventing a concurrent CPA collapse which can free the
PTE that was retrieved here, resulting in a use-after-free.

With the mmap write lock taken on init_mm over CPA collapse, we can now
resolve this race by acquiring an mmap read lock on init_mm over
__change_page_attr_set_clr().

This locks across the whole operation over which the walk and the PTE entry
write occurs, solving the race.

It is safe to do this here, as no spinlocks are held upon entry to
__change_page_attr_set_clr().

However, the lock must not be held over an allocation, as allocation can
trigger reclaim and shrinkers may call into CPA recursively, making
deadlocks possible (init_mm -> ... -> fs_reclaim -> init_mm).

A page table is allocated when a huge page needs to be split:

-> change_page_attr_set_clr()
-> __change_page_attr_set_clr()
-> __change_page_attr()
-> split_large_page()
[ pagetable_alloc() ]
-> __split_large_page()

Avoid deadlocks by dropping the mmap lock across pagetable_alloc() in
split_large_page() and track whether this is needed by adding a new
'init_mm_read_locked' flag to struct cpa_data.

This is safe as __split_large_page() (called with locks re-established)
revalidates that the page table entry is the same as it was prior to the
locks being dropped and __change_page_attr() repeats the entire page table
walk whenever a split occurs, so concurrent split and collapse are
accounted for.

Concurrent ptdump is also safe as the lock is only dropped over page table
allocation during which time the page table has not yet been modified.

The CPA_COLLAPSE flag is only set by set_memory_rox(), which exclusively
operates upon vmalloc ranges, and on x86 only within the module mapping
space.

This is important, because some callers directly invoke
__change_page_attr_set_clr(), bypassing this lock. However, none of these
operate within the module mapping space.

* cpa_process_alias() - a recursive helper called by
  __change_page_attr_set_clr().
* __set_memory_enc_pgtable() - operates on the direct mapping and (via
  __vmbus_establish_gpadl()) the vmalloc mapping space.
* __set_pages_[n]p() - called by set_direct_map_[invalid, default,
  valid]_noflush(), __kernel_map_pages() - operates on the direct map.
* kernel_[un]map_pages_in_pgd() - operates on EFI ranges.

This work is based upon Denis V. Lunev's excellent analysis of the bug with
gratitude.

Link: https://lore.kernel.org/all/20260626163213.2284080-1-den@openvz.org/
Fixes: 41d88484c71c ("x86/mm/pat: restore large ROX pages after fragmentation")
Cc: stable@vger.kernel.org
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 4ba16d72a535..26131ecc0e1c 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -50,7 +50,8 @@ struct cpa_data {
 	unsigned int	flags;
 	unsigned int	force_split		: 1,
 			force_static_prot	: 1,
-			force_flush_all		: 1;
+			force_flush_all		: 1,
+			init_mm_read_locked	: 1;
 	struct page	**pages;
 };
 
@@ -435,8 +436,6 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
 	int collapsed = 0;
 	int i;
 
-	cpa_lock();
-
 	if (cpa->flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
 		for (i = 0; i < cpa->numpages; i++)
 			collapsed += collapse_large_pages(__cpa_addr(cpa, i),
@@ -450,10 +449,8 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
 			collapsed += collapse_large_pages(addr, &pgtables);
 	}
 
-	if (!collapsed) {
-		cpa_unlock();
+	if (!collapsed)
 		return;
-	}
 
 	flush_tlb_all();
 
@@ -461,8 +458,6 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
 		list_del(&ptdesc->pt_list);
 		pagetable_free(ptdesc);
 	}
-
-	cpa_unlock();
 }
 
 static void cpa_collapse_large_pages(struct cpa_data *cpa)
@@ -1270,8 +1265,13 @@ static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
 	struct ptdesc *ptdesc;
 
 	cpa_unlock();
+	if (cpa->init_mm_read_locked)
+		mmap_read_unlock(&init_mm);
 	ptdesc = pagetable_alloc(GFP_KERNEL, 0);
+	if (cpa->init_mm_read_locked)
+		mmap_read_lock(&init_mm);
 	cpa_lock();
+
 	if (!ptdesc)
 		return -ENOMEM;
 
@@ -2139,7 +2139,11 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages,
 	cpa.curpage = 0;
 	cpa.force_split = force_split;
 
-	ret = __change_page_attr_set_clr(&cpa, 1);
+	/* Avoid race with concurrent CPA collapse. */
+	cpa.init_mm_read_locked = true;
+	scoped_guard(mmap_read_lock, &init_mm)
+		ret = __change_page_attr_set_clr(&cpa, 1);
+	cpa.init_mm_read_locked = false;
 
 	/*
 	 * Check whether we really changed something:

-- 
2.53.0

[PATCH 4/5] x86/mm/pat: allocate split page tables as kernel page tables

[Mike Rapoport]

From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>

When splitting a large page in CPA in __split_large_page() we allocate a
PTE directly without going through the standard page table allocation
routines such as pte_alloc_one_kernel().

This means the page table constructor is never called nor is the page table
marked as a kernel page table.

The former results in the folio associated with the page table not being
marked as a page table (__pagetable_ctor() is never called thus neither is
__folio_set_pgtable()) nor are statistics updated to reflect
it (lruvec_stat_add_folio() is never called).

The latter issue of failing to mark the page table as a kernel page
table (ptdesc_set_kernel() is never called) is far more problematic.

Since commit 5ba2f0a15564 ("mm: introduce deferred freeing for kernel page
tables") kernel page table freeing has been batched and since the
subsequent commit e37d5a2d60a3 ("iommu/sva: invalidate stale IOTLB entries
for kernel address space") IOTLB cache entries for kernel page tables have
been invalidated upon being freed.

Since split page tables are freed without this invalidation, the IOTLB can
contain stale entries for them.

Resolve the issue by using the ordinary PTE allocation API at split time.

This results in these kernel page tables invoking a page table constructor,
and thus requires a page table destructor.

Since we cannot assume one is always present (early allocated direct map
page tables are not marked as such), we conditionally call
pagetable_dtor_free() if the PG_table folio flag for the ptdesc is set,
otherwise we free the page table via pagetable_free().

Regardless of which path is taken page tables marked as kernel page tables,
which now includes split page tables, take the correct route through
pagetable_free_kernel().

There is a user-visible side effect in that split page tables will appear
in nr_page_table_pages in /proc/vmstat (as do other kernel page tables
allocated after early boot), however this is a positive change.

This issue started being markedly problematic after commit
5ba2f0a15564 ("mm: introduce deferred freeing for kernel page tables") so
choose this as the Fixes target.

Fixes: 5ba2f0a15564 ("mm: introduce deferred freeing for kernel page tables")
Cc: stable@vger.kernel.org
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Acked-by: Vishal Moola <vishal.moola@gmail.com>
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 26131ecc0e1c..1b63d4caba20 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -456,7 +456,15 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
 
 	list_for_each_entry_safe(ptdesc, tmp, &pgtables, pt_list) {
 		list_del(&ptdesc->pt_list);
-		pagetable_free(ptdesc);
+		/*
+		 * Only early alloc'd direct map should not be flagged PG_table
+		 * here and those shouldn't be collapsed. However be abundantly
+		 * cautious and handle the !PG_table case too.
+		 */
+		if (PageTable((ptdesc_page(ptdesc))))
+			pagetable_dtor_free(ptdesc);
+		else
+			pagetable_free(ptdesc);
 	}
 }
 
@@ -1154,11 +1162,10 @@ static void split_set_pte(struct cpa_data *cpa, pte_t *pte, unsigned long pfn,
 
 static int
 __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
-		   struct ptdesc *ptdesc)
+		   pte_t *pbase)
 {
 	unsigned long lpaddr, lpinc, ref_pfn, pfn, pfninc = 1;
-	struct page *base = ptdesc_page(ptdesc);
-	pte_t *pbase = (pte_t *)page_address(base);
+	struct page *base = virt_to_page(pbase);
 	unsigned int i, level;
 	pgprot_t ref_prot;
 	bool nx, rw;
@@ -1262,21 +1269,21 @@ __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
 static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
 			    unsigned long address)
 {
-	struct ptdesc *ptdesc;
+	pte_t *pte;
 
 	cpa_unlock();
 	if (cpa->init_mm_read_locked)
 		mmap_read_unlock(&init_mm);
-	ptdesc = pagetable_alloc(GFP_KERNEL, 0);
+	pte = pte_alloc_one_kernel(&init_mm);
 	if (cpa->init_mm_read_locked)
 		mmap_read_lock(&init_mm);
 	cpa_lock();
 
-	if (!ptdesc)
+	if (!pte)
 		return -ENOMEM;
 
-	if (__split_large_page(cpa, kpte, address, ptdesc))
-		pagetable_free(ptdesc);
+	if (__split_large_page(cpa, kpte, address, pte))
+		pte_free_kernel(&init_mm, pte);
 
 	return 0;
 }

-- 
2.53.0

[PATCH 5/5] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()

[Mike Rapoport (Microsoft)]

lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
the walked page table levels so that verify_rwx() can detect mappings that
are both writable and executable.

The RW bits are folded into a bool with

	rw &= pXd_flags(*pXd) & _PAGE_RW;

but _PAGE_RW is 0x2. So consider the accumulation line:

        rw &= pXd_flags(*pXd) & _PAGE_RW;

where rw=0x1 and the right side evaluates down to 0x2. It'll end up doing:

        rw = 0x1 & 0x2

and rw always ends up 0.

This way rw becomes false at the first level walked, regardless of the
actual permissions, and verify_rwx() treats every mapping as non-writable
and never reports a W^X violation.

Add double negation to the right side to normalize the _PAGE_RW flag to
0 or 1.

Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
Cc: stable@vger.kernel.org
Assisted-by: Copilot:claude-opus-4.8
Reviewed-by: Juergen Gross <jgross@suse.com>
Tested-by: syzbot@syzkaller.appspotmail.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/x86/mm/pat/set_memory.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 1b63d4caba20..7d656520887c 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -769,7 +769,7 @@ pte_t *lookup_address_in_pgd_attr(pgd_t *pgd, unsigned long address,
 
 	*level = PG_LEVEL_512G;
 	*nx |= pgd_flags(*pgd) & _PAGE_NX;
-	*rw &= pgd_flags(*pgd) & _PAGE_RW;
+	*rw &= !!(pgd_flags(*pgd) & _PAGE_RW);
 
 	p4d = p4d_offset(pgd, address);
 	if (p4d_none(*p4d))
@@ -780,7 +780,7 @@ pte_t *lookup_address_in_pgd_attr(pgd_t *pgd, unsigned long address,
 
 	*level = PG_LEVEL_1G;
 	*nx |= p4d_flags(*p4d) & _PAGE_NX;
-	*rw &= p4d_flags(*p4d) & _PAGE_RW;
+	*rw &= !!(p4d_flags(*p4d) & _PAGE_RW);
 
 	pud = pud_offset(p4d, address);
 	if (pud_none(*pud))
@@ -791,7 +791,7 @@ pte_t *lookup_address_in_pgd_attr(pgd_t *pgd, unsigned long address,
 
 	*level = PG_LEVEL_2M;
 	*nx |= pud_flags(*pud) & _PAGE_NX;
-	*rw &= pud_flags(*pud) & _PAGE_RW;
+	*rw &= !!(pud_flags(*pud) & _PAGE_RW);
 
 	pmd = pmd_offset(pud, address);
 	if (pmd_none(*pmd))
@@ -802,7 +802,7 @@ pte_t *lookup_address_in_pgd_attr(pgd_t *pgd, unsigned long address,
 
 	*level = PG_LEVEL_4K;
 	*nx |= pmd_flags(*pmd) & _PAGE_NX;
-	*rw &= pmd_flags(*pmd) & _PAGE_RW;
+	*rw &= !!(pmd_flags(*pmd) & _PAGE_RW);
 
 	return pte_offset_kernel(pmd, address);
 }

-- 
2.53.0

Re: [PATCH 0/5] x86/mm/pat: CPA fixes

[Lorenzo Stoakes (ARM)]

Ack on all my patches thanks!

Andrew - let's take the x86 patches from my series through the x86 tree, and the
core mm patches through the mm tree.

There's no ordering requirement in the x86 patches.

On Tue, Jul 28, 2026 at 04:07:43PM +0300, Mike Rapoport (Microsoft) wrote:
> There are a couple of CPA fixes floating around:
>
> Denis Lunev fixed races between split and collapse of the large mappings:
>
> https://lore.kernel.org/all/20260715183453.2381141-1-den@openvz.org
>
> Lorenzo Stoakes fixed UAF caused by races between CPA and ptdump:
>
> https://lore.kernel.org/all/20260723-series-vmap-race-fix-v6-0-8cc77dcc0018@kernel.org
>
> and an issue with stale page tables in IOMMU:
>
> https://lore.kernel.org/all/20260721-fix-cpa-kernel-pagetables-v2-1-2b255deed710@kernel.org
>
> Mike Rapoport fixed a check of RW attribute in lookup_address_in_pgd_attr()
> used for the verification of RWX:
>
> https://lore.kernel.org/all/20260715144519.934289-1-rppt@kernel.org
>
> Some of the fixes got merged into x86 tree, some of them got merged into mm
> tree and some are still hanging in the air.

:)))

>
> Beside the fixes there was a supposed simplification of cpa_lock locking
> that looked like removal of an optimization for DEBUG_PAGEALLOC, but it
> turned out that it was not an optimization but rather a correctness
> guard because with DEBUG_PAGEALLOC the locks could be taken in an atomic
> context and couldn't use plain spin_lock()/spin_unlock().
>
> The changes here are collected from all these fixes into a sinlge coherent
> set on top of tip/x86/mm:
>
> * update to cpa_lock handling with DEBUG_PAGEALLOC
> * fix for races between CPA and ptdumpi causing UAF
> * fix for stale page tables in IOMMU
> * update to the fix of the race between split and collapse of large
>   mappings
> * fix for effective RW computation in lookup_address_in_pgd_attr()

Thanks for this!

>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
> Lorenzo Stoakes (ARM) (3):
>       x86/mm/pat: acquire init_mm write lock on collapse to avoid UAF
>       x86/mm/pat: acquire init_mm read lock on attribute change to avoid UAF
>       x86/mm/pat: allocate split page tables as kernel page tables
>
> Mike Rapoport (Microsoft) (2):
>       x86/mm/pat: introcude cpa_lock() and cpa_unlock()
>       x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
>
>  arch/x86/mm/pat/set_memory.c | 95 +++++++++++++++++++++++++++++++-------------
>  include/linux/mmap_lock.h    |  2 +
>  2 files changed, 70 insertions(+), 27 deletions(-)
> ---
> base-commit: a5a162fe1ae130e3d2ceefef3f43afe3773c1d56
> change-id: 20260727-cpa-fixes-d3c73c075672
>
> --
> Sincerely yours,
> Mike.
>

Cheers, Lorenzo

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Lorenzo Stoakes (ARM)]

Typo in subject :) introcude -> introduce

Hopefully Dave can fix up easily.

On Tue, Jul 28, 2026 at 04:07:44PM +0300, Mike Rapoport (Microsoft) wrote:
> The splitting and merging of kernel page table mappings between small and
> large is protected by cpa_lock.
>
> Commit 5fce67641a3e ("x86/mm/pat: Don't gate cpa_lock on
> debug_pagealloc_enabled()") disabled gatig of cpa_lock on
> debug_pagealloc_enabled() to simplify the code presuming that skipping
> the lock when debug_pagealloc_enabled() was an optimization.
>
> However Lorenzo Stoakes notes that:
>
>   __kernel_map_pages() can be called from irq context:
>
>   < GFP_ATOMIC context >
>   kfree() or whatever
>   -> ...
>   -> __free_pages_prepare()
>   -> debug_pagealloc_unmap_pages()
>   -> __kernel_map_pages()
>   -> __change_page_attr_set_clr()
>   -> cpa_lock spins [irqs off]
>
>   So you're spin locking in irq context here, which is probably not a
>   good idea.
>
> It would be possible to unconditionally use spin_lock_irqsave() and
> spin_unlock_irqrestore() but that would complicate locking rules even
> more.
>
> Restore gating of cpa_lock of debug_pagealloc_enabled(), but instead of
> putting the open-coded condition
>
> 	if (debug_pagealloc_enabled())
>
> before every lock and unlock operation, wrap the condition and the
> locking operation into cpa_lock() and cpa_unlock() helpers.

Yeah seems sensible.

>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

LGTM, so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  arch/x86/mm/pat/set_memory.c | 33 +++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index b1e780a465b5..4c8922695fd3 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -65,8 +65,25 @@ static const int cpa_warn_level = CPA_PROTECT;
>   * Serialize cpa() using cpa_lock so that we don't allow any other cpu, with
>   * stale large tlb entries, to change the page attribute in parallel to some
>   * other cpu splitting a large page entry along with changing the attribute.
> + *
> + * When debug_pagealloc_enabled(), page attributes could be changed in atomic
> + * context that would warrant disabling IRQs. But since debug_pagealloc always
> + * uses 4k pages in the direct map there are no races for splits and collapses
> + * and locking can be just skipped altogether.
>   */
> -static DEFINE_SPINLOCK(cpa_lock);
> +static DEFINE_SPINLOCK(_cpa_lock);
> +
> +static inline void cpa_lock(void)
> +{
> +	if (!debug_pagealloc_enabled())
> +		spin_lock(&_cpa_lock);
> +}
> +
> +static inline void cpa_unlock(void)
> +{
> +	if (!debug_pagealloc_enabled())
> +		spin_unlock(&_cpa_lock);
> +}
>
>  #define CPA_FLUSHTLB 1
>  #define CPA_ARRAY 2
> @@ -417,7 +434,7 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
>  	int collapsed = 0;
>  	int i;
>
> -	spin_lock(&cpa_lock);
> +	cpa_lock();
>
>  	if (cpa->flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
>  		for (i = 0; i < cpa->numpages; i++)
> @@ -433,7 +450,7 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
>  	}
>
>  	if (!collapsed) {
> -		spin_unlock(&cpa_lock);
> +		cpa_unlock();
>  		return;
>  	}
>
> @@ -444,7 +461,7 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
>  		pagetable_free(ptdesc);
>  	}
>
> -	spin_unlock(&cpa_lock);
> +	cpa_unlock();
>  }
>
>  static void cpa_flush(struct cpa_data *cpa, int cache)
> @@ -1239,9 +1256,9 @@ static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
>  {
>  	struct ptdesc *ptdesc;
>
> -	spin_unlock(&cpa_lock);
> +	cpa_unlock();
>  	ptdesc = pagetable_alloc(GFP_KERNEL, 0);
> -	spin_lock(&cpa_lock);
> +	cpa_lock();
>  	if (!ptdesc)
>  		return -ENOMEM;
>
> @@ -2025,9 +2042,9 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int primary)
>  		if (cpa->flags & (CPA_ARRAY | CPA_PAGES_ARRAY))
>  			cpa->numpages = 1;
>
> -		spin_lock(&cpa_lock);
> +		cpa_lock();
>  		ret = __change_page_attr(cpa, primary);
> -		spin_unlock(&cpa_lock);
> +		cpa_unlock();
>  		if (ret)
>  			goto out;
>
>
> --
> 2.53.0
>

Cheers, Lorenzo

Re: [PATCH 3/5] x86/mm/pat: acquire init_mm read lock on attribute change to avoid UAF

[Lorenzo Stoakes (ARM)]

On Tue, Jul 28, 2026 at 04:07:46PM +0300, Mike Rapoport wrote:
> From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
>
> A previous commit protected us against races between ptdump and CPA
> collapse, however one still exists between attribute changes and collapse
> as reported by Denis V. Lunev (linked).
>
> When an attribute change arises, a lockless page table walker obtains a PTE
> entry, which is later written to via set_pte_atomic():
>
> ...
> -> change_page_attr_set_clr()
> -> __change_page_attr_set_clr()
> -> __change_page_attr()
> 	-> _lookup_address_cpa()
> 	-> lookup_address_in_pgd_attr()
> 	-> [ lockless page table walker ]
> -> set_pte_atomic()
>
> There is nothing preventing a concurrent CPA collapse which can free the
> PTE that was retrieved here, resulting in a use-after-free.
>
> With the mmap write lock taken on init_mm over CPA collapse, we can now
> resolve this race by acquiring an mmap read lock on init_mm over
> __change_page_attr_set_clr().
>
> This locks across the whole operation over which the walk and the PTE entry
> write occurs, solving the race.
>
> It is safe to do this here, as no spinlocks are held upon entry to
> __change_page_attr_set_clr().
>
> However, the lock must not be held over an allocation, as allocation can
> trigger reclaim and shrinkers may call into CPA recursively, making
> deadlocks possible (init_mm -> ... -> fs_reclaim -> init_mm).
>
> A page table is allocated when a huge page needs to be split:
>
> -> change_page_attr_set_clr()
> -> __change_page_attr_set_clr()
> -> __change_page_attr()
> -> split_large_page()
> [ pagetable_alloc() ]
> -> __split_large_page()
>
> Avoid deadlocks by dropping the mmap lock across pagetable_alloc() in
> split_large_page() and track whether this is needed by adding a new
> 'init_mm_read_locked' flag to struct cpa_data.
>
> This is safe as __split_large_page() (called with locks re-established)
> revalidates that the page table entry is the same as it was prior to the
> locks being dropped and __change_page_attr() repeats the entire page table
> walk whenever a split occurs, so concurrent split and collapse are
> accounted for.
>
> Concurrent ptdump is also safe as the lock is only dropped over page table
> allocation during which time the page table has not yet been modified.
>
> The CPA_COLLAPSE flag is only set by set_memory_rox(), which exclusively
> operates upon vmalloc ranges, and on x86 only within the module mapping
> space.
>
> This is important, because some callers directly invoke
> __change_page_attr_set_clr(), bypassing this lock. However, none of these
> operate within the module mapping space.
>
> * cpa_process_alias() - a recursive helper called by
>   __change_page_attr_set_clr().
> * __set_memory_enc_pgtable() - operates on the direct mapping and (via
>   __vmbus_establish_gpadl()) the vmalloc mapping space.
> * __set_pages_[n]p() - called by set_direct_map_[invalid, default,
>   valid]_noflush(), __kernel_map_pages() - operates on the direct map.
> * kernel_[un]map_pages_in_pgd() - operates on EFI ranges.
>
> This work is based upon Denis V. Lunev's excellent analysis of the bug with
> gratitude.
>
> Link: https://lore.kernel.org/all/20260626163213.2284080-1-den@openvz.org/
> Fixes: 41d88484c71c ("x86/mm/pat: restore large ROX pages after fragmentation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>  arch/x86/mm/pat/set_memory.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index 4ba16d72a535..26131ecc0e1c 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -50,7 +50,8 @@ struct cpa_data {
>  	unsigned int	flags;
>  	unsigned int	force_split		: 1,
>  			force_static_prot	: 1,
> -			force_flush_all		: 1;
> +			force_flush_all		: 1,
> +			init_mm_read_locked	: 1;
>  	struct page	**pages;
>  };
>
> @@ -435,8 +436,6 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
>  	int collapsed = 0;
>  	int i;
>
> -	cpa_lock();
> -
>  	if (cpa->flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
>  		for (i = 0; i < cpa->numpages; i++)
>  			collapsed += collapse_large_pages(__cpa_addr(cpa, i),
> @@ -450,10 +449,8 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
>  			collapsed += collapse_large_pages(addr, &pgtables);
>  	}
>
> -	if (!collapsed) {
> -		cpa_unlock();
> +	if (!collapsed)
>  		return;
> -	}
>
>  	flush_tlb_all();
>
> @@ -461,8 +458,6 @@ static void __cpa_collapse_large_pages(struct cpa_data *cpa)
>  		list_del(&ptdesc->pt_list);
>  		pagetable_free(ptdesc);
>  	}
> -
> -	cpa_unlock();
>  }
>
>  static void cpa_collapse_large_pages(struct cpa_data *cpa)
> @@ -1270,8 +1265,13 @@ static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
>  	struct ptdesc *ptdesc;
>
>  	cpa_unlock();
> +	if (cpa->init_mm_read_locked)
> +		mmap_read_unlock(&init_mm);
>  	ptdesc = pagetable_alloc(GFP_KERNEL, 0);
> +	if (cpa->init_mm_read_locked)
> +		mmap_read_lock(&init_mm);
>  	cpa_lock();

Yeah this retains the correct ordering (sem -> spinlock) so LGTM!

> +
>  	if (!ptdesc)
>  		return -ENOMEM;
>
> @@ -2139,7 +2139,11 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages,
>  	cpa.curpage = 0;
>  	cpa.force_split = force_split;
>
> -	ret = __change_page_attr_set_clr(&cpa, 1);
> +	/* Avoid race with concurrent CPA collapse. */
> +	cpa.init_mm_read_locked = true;
> +	scoped_guard(mmap_read_lock, &init_mm)
> +		ret = __change_page_attr_set_clr(&cpa, 1);
> +	cpa.init_mm_read_locked = false;
>
>  	/*
>  	 * Check whether we really changed something:
>
> --
> 2.53.0
>

Cheers, Lorenzo

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Peter Zijlstra]

On Tue, Jul 28, 2026 at 04:07:44PM +0300, Mike Rapoport (Microsoft) wrote:
> The splitting and merging of kernel page table mappings between small and
> large is protected by cpa_lock.
> 
> Commit 5fce67641a3e ("x86/mm/pat: Don't gate cpa_lock on
> debug_pagealloc_enabled()") disabled gatig of cpa_lock on
> debug_pagealloc_enabled() to simplify the code presuming that skipping
> the lock when debug_pagealloc_enabled() was an optimization.
> 
> However Lorenzo Stoakes notes that:
> 
>   __kernel_map_pages() can be called from irq context:
> 
>   < GFP_ATOMIC context >
>   kfree() or whatever
>   -> ...
>   -> __free_pages_prepare()
>   -> debug_pagealloc_unmap_pages()
>   -> __kernel_map_pages()
>   -> __change_page_attr_set_clr()
>   -> cpa_lock spins [irqs off]
> 
>   So you're spin locking in irq context here, which is probably not a
>   good idea.
> 
> It would be possible to unconditionally use spin_lock_irqsave() and
> spin_unlock_irqrestore() but that would complicate locking rules even
> more.
> 
> Restore gating of cpa_lock of debug_pagealloc_enabled(), but instead of
> putting the open-coded condition
> 
> 	if (debug_pagealloc_enabled())
> 
> before every lock and unlock operation, wrap the condition and the
> locking operation into cpa_lock() and cpa_unlock() helpers.
> 
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>  arch/x86/mm/pat/set_memory.c | 33 +++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index b1e780a465b5..4c8922695fd3 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -65,8 +65,25 @@ static const int cpa_warn_level = CPA_PROTECT;
>   * Serialize cpa() using cpa_lock so that we don't allow any other cpu, with
>   * stale large tlb entries, to change the page attribute in parallel to some
>   * other cpu splitting a large page entry along with changing the attribute.
> + *
> + * When debug_pagealloc_enabled(), page attributes could be changed in atomic
> + * context that would warrant disabling IRQs. But since debug_pagealloc always
> + * uses 4k pages in the direct map there are no races for splits and collapses
> + * and locking can be just skipped altogether.
>   */
> -static DEFINE_SPINLOCK(cpa_lock);
> +static DEFINE_SPINLOCK(_cpa_lock);
> +
> +static inline void cpa_lock(void)
> +{
> +	if (!debug_pagealloc_enabled())
> +		spin_lock(&_cpa_lock);
> +}
> +
> +static inline void cpa_unlock(void)
> +{
> +	if (!debug_pagealloc_enabled())
> +		spin_unlock(&_cpa_lock);
> +}

There was already a patch merged that removed the shole debug_pagealloc
exception. Is that not better?

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Dave Hansen]

On 7/28/26 07:21, Peter Zijlstra wrote:
> There was already a patch merged that removed the shole debug_pagealloc
> exception. Is that not better?

As I'm scanning through email this morning, there's another issue that
popped up with that patch. It's causing hangs on boot.

It's looking like debug pagealloc not taking the lock is actually
functional, not an optimization. Although, I hesitate to say
"functional" and would prefer to use much less nice words to describe it.

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Peter Zijlstra]

On Tue, Jul 28, 2026 at 07:30:27AM -0700, Dave Hansen wrote:
> On 7/28/26 07:21, Peter Zijlstra wrote:
> > There was already a patch merged that removed the shole debug_pagealloc
> > exception. Is that not better?
> 
> As I'm scanning through email this morning, there's another issue that
> popped up with that patch. It's causing hangs on boot.
> 
> It's looking like debug pagealloc not taking the lock is actually
> functional, not an optimization. Although, I hesitate to say
> "functional" and would prefer to use much less nice words to describe it.

Yeah, lets figure out why that is before we retain this wart ;-)

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Mike Rapoport]

On Tue, Jul 28, 2026 at 04:31:35PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 28, 2026 at 07:30:27AM -0700, Dave Hansen wrote:
> > On 7/28/26 07:21, Peter Zijlstra wrote:
> > > There was already a patch merged that removed the shole debug_pagealloc
> > > exception. Is that not better?
> > 
> > As I'm scanning through email this morning, there's another issue that
> > popped up with that patch. It's causing hangs on boot.
> > 
> > It's looking like debug pagealloc not taking the lock is actually
> > functional, not an optimization. Although, I hesitate to say
> > "functional" and would prefer to use much less nice words to describe it.
> 
> Yeah, lets figure out why that is before we retain this wart ;-)

As Lorenzo said:

  __kernel_map_pages() can be called from irq context:

  < GFP_ATOMIC context >
  kfree() or whatever
  -> ...
  -> __free_pages_prepare()
  -> debug_pagealloc_unmap_pages()
  -> __kernel_map_pages()
  -> __change_page_attr_set_clr()
  -> cpa_lock


-- 
Sincerely yours,
Mike.

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Lorenzo Stoakes (ARM)]

On Tue, Jul 28, 2026 at 05:46:30PM +0300, Mike Rapoport wrote:
> On Tue, Jul 28, 2026 at 04:31:35PM +0200, Peter Zijlstra wrote:
> > On Tue, Jul 28, 2026 at 07:30:27AM -0700, Dave Hansen wrote:
> > > On 7/28/26 07:21, Peter Zijlstra wrote:
> > > > There was already a patch merged that removed the shole debug_pagealloc
> > > > exception. Is that not better?
> > >
> > > As I'm scanning through email this morning, there's another issue that
> > > popped up with that patch. It's causing hangs on boot.
> > >
> > > It's looking like debug pagealloc not taking the lock is actually
> > > functional, not an optimization. Although, I hesitate to say
> > > "functional" and would prefer to use much less nice words to describe it.
> >
> > Yeah, lets figure out why that is before we retain this wart ;-)
>
> As Lorenzo said:
>
>   __kernel_map_pages() can be called from irq context:
>
>   < GFP_ATOMIC context >
>   kfree() or whatever
>   -> ...
>   -> __free_pages_prepare()
>   -> debug_pagealloc_unmap_pages()
>   -> __kernel_map_pages()
>   -> __change_page_attr_set_clr()
>   -> cpa_lock

Yup :)

Without the debug stuff you have this issue.

With it you don't...

Also without it Denis's patch is broken.

This series Mike's providing is fairly exhuastively the best way to fix the
various issues, grouped together and parted from mm to make everybody's life a
little easier...

>
>
> --
> Sincerely yours,
> Mike.

Cheers, Lorenzo

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Peter Zijlstra]

On Tue, Jul 28, 2026 at 05:46:30PM +0300, Mike Rapoport wrote:
> On Tue, Jul 28, 2026 at 04:31:35PM +0200, Peter Zijlstra wrote:
> > On Tue, Jul 28, 2026 at 07:30:27AM -0700, Dave Hansen wrote:
> > > On 7/28/26 07:21, Peter Zijlstra wrote:
> > > > There was already a patch merged that removed the shole debug_pagealloc
> > > > exception. Is that not better?
> > > 
> > > As I'm scanning through email this morning, there's another issue that
> > > popped up with that patch. It's causing hangs on boot.
> > > 
> > > It's looking like debug pagealloc not taking the lock is actually
> > > functional, not an optimization. Although, I hesitate to say
> > > "functional" and would prefer to use much less nice words to describe it.
> > 
> > Yeah, lets figure out why that is before we retain this wart ;-)
> 
> As Lorenzo said:
> 
>   __kernel_map_pages() can be called from irq context:
> 
>   < GFP_ATOMIC context >
>   kfree() or whatever
>   -> ...
>   -> __free_pages_prepare()
>   -> debug_pagealloc_unmap_pages()
>   -> __kernel_map_pages()
>   -> __change_page_attr_set_clr()
>   -> cpa_lock
> 

The TLBI hack in __kernel_map_pages() makes me wonder how any of this is
correct to begin with. That comment isn't helping.

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Peter Zijlstra]

On Tue, Jul 28, 2026 at 04:55:28PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 28, 2026 at 05:46:30PM +0300, Mike Rapoport wrote:
> > On Tue, Jul 28, 2026 at 04:31:35PM +0200, Peter Zijlstra wrote:
> > > On Tue, Jul 28, 2026 at 07:30:27AM -0700, Dave Hansen wrote:
> > > > On 7/28/26 07:21, Peter Zijlstra wrote:
> > > > > There was already a patch merged that removed the shole debug_pagealloc
> > > > > exception. Is that not better?
> > > > 
> > > > As I'm scanning through email this morning, there's another issue that
> > > > popped up with that patch. It's causing hangs on boot.
> > > > 
> > > > It's looking like debug pagealloc not taking the lock is actually
> > > > functional, not an optimization. Although, I hesitate to say
> > > > "functional" and would prefer to use much less nice words to describe it.
> > > 
> > > Yeah, lets figure out why that is before we retain this wart ;-)
> > 
> > As Lorenzo said:
> > 
> >   __kernel_map_pages() can be called from irq context:
> > 
> >   < GFP_ATOMIC context >
> >   kfree() or whatever
> >   -> ...
> >   -> __free_pages_prepare()
> >   -> debug_pagealloc_unmap_pages()
> >   -> __kernel_map_pages()
> >   -> __change_page_attr_set_clr()
> >   -> cpa_lock
> > 
> 
> The TLBI hack in __kernel_map_pages() makes me wonder how any of this is
> correct to begin with. That comment isn't helping.
> 

Also, the 'atomic context' usage hereabout seems confused. In particular
the issue is with IRQ-disabled context, they are not the same thing.

This confusion is seen in the existing comments in __set_pages_{p,np}()
and in this patch series. It needs fixing.

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Lorenzo Stoakes (ARM)]

On Tue, Jul 28, 2026 at 04:55:28PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 28, 2026 at 05:46:30PM +0300, Mike Rapoport wrote:
> > On Tue, Jul 28, 2026 at 04:31:35PM +0200, Peter Zijlstra wrote:
> > > On Tue, Jul 28, 2026 at 07:30:27AM -0700, Dave Hansen wrote:
> > > > On 7/28/26 07:21, Peter Zijlstra wrote:
> > > > > There was already a patch merged that removed the shole debug_pagealloc
> > > > > exception. Is that not better?
> > > >
> > > > As I'm scanning through email this morning, there's another issue that
> > > > popped up with that patch. It's causing hangs on boot.
> > > >
> > > > It's looking like debug pagealloc not taking the lock is actually
> > > > functional, not an optimization. Although, I hesitate to say
> > > > "functional" and would prefer to use much less nice words to describe it.
> > >
> > > Yeah, lets figure out why that is before we retain this wart ;-)
> >
> > As Lorenzo said:
> >
> >   __kernel_map_pages() can be called from irq context:
> >
> >   < GFP_ATOMIC context >
> >   kfree() or whatever
> >   -> ...
> >   -> __free_pages_prepare()
> >   -> debug_pagealloc_unmap_pages()
> >   -> __kernel_map_pages()
> >   -> __change_page_attr_set_clr()
> >   -> cpa_lock
> >
>
> The TLBI hack in __kernel_map_pages() makes me wonder how any of this is
> correct to begin with. That comment isn't helping.
>

I think there's probably more horror shows there, it's a case of put out the
most immediate fires, leave the probably remaining flaming trash pile for later
;)

I thought x86 allowing arbitrary mm ptdump walks was crazy enough but this debug
pagealloc stuff kinda pushes things further crazy-wise...

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Peter Zijlstra]

On Tue, Jul 28, 2026 at 04:07:44PM +0300, Mike Rapoport (Microsoft) wrote:

>  arch/x86/mm/pat/set_memory.c | 33 +++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index b1e780a465b5..4c8922695fd3 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -65,8 +65,25 @@ static const int cpa_warn_level = CPA_PROTECT;
>   * Serialize cpa() using cpa_lock so that we don't allow any other cpu, with
>   * stale large tlb entries, to change the page attribute in parallel to some
>   * other cpu splitting a large page entry along with changing the attribute.
> + *
> + * When debug_pagealloc_enabled(), page attributes could be changed in atomic
> + * context that would warrant disabling IRQs. But since debug_pagealloc always
> + * uses 4k pages in the direct map there are no races for splits and collapses
> + * and locking can be just skipped altogether.
>   */
> -static DEFINE_SPINLOCK(cpa_lock);
> +static DEFINE_SPINLOCK(_cpa_lock);
> +
> +static inline void cpa_lock(void)
> +{
> +	if (!debug_pagealloc_enabled())
> +		spin_lock(&_cpa_lock);
> +}
> +
> +static inline void cpa_unlock(void)
> +{
> +	if (!debug_pagealloc_enabled())
> +		spin_unlock(&_cpa_lock);
> +}

If this lives, we should probably stick an assertion in both the split
and alloc cases for holding _cpa_lock. That debug thing seems to rely on
never hitting those, but having that be implicit is asking for pain.

Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Lorenzo Stoakes (ARM)]

On Tue, Jul 28, 2026 at 05:01:26PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 28, 2026 at 04:55:28PM +0200, Peter Zijlstra wrote:
> > On Tue, Jul 28, 2026 at 05:46:30PM +0300, Mike Rapoport wrote:
> > > On Tue, Jul 28, 2026 at 04:31:35PM +0200, Peter Zijlstra wrote:
> > > > On Tue, Jul 28, 2026 at 07:30:27AM -0700, Dave Hansen wrote:
> > > > > On 7/28/26 07:21, Peter Zijlstra wrote:
> > > > > > There was already a patch merged that removed the shole debug_pagealloc
> > > > > > exception. Is that not better?
> > > > >
> > > > > As I'm scanning through email this morning, there's another issue that
> > > > > popped up with that patch. It's causing hangs on boot.
> > > > >
> > > > > It's looking like debug pagealloc not taking the lock is actually
> > > > > functional, not an optimization. Although, I hesitate to say
> > > > > "functional" and would prefer to use much less nice words to describe it.
> > > >
> > > > Yeah, lets figure out why that is before we retain this wart ;-)
> > >
> > > As Lorenzo said:
> > >
> > >   __kernel_map_pages() can be called from irq context:
> > >
> > >   < GFP_ATOMIC context >
> > >   kfree() or whatever
> > >   -> ...
> > >   -> __free_pages_prepare()
> > >   -> debug_pagealloc_unmap_pages()
> > >   -> __kernel_map_pages()
> > >   -> __change_page_attr_set_clr()
> > >   -> cpa_lock
> > >
> >
> > The TLBI hack in __kernel_map_pages() makes me wonder how any of this is
> > correct to begin with. That comment isn't helping.
> >
>
> Also, the 'atomic context' usage hereabout seems confused. In particular
> the issue is with IRQ-disabled context, they are not the same thing.

I mean yeah the IRQs being off is the issue with holding the lock over an IPI, a
softirq allocating GFP_ATOMIC won't be a problem for that, but I think all the
conclusions are still the same.

>
> This confusion is seen in the existing comments in __set_pages_{p,np}()
> and in this patch series. It needs fixing.