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

Thanks,

If Mike's going to respin worth examining this.

Let me paste in the attached patch to make life easier:

>From time to time, the following BUG can be observed[0]:
>
>> kernel BUG at arch/x86/kernel/alternative.c:2576!
>> Oops: invalid opcode: 0000 [#1] SMP NOPTI
>> CPU: 0 UID: 0 PID: 355 Comm: (udev-worker) Not tainted 7.1.3-1-default #1 PREEMPT(full) openSUSE Tumbleweed  8c1795b03ec64f997e57a8ad38b1161e3b98da64
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS unknown 02/02/2022
>> RIP: 0010:__text_poke+0x2aa/0x450
>> Call Trace:
>>  <TASK>
>>  smp_text_poke_batch_finish+0x2a7/0x320
>>  __static_call_transform+0xb7/0x220
>>  arch_static_call_transform+0x5b/0xb0
>>  __static_call_init+0xe9/0x270
>>  static_call_module_notify+0x11f/0x150
>>  notifier_call_chain+0x61/0xe0
>>  blocking_notifier_call_chain_robust+0x63/0xc0
>>  load_module+0x1c92/0x20c0
>>  init_module_from_file+0xd8/0x140
>>  idempotent_init_module+0x100/0x2f0
>>  __x64_sys_finit_module+0x71/0xe0
>>  do_syscall_64+0xe1/0x610
>>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
>which matches the following BUG_ON in alternative.c:
>	/*
>	 * If something went wrong, crash and burn since recovery paths are not
>	 * implemented.
>	 */
>	BUG_ON(!pages[0] || (cross_page_boundary && !pages[1]));

Ugh yeah, it seems the CPA code is just teeming with this kind of thing.

>
>This can happen if vmalloc_to_page() fails, for any reason. Such can happen
>if text poking races with CPA, which can possibly result in the collapsing
>of page tables (or breaking of PMD hugepages). It is not a problem for most
>users of vmalloc_to_page() (they solely own the vmalloc'd range) but, when
>CONFIG_ARCH_HAS_EXECMEM_ROX=y, various modules own a single execmem vmalloc
>range, and can call set_memory_*() in parallel on it. This can happen to
>race against __text_poke and cause havoc in vmalloc_to_page().
>
>Fix it by excluding against CPA using the init_mm mmap read lock.
>
>Fixes: 64f6a4e10c05 ("x86: re-enable EXECMEM_ROX support")

You'd need to somehow state the dependency on commit "x86/mm/pat: acquire
init_mm write lock on collapse to avoid UAF" from this series because this
depends on that.

That only goes back to commit 41d88484c71c ("x86/mm/pat: restore large ROX pages
after fragmentation") so you'd need to somehow indicate the backport would need
to port my change even further back...

>Reported-by: Jiri Slaby <[email protected]>
>Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1271202 [0]
>Reported-by: Steffen Dirkwinkel <[email protected]>
>Link: https://lore.kernel.org/linux-mm/[email protected]/
>Cc: [email protected]
>Signed-off-by: Pedro Falcato <[email protected]>
>---
> arch/x86/kernel/alternative.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>index 62936a3bde19..9071eb870eab 100644
>--- a/arch/x86/kernel/alternative.c
>+++ b/arch/x86/kernel/alternative.c

Should include cleanup.h.

>@@ -2559,6 +2559,14 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
> 	 */
> 	BUG_ON(!after_bootmem);
>
>+	/*
>+	 * Exclude against change_page_attr() collapse in execmem ROX regions.
>+	 * These are PMD sized and this module may not own the whole PMD,
>+	 * thus breakdown/collapse may happen at any moment by concurrent module
>+	 * loading, which races with vmalloc_to_page().
>+	 */

Worth saying what this pairs with. Right now the comment doesn't really explain
why you're taking this lock.

>+	guard(mmap_read_lock)(&init_mm);

This is problematic.

text_poke_kgdb() -> __text_poke() can be called from pretty much any
context it seems. It's another debug_pagealloc type situation :)

Claude tells me there's a in_dbg_master() variable you can check to avoid this
and all other CPUs are stopped when it does this so it's safe anyway.

>+
> 	if (!core_kernel_text((unsigned long)addr)) {
> 		pages[0] = vmalloc_to_page(addr);
> 		if (cross_page_boundary)
>--
>2.55.0
>

I attach a patch (hand-written :) that addresses all this. Feel free to use it
as you like.

Cheers, Lorenzo

----8<----
From 2759f0ea4457e572c2972c7e5a0bbb39f2a9a2e5 Mon Sep 17 00:00:00 2001
From: "Lorenzo Stoakes (ARM)" <[email protected]>
Date: Fri, 7 Aug 2026 16:23:40 +0100
Subject: [PATCH] fix

---
 arch/x86/kernel/alternative.c | 39 ++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 62936a3bde19..cafcac95e90e 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -6,6 +6,9 @@
 #include <linux/vmalloc.h>
 #include <linux/memory.h>
 #include <linux/execmem.h>
+#include <linux/cleanup.h>
+#include <linux/kgdb.h>
+#include <linux/mmap_lock.h>

 #include <asm/text-patching.h>
 #include <asm/insn.h>
@@ -2543,6 +2546,30 @@ static void text_poke_memset(void *dst, const void *src, size_t len)

 typedef void text_poke_f(void *dst, const void *src, size_t len);

+static void poke_vmalloc_pages(struct page **pages, void *addr,
+			       bool cross_page_boundary)
+{
+	pages[0] = vmalloc_to_page(addr);
+	if (cross_page_boundary)
+		pages[1] = vmalloc_to_page(addr + PAGE_SIZE);
+}
+
+static void poke_vmalloc_pages_safe(struct page **pages, void *addr,
+				    bool cross_page_boundary)
+{
+	/*
+	 * execmem ROX ranges are shared between modules and can be collapsed to
+	 * huge PMD entries, and this collapse can happen concurrently with a
+	 * racing set_memory_rox().
+	 *
+	 * Prevent vmalloc_to_page() from racing by acquiring an init_mm read
+	 * lock which pairs with the init_mm write lock in
+	 * cpa_collapse_large_pages().
+	 */
+	guard(mmap_read_lock)(&init_mm);
+	poke_vmalloc_pages(pages, addr, cross_page_boundary);
+}
+
 static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t len)
 {
 	bool cross_page_boundary = offset_in_page(addr) + len > PAGE_SIZE;
@@ -2560,9 +2587,15 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
 	BUG_ON(!after_bootmem);

 	if (!core_kernel_text((unsigned long)addr)) {
-		pages[0] = vmalloc_to_page(addr);
-		if (cross_page_boundary)
-			pages[1] = vmalloc_to_page(addr + PAGE_SIZE);
+		/*
+		 * If called from kgdb cannot sleep, but all other CPUs stopped
+		 * anyway so safe.
+		 */
+		if (in_dbg_master())
+			poke_vmalloc_pages(pages, addr, cross_page_boundary);
+		else
+			poke_vmalloc_pages_safe(pages, addr,
+						cross_page_boundary);
 	} else {
 		pages[0] = virt_to_page(addr);
 		WARN_ON(!PageReserved(pages[0]));