[PATCH] x86/alternative: delay freeing of smp_locks section

Mike Rapoport posted 1 patch 5 days, 4 hours ago
There is a newer version of this series
arch/x86/kernel/alternative.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
[PATCH] x86/alternative: delay freeing of smp_locks section
Posted by Mike Rapoport 5 days, 4 hours ago
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

On UP systems alternative_instructions() frees memory occupied by smp_locks
section immediately after patching the lock instructions.

With CONFIG_DEFERRED_STRUCT_PAGE_INIT enabled this happens before the
memory map is fully initialized and the struct pages representing the freed
memory might get overwritten by deferred initialization of the memory map.

Move freeing of smp_locks section to an initcall to ensure it will happen
after the memory map is fully initialized.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Tested-By: Bert Karwatzki <spasswolf@web.de>
---
 arch/x86/kernel/alternative.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index e87da25d1236..62936a3bde19 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2448,19 +2448,31 @@ void __init alternative_instructions(void)
 					    __smp_locks, __smp_locks_end,
 					    _text, _etext);
 	}
+#endif
 
+	restart_nmi();
+	alternatives_patched = 1;
+
+	alt_reloc_selftest();
+}
+
+#ifdef CONFIG_SMP
+/*
+ * With CONFIG_DEFERRED_STRUCT_PAGE_INIT enabled we can free_init_pages() only
+ * after the deferred initialization of the memory map is complete.
+ */
+static int __init free_smp_locks(void)
+{
 	if (!uniproc_patched || num_possible_cpus() == 1) {
 		free_init_pages("SMP alternatives",
 				(unsigned long)__smp_locks,
 				(unsigned long)__smp_locks_end);
 	}
-#endif
 
-	restart_nmi();
-	alternatives_patched = 1;
-
-	alt_reloc_selftest();
+	return 0;
 }
+arch_initcall(free_smp_locks);
+#endif
 
 /**
  * text_poke_early - Update instructions on a live kernel at boot time

base-commit: e77a5a5cfe43b4c25bd44a3818e487033287517f
-- 
2.53.0
Re: [PATCH] x86/alternative: delay freeing of smp_locks section
Posted by Borislav Petkov 5 days ago
On Sat, Mar 28, 2026 at 11:16:34AM +0300, Mike Rapoport wrote:
> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> 
> On UP systems alternative_instructions() frees memory occupied by smp_locks

UP systems?

I don't understand - Bert's machine is a SMP.

> section immediately after patching the lock instructions.
> 
> With CONFIG_DEFERRED_STRUCT_PAGE_INIT enabled this happens before the
> memory map is fully initialized and the struct pages representing the freed
> memory might get overwritten by deferred initialization of the memory map.
> 
> Move freeing of smp_locks section to an initcall to ensure it will happen
> after the memory map is fully initialized.
> 
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Tested-By: Bert Karwatzki <spasswolf@web.de>

I don't understand even more: why have we not hit this before?

No Fixes: tag?

Something must've changed for this to fire...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86/alternative: delay freeing of smp_locks section
Posted by Mike Rapoport 5 days ago
On Sat, Mar 28, 2026 at 12:41:18PM +0100, Borislav Petkov wrote:
> On Sat, Mar 28, 2026 at 11:16:34AM +0300, Mike Rapoport wrote:
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> > 
> > On UP systems alternative_instructions() frees memory occupied by smp_locks
> 
> UP systems?
> 
> I don't understand - Bert's machine is a SMP.

Argh, I misread the 'if (!uniproc_patched' :(
 
> > section immediately after patching the lock instructions.
> > 
> > With CONFIG_DEFERRED_STRUCT_PAGE_INIT enabled this happens before the
> > memory map is fully initialized and the struct pages representing the freed
> > memory might get overwritten by deferred initialization of the memory map.
> > 
> > Move freeing of smp_locks section to an initcall to ensure it will happen
> > after the memory map is fully initialized.
> > 
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > Tested-By: Bert Karwatzki <spasswolf@web.de>
> 
> I don't understand even more: why have we not hit this before?

That memory was never actually freed, it remained reserved because
free_init_pages() calls free_reserved_area() but does not update memblock.
 
> No Fixes: tag?

It's as old as CONFIG_DEFERRED_STRUCT_PAGE_INIT (v4.2) or even before that.
If you think that fixing this leak is important enough to backport, it
affects all mainlined stable releases.

> Something must've changed for this to fire...

Yes, I added a WARN() in free_reserved_area() to lure such cases and
prevent them in the future.

I'll wait a bit for more comments before rewriting changelog and reposting.

> -- 
> Regards/Gruss,
>     Boris.

-- 
Sincerely yours,
Mike.
Re: [PATCH] x86/alternative: delay freeing of smp_locks section
Posted by Borislav Petkov 4 days, 16 hours ago
On Sat, Mar 28, 2026 at 03:39:15PM +0300, Mike Rapoport wrote:
> It's as old as CONFIG_DEFERRED_STRUCT_PAGE_INIT (v4.2) or even before that.
> If you think that fixing this leak is important enough to backport, it
> affects all mainlined stable releases.

Probably not all but perhaps the last two - 6.12 and 6.18...

> Yes, I added a WARN() in free_reserved_area() to lure such cases and
> prevent them in the future.

This warn is supposed to catch the leaks. I guess it is important enough to
add the warning so it is just as important to fix them leaks, right :-)

As to the fix itself, arch_initcall() is the magic time where deferred
initialization is complete, I presume?

> I'll wait a bit for more comments before rewriting changelog and reposting.

Yes please. You could also expand on why the arch initcall is the proper fix.
I guess deferred init completes with an earlier initcall. So yeah, it would be
good to have that written down explicitly too.

On and pls use those tags:

Tested-by: Bert Karwatzki <spasswolf@web.de>
Reported-by: Bert Karwatzki <spasswolf@web.de>
Link: https://lore.kernel.org/r/20260327140109.7561-1-spasswolf@web.de

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86/alternative: delay freeing of smp_locks section
Posted by Mike Rapoport 2 days, 19 hours ago
On Sat, Mar 28, 2026 at 08:58:27PM +0100, Borislav Petkov wrote:
> On Sat, Mar 28, 2026 at 03:39:15PM +0300, Mike Rapoport wrote:
> > It's as old as CONFIG_DEFERRED_STRUCT_PAGE_INIT (v4.2) or even before that.
> > If you think that fixing this leak is important enough to backport, it
> > affects all mainlined stable releases.
> 
> Probably not all but perhaps the last two - 6.12 and 6.18...
> 
> > Yes, I added a WARN() in free_reserved_area() to lure such cases and
> > prevent them in the future.
> 
> This warn is supposed to catch the leaks. I guess it is important enough to
> add the warning so it is just as important to fix them leaks, right :-)

After sleeping on it, I realised there was no leak, and this is only an
integration issue of recent memblock changes, so there's no need for cc
@stable.

Since this fixes a fallout from memblock changes, I can take it together
with them via memblock tree with Ack from x86 folks if you prefer.
 
> As to the fix itself, arch_initcall() is the magic time where deferred
> initialization is complete, I presume?
> 
> > I'll wait a bit for more comments before rewriting changelog and reposting.
> 
> Yes please. You could also expand on why the arch initcall is the proper fix.
> I guess deferred init completes with an earlier initcall. So yeah, it would be
> good to have that written down explicitly too.
> 
> On and pls use those tags:
> 
> Tested-by: Bert Karwatzki <spasswolf@web.de>
> Reported-by: Bert Karwatzki <spasswolf@web.de>
> Link: https://lore.kernel.org/r/20260327140109.7561-1-spasswolf@web.de
> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

-- 
Sincerely yours,
Mike.
Re: [PATCH] x86/alternative: delay freeing of smp_locks section
Posted by Borislav Petkov 2 days, 18 hours ago
On Mon, Mar 30, 2026 at 08:16:32PM +0300, Mike Rapoport wrote:
> Since this fixes a fallout from memblock changes, I can take it together
> with them via memblock tree with Ack from x86 folks if you prefer.

Sure, once you send me a patch with a proper explanation which I can ack... :)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86/alternative: delay freeing of smp_locks section
Posted by Peter Zijlstra 2 days, 17 hours ago
On Mon, Mar 30, 2026 at 07:44:00PM +0200, Borislav Petkov wrote:
> On Mon, Mar 30, 2026 at 08:16:32PM +0300, Mike Rapoport wrote:
> > Since this fixes a fallout from memblock changes, I can take it together
> > with them via memblock tree with Ack from x86 folks if you prefer.
> 
> Sure, once you send me a patch with a proper explanation which I can ack... :)

Another silly question; why not delete the whole SMP locks thing? I
mean, who cares about UP these days?
Re: [PATCH] x86/alternative: delay freeing of smp_locks section
Posted by H. Peter Anvin 2 days, 16 hours ago
On March 30, 2026 12:36:25 PM PDT, Peter Zijlstra <peterz@infradead.org> wrote:
>On Mon, Mar 30, 2026 at 07:44:00PM +0200, Borislav Petkov wrote:
>> On Mon, Mar 30, 2026 at 08:16:32PM +0300, Mike Rapoport wrote:
>> > Since this fixes a fallout from memblock changes, I can take it together
>> > with them via memblock tree with Ack from x86 folks if you prefer.
>> 
>> Sure, once you send me a patch with a proper explanation which I can ack... :)
>
>Another silly question; why not delete the whole SMP locks thing? I
>mean, who cares about UP these days?

Specifically, who cares about *running SMP kernels on UP?*

Compiling for UP explicitly is an entirely different matter.
Re: [PATCH] x86/alternative: delay freeing of smp_locks section
Posted by Borislav Petkov 2 days, 16 hours ago
On Mon, Mar 30, 2026 at 12:43:04PM -0700, H. Peter Anvin wrote:
> >Another silly question; why not delete the whole SMP locks thing? I
> >mean, who cares about UP these days?
> 
> Specifically, who cares about *running SMP kernels on UP?*
> 
> Compiling for UP explicitly is an entirely different matter.

Yah, let's zap them.

That close to the merge window maybe not but something for 7.2 perhaps...

So patches are welcome.

:-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette