* [PATCH] um: mprotect() __init memory @ 2026-09-21 10:29 Johannes Berg 2026-09-26 2:12 ` Hajime Tazaki 0 siblings, 1 reply; 4+ messages in thread From: Johannes Berg @ 2026-09-21 10:29 UTC (permalink / raw) To: linux-um; +Cc: Johannes Berg From: Johannes Berg <johannes.berg@intel.com> Unlike what the comment says, we could munmap() this (but not reuse it for guest allocations), but then stray libc allocations could technically conflict, so mprotect() it to catch access bugs. Align it in the linker scripts too so that all of it can be covered, not just some. Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- arch/um/kernel/dyn.lds.S | 2 ++ arch/um/kernel/mem.c | 11 ++++++++--- arch/um/kernel/uml.lds.S | 2 ++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S index ad3cefeff2ac..5d3d5ef6ebec 100644 --- a/arch/um/kernel/dyn.lds.S +++ b/arch/um/kernel/dyn.lds.S @@ -98,8 +98,10 @@ SECTIONS #include <asm/common.lds.S> + . = ALIGN(PAGE_SIZE); __init_begin = .; init.data : { INIT_DATA } + . = ALIGN(PAGE_SIZE); __init_end = .; /* Ensure the __preinit_array_start label is properly aligned. We diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c index 1eef0e42ef5d..00c469fd28ee 100644 --- a/arch/um/kernel/mem.c +++ b/arch/um/kernel/mem.c @@ -83,12 +83,17 @@ void __init arch_zone_limits_init(unsigned long *max_zone_pfns) } /* - * This can't do anything because nothing in the kernel image can be freed - * since it's not in kernel physical memory. + * We could munmap() this instead, but then libc allocations could + * land in this area and stray initdata access could erroneosly + * succeeded - just mprotect() it to reliably catch bad accesses. */ - void free_initmem(void) { + unsigned long start = PAGE_ALIGN((unsigned long)__init_begin); + unsigned long end = round_down((unsigned long)__init_end, PAGE_SIZE); + + if (end > start) + os_protect_memory((void *)start, end - start, 0, 0, 0); } /* Allocate and free page tables. */ diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S index 30aa24348d60..7085ba6fcb93 100644 --- a/arch/um/kernel/uml.lds.S +++ b/arch/um/kernel/uml.lds.S @@ -70,8 +70,10 @@ SECTIONS #include <asm/common.lds.S> + . = ALIGN(PAGE_SIZE); __init_begin = .; init.data : { INIT_DATA } + . = ALIGN(PAGE_SIZE); __init_end = .; .data : -- 2.55.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] um: mprotect() __init memory 2026-09-21 10:29 [PATCH] um: mprotect() __init memory Johannes Berg @ 2026-09-26 2:12 ` Hajime Tazaki 2026-09-26 9:16 ` Johannes Berg 0 siblings, 1 reply; 4+ messages in thread From: Hajime Tazaki @ 2026-09-26 2:12 UTC (permalink / raw) To: johannes; +Cc: linux-um, johannes.berg Hello, I recently pulled uml/next branch and faced an issue on the exit. ``` Thread 1 "vmlinux" received signal SIGSEGV, Segmentation fault. 0x0000000060003cf3 in start_uml () at ../arch/um/kernel/skas/process.c:41 41 } (gdb) bt #0 0x0000000060003cf3 in start_uml () at ../arch/um/kernel/skas/process.c:41 #1 0x0000000060003a69 in linux_main (argc=argc@entry=9, argv=argv@entry=0x7fffffffe118, envp=envp@entry=0x7fffffffe168) at ../arch/um/kernel/um_arch.c:404 #2 0x00000000600049dc in main (argc=9, argv=0x7fffffffe118, envp=0x7fffffffe168) at ../arch/um/os-Linux/main.c:153 ``` and bisected that this commit is the first rev to introduce this. indeed, now .text section is in __init label but some of startup code (start_uml, linux_main) remain un-returned even after init is done. a quick change below avoid this issue, but not sure if it is your intention of the original patch. --- a/arch/um/kernel/mem.c +++ b/arch/um/kernel/mem.c @@ -93,7 +93,7 @@ void free_initmem(void) unsigned long end = round_down((unsigned long)__init_end, PAGE_SIZE); if (end > start) - os_protect_memory((void *)start, end - start, 0, 0, 0); + os_protect_memory((void *)start, end - start, 0, 0, 1); } I guess you're already aware of it (if you boot and halt a UML instance it should be 100% reproducible), but in case not. -- Hajime On Mon, 21 Sep 2026 19:29:37 +0900, Johannes Berg wrote: > > From: Johannes Berg <johannes.berg@intel.com> > > Unlike what the comment says, we could munmap() this (but > not reuse it for guest allocations), but then stray libc > allocations could technically conflict, so mprotect() it > to catch access bugs. Align it in the linker scripts too > so that all of it can be covered, not just some. > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > arch/um/kernel/dyn.lds.S | 2 ++ > arch/um/kernel/mem.c | 11 ++++++++--- > arch/um/kernel/uml.lds.S | 2 ++ > 3 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S > index ad3cefeff2ac..5d3d5ef6ebec 100644 > --- a/arch/um/kernel/dyn.lds.S > +++ b/arch/um/kernel/dyn.lds.S > @@ -98,8 +98,10 @@ SECTIONS > > #include <asm/common.lds.S> > > + . = ALIGN(PAGE_SIZE); > __init_begin = .; > init.data : { INIT_DATA } > + . = ALIGN(PAGE_SIZE); > __init_end = .; > > /* Ensure the __preinit_array_start label is properly aligned. We > diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c > index 1eef0e42ef5d..00c469fd28ee 100644 > --- a/arch/um/kernel/mem.c > +++ b/arch/um/kernel/mem.c > @@ -83,12 +83,17 @@ void __init arch_zone_limits_init(unsigned long *max_zone_pfns) > } > > /* > - * This can't do anything because nothing in the kernel image can be freed > - * since it's not in kernel physical memory. > + * We could munmap() this instead, but then libc allocations could > + * land in this area and stray initdata access could erroneosly > + * succeeded - just mprotect() it to reliably catch bad accesses. > */ > - > void free_initmem(void) > { > + unsigned long start = PAGE_ALIGN((unsigned long)__init_begin); > + unsigned long end = round_down((unsigned long)__init_end, PAGE_SIZE); > + > + if (end > start) > + os_protect_memory((void *)start, end - start, 0, 0, 0); > } > > /* Allocate and free page tables. */ > diff --git a/arch/um/kernel/uml.lds.S b/arch/um/kernel/uml.lds.S > index 30aa24348d60..7085ba6fcb93 100644 > --- a/arch/um/kernel/uml.lds.S > +++ b/arch/um/kernel/uml.lds.S > @@ -70,8 +70,10 @@ SECTIONS > > #include <asm/common.lds.S> > > + . = ALIGN(PAGE_SIZE); > __init_begin = .; > init.data : { INIT_DATA } > + . = ALIGN(PAGE_SIZE); > __init_end = .; > > .data : > -- > 2.55.0 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] um: mprotect() __init memory 2026-09-26 2:12 ` Hajime Tazaki @ 2026-09-26 9:16 ` Johannes Berg 2026-09-28 7:33 ` Johannes Berg 0 siblings, 1 reply; 4+ messages in thread From: Johannes Berg @ 2026-09-26 9:16 UTC (permalink / raw) To: Hajime Tazaki; +Cc: linux-um On Sat, 2026-09-26 at 11:12 +0900, Hajime Tazaki wrote: > Hello, > > I recently pulled uml/next branch and faced an issue on the exit. > > ``` > Thread 1 "vmlinux" received signal SIGSEGV, Segmentation fault. > 0x0000000060003cf3 in start_uml () at ../arch/um/kernel/skas/process.c:41 > 41 } > (gdb) bt > #0 0x0000000060003cf3 in start_uml () at ../arch/um/kernel/skas/process.c:41 > #1 0x0000000060003a69 in linux_main (argc=argc@entry=9, argv=argv@entry=0x7fffffffe118, > envp=envp@entry=0x7fffffffe168) at ../arch/um/kernel/um_arch.c:404 > #2 0x00000000600049dc in main (argc=9, argv=0x7fffffffe118, envp=0x7fffffffe168) > at ../arch/um/os-Linux/main.c:153 > ``` > > and bisected that this commit is the first rev to introduce this. I didn't get that, hmm. > indeed, now .text section is in __init label but some of startup code > (start_uml, linux_main) remain un-returned even after init is done. > > a quick change below avoid this issue, but not sure if it is your > intention of the original patch. No, clearly not - we shouldn't execute __init code after init either :) So I guess we found a but elsewhere, but I don't immediately see what needs to be not marked __init, clearly linux_main() is still used on the way out so that must be, but ... Or we could mprotect(..., 0, 0, 1) internally on the way out again, since really _while_ running we also shouldn't be in __init code, it's just a side effect of how UML shuts down? Got a config or so? I really don't see this. johannes ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] um: mprotect() __init memory 2026-09-26 9:16 ` Johannes Berg @ 2026-09-28 7:33 ` Johannes Berg 0 siblings, 0 replies; 4+ messages in thread From: Johannes Berg @ 2026-09-28 7:33 UTC (permalink / raw) To: Hajime Tazaki; +Cc: linux-um On Sat, 2026-09-26 at 11:16 +0200, Johannes Berg wrote: > On Sat, 2026-09-26 at 11:12 +0900, Hajime Tazaki wrote: > > Hello, > > > > I recently pulled uml/next branch and faced an issue on the exit. > > > > ``` > > Thread 1 "vmlinux" received signal SIGSEGV, Segmentation fault. > > 0x0000000060003cf3 in start_uml () at ../arch/um/kernel/skas/process.c:41 > > 41 } > > (gdb) bt > > #0 0x0000000060003cf3 in start_uml () at ../arch/um/kernel/skas/process.c:41 > > #1 0x0000000060003a69 in linux_main (argc=argc@entry=9, argv=argv@entry=0x7fffffffe118, > > envp=envp@entry=0x7fffffffe168) at ../arch/um/kernel/um_arch.c:404 > > #2 0x00000000600049dc in main (argc=9, argv=0x7fffffffe118, envp=0x7fffffffe168) > > at ../arch/um/os-Linux/main.c:153 > > ``` > > > > and bisected that this commit is the first rev to introduce this. > > I didn't get that, hmm. Actually I did, just no backtrace (console was already shut down), only "Aborted" and I missed that. Looking at it with a fresh head, I think removing __init everywhere is a bad idea. But we're already longjmp()ing around, so we don't really _need_ to ever return from start_idle_thread(). We can have that call an "os_exit()" function instead of returning through all the functions, and that fixes the bug while keeping the __init behaviour. Btw, it's not just that commit, but also the other one moving init text into the right place. I'll send a patch in a minute. johannes ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-28 7:34 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-21 10:29 [PATCH] um: mprotect() __init memory Johannes Berg 2026-09-26 2:12 ` Hajime Tazaki 2026-09-26 9:16 ` Johannes Berg 2026-09-28 7:33 ` Johannes Berg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox