The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Pedro Falcato <pfalcato@suse.de>
To: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>,
	 "Mike Rapoport (Microsoft)" <rppt@kernel.org>
Cc: Steffen Dirkwinkel <lists@steffen.cc>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	"Denis V. Lunev" <den@openvz.org>,
	 Andrew Morton <akpm@linux-foundation.org>,
	Andy Lutomirski <luto@kernel.org>,
	 Borislav Petkov <bp@alien8.de>,
	David CARLIER <devnexen@gmail.com>,
	 David Hildenbrand <david@kernel.org>,
	Ingo Molnar <mingo@redhat.com>, Jason Gunthorpe <jgg@ziepe.ca>,
	 Juergen Gross <jgross@suse.com>,
	Kevin Tian <kevin.tian@intel.com>,
	 Kiryl Shutsemau <kas@kernel.org>,
	"Liam R. Howlett" <liam@infradead.org>,
	 Lu Baolu <baolu.lu@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	 Peter Zijlstra <peterz@infradead.org>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	 Suren Baghdasaryan <surenb@google.com>,
	Thomas Gleixner <tglx@kernel.org>,
	 Toshi Kani <toshi.kani@hpe.com>,
	Vishal Moola <vishal.moola@gmail.com>,
	 Vlastimil Babka <vbabka@kernel.org>,
	Will Deacon <will@kernel.org>,
	iommu@lists.linux.dev,  linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, stable@vger.kernel.org, x86@kernel.org,
	 syzbot@syzkaller.appspotmail.com,
	Jiri Slaby <jirislaby@kernel.org>
Subject: Re: [PATCH 0/5] x86/mm/pat: CPA fixes
Date: Wed, 12 Aug 2026 12:46:09 +0100	[thread overview]
Message-ID: <anxbSdxDlyq__-T1@pedro-suse.lan> (raw)
In-Reply-To: <anXvODEvV4f_Taj7@lucifer>

On Fri, Aug 07, 2026 at 04:36:08PM +0100, Lorenzo Stoakes (ARM) wrote:
> Thanks,
> 
> If Mike's going to respin worth examining this.

Thanks for taking a look!

> 
> 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.

Yeah, it's supposed to be queued up on top of the series.
> 
> 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...

Hmm, I think this is the correct Fixes:...

> 
> >Reported-by: Jiri Slaby <jirislaby@kernel.org>
> >Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1271202 [0]
> >Reported-by: Steffen Dirkwinkel <lists@steffen.cc>
> >Link: https://lore.kernel.org/linux-mm/555ea1d43a12c30a8f1eaf10c899b3790d728f33.camel@dirkwinkel.cc/
> >Cc: stable@vger.kernel.org
> >Signed-off-by: Pedro Falcato <pfalcato@suse.de>
> >---
> > 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

Artisanal!

> as you like.
> 
> Cheers, Lorenzo
> 
> ----8<----
> From 2759f0ea4457e572c2972c7e5a0bbb39f2a9a2e5 Mon Sep 17 00:00:00 2001
> From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
> 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]));

Hmm, yeah, this looks More Correct(tm), thanks!

(for the record, after this email I tried exploring if we could feasibly
Just Hold the mmap lock on callers, but it becomes messy quite quickly)

Mike, if you get to queue up my patch then please replace the diff with
Lorenzo's (with a Co-developed-by:, I guess, since he went to the trouble
of moving quite a few things around). Otherwise, I'll figure out how to
re-submit it after your series lands.

-- 
Pedro

  reply	other threads:[~2026-08-12 11:46 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 13:07 [PATCH 0/5] x86/mm/pat: CPA fixes Mike Rapoport (Microsoft)
2026-07-28 13:07 ` [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock() Mike Rapoport (Microsoft)
2026-07-28 13:13   ` Lorenzo Stoakes (ARM)
2026-07-28 14:21   ` Peter Zijlstra
2026-07-28 14:30     ` Dave Hansen
2026-07-28 14:31       ` Peter Zijlstra
2026-07-28 14:46         ` Mike Rapoport
2026-07-28 14:50           ` Lorenzo Stoakes (ARM)
2026-07-28 14:55           ` Peter Zijlstra
2026-07-28 15:01             ` Peter Zijlstra
2026-07-28 15:20               ` Lorenzo Stoakes (ARM)
2026-07-28 15:33                 ` Peter Zijlstra
2026-07-28 15:54                   ` Mike Rapoport
2026-07-28 15:02             ` Lorenzo Stoakes (ARM)
2026-07-28 15:30             ` Peter Zijlstra
2026-07-28 15:16   ` Peter Zijlstra
2026-07-28 16:01     ` Mike Rapoport
2026-07-28 13:07 ` [PATCH 2/5] x86/mm/pat: acquire init_mm write lock on collapse to avoid UAF Mike Rapoport
2026-07-28 13:07 ` [PATCH 3/5] x86/mm/pat: acquire init_mm read lock on attribute change " Mike Rapoport
2026-07-28 13:14   ` Lorenzo Stoakes (ARM)
2026-07-28 13:07 ` [PATCH 4/5] x86/mm/pat: allocate split page tables as kernel page tables Mike Rapoport
2026-07-28 13:07 ` [PATCH 5/5] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr() Mike Rapoport (Microsoft)
2026-07-28 13:11 ` [PATCH 0/5] x86/mm/pat: CPA fixes Lorenzo Stoakes (ARM)
2026-07-30 15:53 ` Steffen Dirkwinkel
2026-08-03 12:41   ` Pedro Falcato
2026-08-07 15:36     ` Lorenzo Stoakes (ARM)
2026-08-12 11:46       ` Pedro Falcato [this message]
2026-08-12 15:15         ` Mike Rapoport
2026-08-12 15:42           ` Lorenzo Stoakes (ARM)
2026-08-03 15:14 ` Mike Rapoport
2026-08-07 14:34   ` Lorenzo Stoakes (ARM)
2026-08-07 15:00     ` Lorenzo Stoakes (ARM)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=anxbSdxDlyq__-T1@pedro-suse.lan \
    --to=pfalcato@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=baolu.lu@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@kernel.org \
    --cc=den@openvz.org \
    --cc=devnexen@gmail.com \
    --cc=hpa@zytor.com \
    --cc=iommu@lists.linux.dev \
    --cc=jgg@ziepe.ca \
    --cc=jgross@suse.com \
    --cc=jirislaby@kernel.org \
    --cc=kas@kernel.org \
    --cc=kevin.tian@intel.com \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lists@steffen.cc \
    --cc=ljs@kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rppt@kernel.org \
    --cc=shakeel.butt@linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=syzbot@syzkaller.appspotmail.com \
    --cc=tglx@kernel.org \
    --cc=toshi.kani@hpe.com \
    --cc=vbabka@kernel.org \
    --cc=vishal.moola@gmail.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox