Message ID | 20200206204937.db680692f366.Ie05fc2ad440c8db59c1519194aeac356469efa72@changeid |
---|---|
State | Not Applicable |
Headers | show |
Series | [DEMO] um: demonstrate super early constructors | expand |
Now, > +static void early_print(void) > +{ > + printf("I'm super early, before constructors\n"); > +} [...] > +static void (*early_print_ptr)(void) > +__attribute__((section(".kasan_init"), used)) > + = early_print; As you can see from the section name, I was obviously thinking of KASAN here :-) You had the following kasan_init function, and doing it this early will be a bit problematic: > +void kasan_init(void) > +{ > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); This should be OK. I actually thought we'd have to know the memory size even for this, but it looks like not, which is great! I guess you just made it have a mapping that's basically large enough for everything. > + // unpoison the kernel text which is form uml_physmem -> uml_reserved > + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); This I guess you'd have to defer until they're actually set up, but that's OK - we can only use them after that. So just moving that to wherever uml_physmem/physmem_size are initialized should be OK. > + // unpoison the vmalloc region, which is start_vm -> end_vm > + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); This I'm not sure about, but again - I suppose kasan_unpoison_shadow() really only needs the shadow memory, so we can do it whenever we actually init the start_vm/end_vm variables (i.e. in linux_main()). > + init_task.kasan_depth = 0; I guess that's OK, though it should be 0-initialized anyway? > + pr_info("KernelAddressSanitizer initialized\n"); That might be a bit dangerous this early in the init process, but I guess we don't really need it :-) Can always print something much later saying that yes, you have KASAN enabled. johannes
On Thu, Feb 6, 2020 at 11:55 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > Now, > > > > +static void early_print(void) > > +{ > > + printf("I'm super early, before constructors\n"); > > +} > [...] > > +static void (*early_print_ptr)(void) > > +__attribute__((section(".kasan_init"), used)) > > + = early_print; > > As you can see from the section name, I was obviously thinking of KASAN > here :-) > > You had the following kasan_init function, and doing it this early will > be a bit problematic: > With my new changes, this actually works great! Thank you! > > +void kasan_init(void) > > +{ > > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > > This should be OK. I actually thought we'd have to know the memory size > even for this, but it looks like not, which is great! I guess you just > made it have a mapping that's basically large enough for everything. > > > + // unpoison the kernel text which is form uml_physmem -> uml_reserved > > + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); > > This I guess you'd have to defer until they're actually set up, but > that's OK - we can only use them after that. So just moving that to > wherever uml_physmem/physmem_size are initialized should be OK. > > > + // unpoison the vmalloc region, which is start_vm -> end_vm > > + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); > > This I'm not sure about, but again - I suppose kasan_unpoison_shadow() > really only needs the shadow memory, so we can do it whenever we > actually init the start_vm/end_vm variables (i.e. in linux_main()). > These are actually extraneous lines of code, as pointed out by Dmitry Vyukov, because mmap always gives zeroed memory so they're no longer a problem. Thank you so much for this patch!! I have been trying to find an elegant fix all week and this is perfect!
Hi, > Thank you so much for this patch!! I have been trying to find an > elegant fix all week and this is perfect! Glad it helped :-) johannes
diff --git a/arch/um/include/asm/common.lds.S b/arch/um/include/asm/common.lds.S index eca6c452a41b..731f8c8422a2 100644 --- a/arch/um/include/asm/common.lds.S +++ b/arch/um/include/asm/common.lds.S @@ -83,6 +83,7 @@ } .init_array : { __init_array_start = .; + *(.kasan_init) *(.init_array) __init_array_end = .; } diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S index f5001481010c..4f0447cd9e38 100644 --- a/arch/um/kernel/dyn.lds.S +++ b/arch/um/kernel/dyn.lds.S @@ -103,7 +103,10 @@ SECTIONS be empty, which isn't pretty. */ . = ALIGN(32 / 8); .preinit_array : { *(.preinit_array) } - .init_array : { *(.init_array) } + .init_array : { + *(.kasan_init) + *(.init_array) + } .fini_array : { *(.fini_array) } .data : { INIT_TASK_DATA(KERNEL_STACK_SIZE) diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c index 30885d0b94ac..32fc941c80f7 100644 --- a/arch/um/kernel/mem.c +++ b/arch/um/kernel/mem.c @@ -19,6 +19,21 @@ #include <mem_user.h> #include <os.h> +extern int printf(const char *msg, ...); +static void early_print(void) +{ + printf("I'm super early, before constructors\n"); +} + +static void __attribute__((constructor)) constructor_test(void) +{ + printf("yes, you can see it\n"); +} + +static void (*early_print_ptr)(void) +__attribute__((section(".kasan_init"), used)) + = early_print; + /* allocated in paging_init, zeroed in mem_init, and unchanged thereafter */ unsigned long *empty_zero_page = NULL; EXPORT_SYMBOL(empty_zero_page);
From: Johannes Berg <johannes.berg@intel.com> For KASAN, we need super early constructors, demonstrate how we could perhaps do that. I'm not sure why we have two places with .init_array in the linker scripts, the common one does get included, but then the dyn one seems to override it, perhaps for alignment? but it seems it would be harmless to do the alignment in the common one ... not sure. --- arch/um/include/asm/common.lds.S | 1 + arch/um/kernel/dyn.lds.S | 5 ++++- arch/um/kernel/mem.c | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-)