[PATCH 1/7] tracing: add ftrace_regs to function_graph_enter()
Will be used later for showing function arguments in the function graph tracer. Signed-off-by: Sven Schnelle --- arch/arm/kernel/ftrace.c | 2 +- arch/arm64/kernel/ftrace.c | 2 +- arch/csky/kernel/ftrace.c | 2 +- arch/loongarch/kernel/ftrace.c | 2 +- arch/loongarch/kernel/ftrace_dyn.c | 2 +- arch/microblaze/kernel/ftrace.c| 2 +- arch/mips/kernel/ftrace.c | 2 +- arch/parisc/kernel/ftrace.c| 2 +- arch/powerpc/kernel/trace/ftrace.c | 2 +- arch/riscv/kernel/ftrace.c | 2 +- arch/s390/kernel/ftrace.c | 2 +- arch/sh/kernel/ftrace.c| 2 +- arch/sparc/kernel/ftrace.c | 2 +- arch/x86/kernel/ftrace.c | 2 +- include/linux/ftrace.h | 3 ++- kernel/trace/fgraph.c | 3 ++- 16 files changed, 18 insertions(+), 16 deletions(-) diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c index e61591f33a6c..1f8802439e34 100644 --- a/arch/arm/kernel/ftrace.c +++ b/arch/arm/kernel/ftrace.c @@ -267,7 +267,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, old = *parent; *parent = return_hooker; - if (function_graph_enter(old, self_addr, frame_pointer, NULL)) + if (function_graph_enter(old, self_addr, frame_pointer, NULL, NULL)) *parent = old; } diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c index a650f5e11fc5..686fbebb0432 100644 --- a/arch/arm64/kernel/ftrace.c +++ b/arch/arm64/kernel/ftrace.c @@ -472,7 +472,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, old = *parent; if (!function_graph_enter(old, self_addr, frame_pointer, - (void *)frame_pointer)) { + (void *)frame_pointer, NULL)) { *parent = return_hooker; } } diff --git a/arch/csky/kernel/ftrace.c b/arch/csky/kernel/ftrace.c index 50bfcf129078..c12af268c1cb 100644 --- a/arch/csky/kernel/ftrace.c +++ b/arch/csky/kernel/ftrace.c @@ -156,7 +156,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, old = *parent; if (!function_graph_enter(old, self_addr, - *(unsigned long *)frame_pointer, parent)) { + *(unsigned long *)frame_pointer, parent, NULL)) { /* * For csky-gcc function has sub-call: * subi sp, sp, 8 diff --git a/arch/loongarch/kernel/ftrace.c b/arch/loongarch/kernel/ftrace.c index 8c3ec1bc7aad..43d908b01718 100644 --- a/arch/loongarch/kernel/ftrace.c +++ b/arch/loongarch/kernel/ftrace.c @@ -61,7 +61,7 @@ void prepare_ftrace_return(unsigned long self_addr, if (ftrace_get_parent_ra_addr(self_addr, &ra_off)) goto out; - if (!function_graph_enter(old, self_addr, 0, NULL)) + if (!function_graph_enter(old, self_addr, 0, NULL, NULL)) *(unsigned long *)(callsite_sp + ra_off) = return_hooker; return; diff --git a/arch/loongarch/kernel/ftrace_dyn.c b/arch/loongarch/kernel/ftrace_dyn.c index bff058317062..eab16231d09d 100644 --- a/arch/loongarch/kernel/ftrace_dyn.c +++ b/arch/loongarch/kernel/ftrace_dyn.c @@ -233,7 +233,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent) old = *parent; - if (!function_graph_enter(old, self_addr, 0, parent)) + if (!function_graph_enter(old, self_addr, 0, parent, NULL)) *parent = return_hooker; } diff --git a/arch/microblaze/kernel/ftrace.c b/arch/microblaze/kernel/ftrace.c index 188749d62709..009800d7e54f 100644 --- a/arch/microblaze/kernel/ftrace.c +++ b/arch/microblaze/kernel/ftrace.c @@ -62,7 +62,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr) return; } - if (function_graph_enter(old, self_addr, 0, NULL)) + if (function_graph_enter(old, self_addr, 0, NULL, NULL)) *parent = old; } #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c index 8c401e42301c..65f29de35a59 100644 --- a/arch/mips/kernel/ftrace.c +++ b/arch/mips/kernel/ftrace.c @@ -362,7 +362,7 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra, insns = core_kernel_text(self_ra) ? 2 : MCOUNT_OFFSET_INSNS + 1; self_ra -= (MCOUNT_INSN_SIZE * insns); - if (function_graph_enter(old_parent_ra, self_ra, fp, NULL)) + if (function_graph_enter(old_parent_ra, self_ra, fp, NULL, NULL)) *parent_ra_addr = old_parent_ra; return; out: diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c index c91f9c2e61ed..c8d926f057a6 100644 --- a/arch/parisc/kernel/ftrace.c +++ b/arch/parisc/kernel/ftrace.c @@ -45,7 +45,7 @@ static void __hot prepare_ftrace_return(unsigned long *parent, old = *parent; - if (!function_graph_enter
Re: [PATCH v2 1/4] mm: Add optional close() to struct vm_special_mapping
Linus Torvalds writes: > On Mon, 2 Sept 2024 at 13:49, Andrew Morton wrote: >> >> uprobe_clear_state() is a pretty simple low-level thing. Side-effects >> seem unlikely? > > I think uprobe_clear_state() should be removed from fork.c entirely, > made 'static', and then we'd have > > area->xol_mapping.close = uprobe_clear_state; > > in __create_xol_area() instead (ok, the arguments change, instead of > looking up "mm->uprobes_state.xol_area", it would get it as the vma > argument) > > That's how it should always have been, except we didn't have a close() > function. > > Hmm? Indeed, that's much better. I'll prepare a patch. Thanks!
Re: [PATCH v2 1/4] mm: Add optional close() to struct vm_special_mapping
Nathan Chancellor writes: > Hi Michael, > > On Mon, Aug 12, 2024 at 06:26:02PM +1000, Michael Ellerman wrote: >> Add an optional close() callback to struct vm_special_mapping. It will >> be used, by powerpc at least, to handle unmapping of the VDSO. >> >> Although support for unmapping the VDSO was initially added >> for CRIU[1], it is not desirable to guard that support behind >> CONFIG_CHECKPOINT_RESTORE. >> >> There are other known users of unmapping the VDSO which are not related >> to CRIU, eg. Valgrind [2] and void-ship [3]. >> >> The powerpc arch_unmap() hook has been in place for ~9 years, with no >> ifdef, so there may be other unknown users that have come to rely on >> unmapping the VDSO. Even if the code was behind an ifdef, major distros >> enable CHECKPOINT_RESTORE so users may not realise unmapping the VDSO >> depends on that configuration option. >> >> It's also undesirable to have such core mm behaviour behind a relatively >> obscure CONFIG option. >> >> Longer term the unmap behaviour should be standardised across >> architectures, however that is complicated by the fact the VDSO pointer >> is stored differently across architectures. There was a previous attempt >> to unify that handling [4], which could be revived. >> >> See [5] for further discussion. >> >> [1]: commit 83d3f0e90c6c ("powerpc/mm: tracking vDSO remap") >> [2]: >> https://sourceware.org/git/?p=valgrind.git;a=commit;h=3a004915a2cbdcdebafc1612427576bf3321eef5 >> [3]: https://github.com/insanitybit/void-ship >> [4]: https://lore.kernel.org/lkml/20210611180242.711399-17-d...@arista.com/ >> [5]: >> https://lore.kernel.org/linuxppc-dev/shiq5v3jrmyi6ncwke7wgl76ojysgbhrchsk32q4lbx2hadqqc@kzyy2igem256 >> >> Suggested-by: Linus Torvalds >> Signed-off-by: Michael Ellerman >> Reviewed-by: David Hildenbrand >> --- >> include/linux/mm_types.h | 3 +++ >> mm/mmap.c| 6 ++ >> 2 files changed, 9 insertions(+) >> >> v2: >> - Add some blank lines as requested. >> - Expand special_mapping_close() comment. >> - Add David's reviewed-by. >> - Expand change log to capture review discussion. >> >> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h >> index 485424979254..78bdfc59abe5 100644 >> --- a/include/linux/mm_types.h >> +++ b/include/linux/mm_types.h >> @@ -1313,6 +1313,9 @@ struct vm_special_mapping { >> >> int (*mremap)(const struct vm_special_mapping *sm, >> struct vm_area_struct *new_vma); >> + >> +void (*close)(const struct vm_special_mapping *sm, >> + struct vm_area_struct *vma); >> }; >> >> enum tlb_flush_reason { >> diff --git a/mm/mmap.c b/mm/mmap.c >> index d0dfc85b209b..af4dbf0d3bd4 100644 >> --- a/mm/mmap.c >> +++ b/mm/mmap.c >> @@ -3620,10 +3620,16 @@ void vm_stat_account(struct mm_struct *mm, >> vm_flags_t flags, long npages) >> static vm_fault_t special_mapping_fault(struct vm_fault *vmf); >> >> /* >> + * Close hook, called for unmap() and on the old vma for mremap(). >> + * >> * Having a close hook prevents vma merging regardless of flags. >> */ >> static void special_mapping_close(struct vm_area_struct *vma) >> { >> +const struct vm_special_mapping *sm = vma->vm_private_data; >> + >> +if (sm->close) >> +sm->close(sm, vma); >> } >> >> static const char *special_mapping_name(struct vm_area_struct *vma) >> -- >> 2.45.2 >> > > This change is now in -next and I bisected a crash that our CI sees with > ARCH=um to it: I see another crash on s390, which seems related, but rather an issue in uprobe. This can be reproduced by # cd linux-next/tools/testing/selftests/ftrace # ./ftracetest ./test.d/dynevent/add_remove_uprobe.tc The 'mm: Add optional close() to struct vm_special_mapping' patch just makes it visible. I enabled KASAN, and that shows me: [ 44.505448] == 20:37:27 [3421/145075] [ 44.505455] BUG: KASAN: slab-use-after-free in special_mapping_close+0x9c/0xc8 [ 44.505471] Read of size 8 at addr 868dac48 by task sh/1384 [ 44.505479] [ 44.505486] CPU: 51 UID: 0 PID: 1384 Comm: sh Not tainted 6.11.0-rc6-next-20240902-dirty #1496 [ 44.505503] Hardware name: IBM 3931 A01 704 (z/VM 7.3.0) [ 44.505508] Call Trace: [ 44.505511] [<000b0324d2f78080>] dump_stack_lvl+0xd0/0x108 [ 44.505521] [<000b0324d2f5435c>] print_address_description.constprop.0+0x34/0x2e0 [ 44.505529] [<000b0324d2f5464c>] print_report+0x44/0x138 [ 44.505536] [<000b0324d1383192>] kasan_report+0xc2/0x140 [ 44.505543] [<000b0324d2f52904>] special_mapping_close+0x9c/0xc8 [ 44.505550] [<000b0324d12c7978>] remove_vma+0x78/0x120 [ 44.505557] [<000b0324d128a2c6>] exit_mmap+0x326/0x750 [ 44.505563] [<000b0324d0ba655a>] __mmput+0x9a/0x370 [ 44.505570] [<000b0324d0bbfbe0>] exit_mm+0x240/0x340 [ 44.505575] [<000b0324d0bc0228>] do_exit+0x548/0xd70 [ 44.505580] [<000b0324d0bc
Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'
Josh Poimboeuf writes: > With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative > pointers are calculated weirdly: based on the beginning of the bug_entry > struct address, rather than their respective pointer addresses. > > Make the relative pointers less surprising to both humans and tools by > calculating them the normal way. > > Signed-off-by: Josh Poimboeuf Acked-by: Sven Schnelle # s390 > --- > arch/arm64/include/asm/asm-bug.h | 4 ++-- > arch/powerpc/include/asm/bug.h | 5 +++-- > arch/riscv/include/asm/bug.h | 4 ++-- > arch/s390/include/asm/bug.h | 5 +++-- > arch/x86/include/asm/bug.h | 2 +- > lib/bug.c| 15 +++ > 6 files changed, 18 insertions(+), 17 deletions(-) > > diff --git a/arch/arm64/include/asm/asm-bug.h > b/arch/arm64/include/asm/asm-bug.h > index 03f52f84a4f3..c762038ba400 100644 > --- a/arch/arm64/include/asm/asm-bug.h > +++ b/arch/arm64/include/asm/asm-bug.h > @@ -14,7 +14,7 @@ > 14472: .string file; \ > .popsection;\ > \ > - .long 14472b - 14470b; \ > + .long 14472b - .; \ > .short line; > #else > #define _BUGVERBOSE_LOCATION(file, line) > @@ -25,7 +25,7 @@ > #define __BUG_ENTRY(flags) \ > .pushsection __bug_table,"aw"; \ > .align 2; \ > - 14470: .long 14471f - 14470b; \ > + 14470: .long 14471f - .; \ > _BUGVERBOSE_LOCATION(__FILE__, __LINE__) \ > .short flags; \ > .popsection;\ > diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h > index ecbae1832de3..76252576d889 100644 > --- a/arch/powerpc/include/asm/bug.h > +++ b/arch/powerpc/include/asm/bug.h > @@ -13,7 +13,8 @@ > #ifdef CONFIG_DEBUG_BUGVERBOSE > .macro __EMIT_BUG_ENTRY addr,file,line,flags >.section __bug_table,"aw" > -5001: .4byte \addr - 5001b, 5002f - 5001b > +5001: .4byte \addr - . > + .4byte 5002f - . >.short \line, \flags >.org 5001b+BUG_ENTRY_SIZE >.previous > @@ -24,7 +25,7 @@ > #else > .macro __EMIT_BUG_ENTRY addr,file,line,flags >.section __bug_table,"aw" > -5001: .4byte \addr - 5001b > +5001: .4byte \addr - . >.short \flags >.org 5001b+BUG_ENTRY_SIZE >.previous > diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h > index d3804a2f9aad..1aaea81fb141 100644 > --- a/arch/riscv/include/asm/bug.h > +++ b/arch/riscv/include/asm/bug.h > @@ -30,8 +30,8 @@ > typedef u32 bug_insn_t; > > #ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS > -#define __BUG_ENTRY_ADDR RISCV_INT " 1b - 2b" > -#define __BUG_ENTRY_FILE RISCV_INT " %0 - 2b" > +#define __BUG_ENTRY_ADDR RISCV_INT " 1b - ." > +#define __BUG_ENTRY_FILE RISCV_INT " %0 - ." > #else > #define __BUG_ENTRY_ADDR RISCV_PTR " 1b" > #define __BUG_ENTRY_FILE RISCV_PTR " %0" > diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h > index 0b25f28351ed..aebe1e22c7be 100644 > --- a/arch/s390/include/asm/bug.h > +++ b/arch/s390/include/asm/bug.h > @@ -15,7 +15,8 @@ > "1: .asciz \""__FILE__"\"\n" \ > ".previous\n" \ > ".section __bug_table,\"awM\",@progbits,%2\n" \ > - "2: .long 0b-2b,1b-2b\n" \ > + "2: .long 0b-.\n" \ > + " .long 1b-.\n" \ > " .short %0,%1\n"\ > " .org2b+%2\n"\ > ".previous\n" \ > @@ -30,7 +31,7 @@ > asm_inline volatile(\ > "0: mc 0,0\n" \ > ".section __bug_table,\"awM\",@progbits,%1\n" \ > - "1: .long 0b-1b\n"\ > + "1: .long 0b-.\n" \ &g
Re: [PATCH v2 09/13] powerpc/ftrace: Implement CONFIG_DYNAMIC_FTRACE_WITH_ARGS
Heiko Carstens writes: > So, the in both variants s390 provides nearly identical data. The only > difference is that for FL_SAVE_REGS the program status word mask is > missing; therefore it is not possible to figure out the condition code > or if interrupts were enabled/disabled. > > Vasily, Sven, I think we have two options here: > > - don't provide sane psw mask contents at all and say (again) that > ptregs contents are identical > > - provide (finally) a full psw mask contents using epsw, and indicate > validity with a flags bit in pt_regs > > I would vote for the second option, even though epsw is slow. But this > is about the third or fourth time this came up in different > contexts. So I'd guess we should go for the slow but complete > solution. Opinions? Given that this only affects ftrace_regs_caller, i would also vote for the second option.
Re: ftrace hangs waiting for rcu
Hi Mark, Mark Rutland writes: > On Fri, Jan 28, 2022 at 05:08:48PM +0100, Sven Schnelle wrote: >> We noticed the PR from Paul and are currently testing the fix. So far >> it's looking good. The configuration where we have seen the hang is a >> bit unusual: >> >> - 16 physical CPUs on the kvm host >> - 248 logical CPUs inside kvm > > Aha! 248 is notably *NOT* a power of two, and in this case the shift would be > wrong (ilog2() would give 7, when we need a shift of 8). > > So I suspect you're hitting the same issue as I was. Argh, indeed! I somehow changed 'power of two' to 'odd number' in my head. I guess it's time for the weekend. :-) Thanks!
Re: ftrace hangs waiting for rcu
Hi Mark, Mark Rutland writes: > On arm64 I bisected this down to: > > 7a30871b6a27de1a ("rcu-tasks: Introduce ->percpu_enqueue_shift for dynamic > queue selection") > > Which was going wrong because ilog2() rounds down, and so the shift was wrong > for any nr_cpus that was not a power-of-two. Paul had already fixed that in > rcu-next, and just sent a pull request to Linus: > > > https://lore.kernel.org/lkml/20220128143251.GA2398275@paulmck-ThinkPad-P17-Gen-1/ > > With that applied, I no longer see these hangs. > > Does your s390 test machine have a non-power-of-two nr_cpus, and does that fix > the issue for you? We noticed the PR from Paul and are currently testing the fix. So far it's looking good. The configuration where we have seen the hang is a bit unusual: - 16 physical CPUs on the kvm host - 248 logical CPUs inside kvm - debug kernel both on the host and kvm guest So things are likely a bit slow in the kvm guest. Interesting is that the number of CPUs is even. But maybe RCU sees an odd number of CPUs and gets confused before all cpus are brought up. Have to read code/test to see whether that could be possible. Thanks for investigating! Sven
ftrace hangs waiting for rcu (was: Re: [PATCH] ftrace: Have architectures opt-in for mcount build time sorting)
Hi Mark, Mark Rutland writes: > * I intermittently see a hang when running the tests. I previously hit that > when originally trying to bisect this issue (and IIRC that bisected down to > some RCU changes, but I need to re-run that). When the tests hang I > magic-srsrq + L tells me: > > [ 271.938438] sysrq: Show Blocked State > [ 271.939245] task:ftracetest state:D stack:0 pid: 5687 ppid: > 5627 flags:0x0200 > [ 271.940961] Call trace: > [ 271.941472] __switch_to+0x104/0x160 > [ 271.942213] __schedule+0x2b0/0x6e0 > [ 271.942933] schedule+0x5c/0xf0 > [ 271.943586] schedule_timeout+0x184/0x1c4 > [ 271.944410] wait_for_completion+0x8c/0x12c > [ 271.945274] __wait_rcu_gp+0x184/0x190 > [ 271.946047] synchronize_rcu_tasks_rude+0x48/0x70 > [ 271.947007] update_ftrace_function+0xa4/0xec > [ 271.947897] __unregister_ftrace_function+0xa4/0xf0 > [ 271.948898] unregister_ftrace_function+0x34/0x70 > [ 271.949857] wakeup_tracer_reset+0x4c/0x100 > [ 271.950713] tracing_set_tracer+0xd0/0x2b0 > [ 271.951552] tracing_set_trace_write+0xe8/0x150 > [ 271.952477] vfs_write+0xfc/0x284 > [ 271.953171] ksys_write+0x7c/0x110 > [ 271.953874] __arm64_sys_write+0x2c/0x40 > [ 271.954678] invoke_syscall+0x5c/0x130 > [ 271.955442] el0_svc_common.constprop.0+0x108/0x130 > [ 271.956435] do_el0_svc+0x74/0x90 > [ 271.957124] el0_svc+0x2c/0x90 > [ 271.957757] el0t_64_sync_handler+0xa8/0x12c > [ 271.958629] el0t_64_sync+0x1a0/0x1a4 that's interesting. On s390 i'm seeing the same problem in CI, but with the startup ftrace tests. So that's likely not arm64 spacific. On s390, the last messages from ftrace are [5.663568] clocksource: jiffies: mask: 0x max_cycles: 0x, max_idle_ns: 1911260446275 ns [5.667099] futex hash table entries: 65536 (order: 12, 16777216 bytes, vmalloc) [5.739549] Running postponed tracer tests: [5.740662] Testing tracer function: PASSED [6.194635] Testing dynamic ftrace: PASSED [6.471213] Testing dynamic ftrace ops #1: [6.558445] (1 0 1 0 0) [6.558458] (1 1 2 0 0) [6.699135] (2 1 3 0 764347) [6.699252] (2 2 4 0 766466) [6.759857] (3 2 4 0 1159604) [..] hangs here The backtrace looks like this, which is very similar to the one above: crash> bt 1 PID: 1 TASK: 80e68100 CPU: 133 COMMAND: "swapper/0" #0 [380004df808] __schedule at cda39f0e #1 [380004df880] schedule at cda3a488 #2 [380004df8b0] schedule_timeout at cda41ef6 #3 [380004df978] wait_for_completion at cda3bd0a #4 [380004df9d8] __wait_rcu_gp at cc92 #5 [380004dfa30] synchronize_rcu_tasks_generic at ccdde0aa #6 [380004dfad8] ftrace_shutdown at cce7b050 #7 [380004dfb18] unregister_ftrace_function at cce7b192 #8 [380004dfb50] trace_selftest_ops at cda1e0fa #9 [380004dfba0] run_tracer_selftest at cda1e4f2 #10 [380004dfc00] trace_selftest_startup_function at ce74355c #11 [380004dfc58] run_tracer_selftest at cda1e2fc #12 [380004dfc98] init_trace_selftests at ce742d30 #13 [380004dfcd0] do_one_initcall at cccdca16 #14 [380004dfd68] do_initcalls at ce72e776 #15 [380004dfde0] kernel_init_freeable at ce72ea60 #16 [380004dfe50] kernel_init at cda333fe #17 [380004dfe68] __ret_from_fork at cccdf920 #18 [380004dfe98] ret_from_fork at cda444ca I didn't had success reproducing it so far, but it is good to know that this also happens when running the ftrace testsuite. I have several crashdumps, so i could try to pull out some information if someone tells me what to look for. Thanks, Sven
Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
Mark Rutland writes: > On Thu, Jan 27, 2022 at 07:46:01AM -0500, Steven Rostedt wrote: >> On Thu, 27 Jan 2022 12:27:04 + >> Mark Rutland wrote: >> >> > Ah, so those non-ELF relocations for the mcount_loc table just mean "apply >> > the >> > KASLR offset here", which is equivalent for all entries. >> > >> > That makes sense, thanks! >> >> And this is why we were having such a hard time understanding each other ;-) > > ;) > > With that in mind, I think that we understand that the build-time sort works > for: > > * arch/x86, becuase the non-ELF relocations for mcount_loc happen to be > equivalent. > > * arch/arm, because there's no dynamic relocaiton and the mcount_loc entries > have been finalized prior to sorting. > > ... but doesn't work for anyone else (including arm64) because the ELF > relocations are not equivalent, and need special care that is not yet > implemented. For s390 my idea is to just skip the addresses between __start_mcount_loc and __stop_mcount_loc, because for these addresses we know that they are 64 bits wide, so we just need to add the KASLR offset. I'm thinking about something like this: diff --git a/arch/s390/boot/compressed/decompressor.h b/arch/s390/boot/compressed/decompressor.h index f75cc31a77dd..015d7e2e94ef 100644 --- a/arch/s390/boot/compressed/decompressor.h +++ b/arch/s390/boot/compressed/decompressor.h @@ -25,6 +25,8 @@ struct vmlinux_info { unsigned long rela_dyn_start; unsigned long rela_dyn_end; unsigned long amode31_size; + unsigned long start_mcount_loc; + unsigned long stop_mcount_loc; }; /* Symbols defined by linker scripts */ diff --git a/arch/s390/boot/startup.c b/arch/s390/boot/startup.c index 1aa11a8f57dd..7bb0d88db5c6 100644 --- a/arch/s390/boot/startup.c +++ b/arch/s390/boot/startup.c @@ -88,6 +88,11 @@ static void handle_relocs(unsigned long offset) dynsym = (Elf64_Sym *) vmlinux.dynsym_start; for (rela = rela_start; rela < rela_end; rela++) { loc = rela->r_offset + offset; + if ((loc >= vmlinux.start_mcount_loc) && + (loc < vmlinux.stop_mcount_loc)) { + (*(unsigned long *)loc) += offset; + continue; + } val = rela->r_addend; r_sym = ELF64_R_SYM(rela->r_info); if (r_sym) { @@ -232,6 +237,8 @@ static void offset_vmlinux_info(unsigned long offset) vmlinux.rela_dyn_start += offset; vmlinux.rela_dyn_end += offset; vmlinux.dynsym_start += offset; + vmlinux.start_mcount_loc += offset; + vmlinux.stop_mcount_loc += offset; } static unsigned long reserve_amode31(unsigned long safe_addr) diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S index 42c43521878f..51c773405608 100644 --- a/arch/s390/kernel/vmlinux.lds.S +++ b/arch/s390/kernel/vmlinux.lds.S @@ -213,6 +213,8 @@ SECTIONS QUAD(__rela_dyn_start) /* rela_dyn_start */ QUAD(__rela_dyn_end)/* rela_dyn_end */ QUAD(_eamode31 - _samode31) /* amode31_size */ + QUAD(__start_mcount_loc) + QUAD(__stop_mcount_loc) } :NONE /* Debugging sections. */ Not sure whether that would also work on power, and also not sure whether i missed something thinking about that. Maybe it doesn't even work. ;-)
Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
Mark Rutland writes: >> Isn't x86 relocatable in some configurations (e.g. for KASLR)? >> >> I can't see how the sort works for those cases, because the mcount_loc >> entries >> are absolute, and either: >> >> * The sorted entries will get overwritten by the unsorted relocation entries, >> and won't be sorted. >> >> * The sorted entries won't get overwritten, but then the absolute address >> will >> be wrong since they hadn't been relocated. >> >> How does that work? >From what i've seen when looking into this ftrace sort problem x86 has a a relocation tool, which is run before final linking: arch/x86/tools/relocs.c This tools converts all the required relocations to three types: - 32 bit relocations - 64 bit relocations - inverse 32 bit relocations These are added to the end of the image. The decompressor then iterates over that array, and just adds/subtracts the KASLR offset - see arch/x86/boot/compressed/misc.c, handle_relocations() So IMHO x86 never uses 'real' relocations during boot, and just adds/subtracts. That's why the order stays the same, and the compile time sort works. /Sven
[PATCH v5 5/7] kexec_elf: remove Elf_Rel macro
It wasn't used anywhere, so lets drop it. Reviewed-by: Christophe Leroy Reviewed-by: Thiago Jung Bauermann Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 4 1 file changed, 4 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 87935bd5e2ba..6c806ce96ac1 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -23,10 +23,6 @@ #define elf_addr_to_cpuelf64_to_cpu -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - static inline bool elf_is_elf_file(const struct elfhdr *ehdr) { return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -- 2.23.0.rc1
[PATCH v5 7/7] kexec_elf: support 32 bit ELF files
The powerpc version only supported 64 bit. Add some code to switch decoding of fields during runtime so we can kexec a 32 bit kernel from a 64 bit kernel and vice versa. Signed-off-by: Sven Schnelle Reviewed-by: Thiago Jung Bauermann --- kernel/kexec_elf.c | 57 ++ 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 85f2bd177d6e..d3689632e8b9 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -21,8 +21,6 @@ #include #include -#define elf_addr_to_cpuelf64_to_cpu - static inline bool elf_is_elf_file(const struct elfhdr *ehdr) { return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; @@ -152,9 +150,6 @@ static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr) ehdr->e_type = elf16_to_cpu(ehdr, buf_ehdr->e_type); ehdr->e_machine = elf16_to_cpu(ehdr, buf_ehdr->e_machine); ehdr->e_version = elf32_to_cpu(ehdr, buf_ehdr->e_version); - ehdr->e_entry = elf_addr_to_cpu(ehdr, buf_ehdr->e_entry); - ehdr->e_phoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_phoff); - ehdr->e_shoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_shoff); ehdr->e_flags = elf32_to_cpu(ehdr, buf_ehdr->e_flags); ehdr->e_phentsize = elf16_to_cpu(ehdr, buf_ehdr->e_phentsize); ehdr->e_phnum = elf16_to_cpu(ehdr, buf_ehdr->e_phnum); @@ -162,6 +157,24 @@ static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr) ehdr->e_shnum = elf16_to_cpu(ehdr, buf_ehdr->e_shnum); ehdr->e_shstrndx = elf16_to_cpu(ehdr, buf_ehdr->e_shstrndx); + switch (ehdr->e_ident[EI_CLASS]) { + case ELFCLASS64: + ehdr->e_entry = elf64_to_cpu(ehdr, buf_ehdr->e_entry); + ehdr->e_phoff = elf64_to_cpu(ehdr, buf_ehdr->e_phoff); + ehdr->e_shoff = elf64_to_cpu(ehdr, buf_ehdr->e_shoff); + break; + + case ELFCLASS32: + ehdr->e_entry = elf32_to_cpu(ehdr, buf_ehdr->e_entry); + ehdr->e_phoff = elf32_to_cpu(ehdr, buf_ehdr->e_phoff); + ehdr->e_shoff = elf32_to_cpu(ehdr, buf_ehdr->e_shoff); + break; + + default: + pr_debug("Unknown ELF class.\n"); + return -EINVAL; + } + return elf_is_ehdr_sane(ehdr, len) ? 0 : -ENOEXEC; } @@ -192,6 +205,7 @@ static int elf_read_phdr(const char *buf, size_t len, { /* Override the const in proghdrs, we are the ones doing the loading. */ struct elf_phdr *phdr = (struct elf_phdr *) &elf_info->proghdrs[idx]; + const struct elfhdr *ehdr = elf_info->ehdr; const char *pbuf; struct elf_phdr *buf_phdr; @@ -199,18 +213,31 @@ static int elf_read_phdr(const char *buf, size_t len, buf_phdr = (struct elf_phdr *) pbuf; phdr->p_type = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_type); - phdr->p_offset = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_offset); - phdr->p_paddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_paddr); - phdr->p_vaddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr); phdr->p_flags = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_flags); - /* -* The following fields have a type equivalent to Elf_Addr -* both in 32 bit and 64 bit ELF. -*/ - phdr->p_filesz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_filesz); - phdr->p_memsz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_memsz); - phdr->p_align = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_align); + switch (ehdr->e_ident[EI_CLASS]) { + case ELFCLASS64: + phdr->p_offset = elf64_to_cpu(ehdr, buf_phdr->p_offset); + phdr->p_paddr = elf64_to_cpu(ehdr, buf_phdr->p_paddr); + phdr->p_vaddr = elf64_to_cpu(ehdr, buf_phdr->p_vaddr); + phdr->p_filesz = elf64_to_cpu(ehdr, buf_phdr->p_filesz); + phdr->p_memsz = elf64_to_cpu(ehdr, buf_phdr->p_memsz); + phdr->p_align = elf64_to_cpu(ehdr, buf_phdr->p_align); + break; + + case ELFCLASS32: + phdr->p_offset = elf32_to_cpu(ehdr, buf_phdr->p_offset); + phdr->p_paddr = elf32_to_cpu(ehdr, buf_phdr->p_paddr); + phdr->p_vaddr = elf32_to_cpu(ehdr, buf_phdr->p_vaddr); + phdr->p_filesz = elf32_to_cpu(ehdr, buf_phdr->p_filesz); + phdr->p_memsz = elf32_to_cpu(ehdr, buf_phdr->p_memsz); + phdr->p_align = elf32_to_cpu(ehdr, buf_phdr->p_align); + break; + + default: + pr_debug("Unknown ELF class.\n"); + return -EINVAL; + } return elf_is_phdr_sane(phdr, len) ? 0 : -ENOEXEC; } -- 2.23.0.rc1
[PATCH v5 4/7] kexec_elf: remove PURGATORY_STACK_SIZE
It's not used anywhere so just drop it. Signed-off-by: Sven Schnelle Reviewed-by: Thiago Jung Bauermann --- kernel/kexec_elf.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 137037603117..87935bd5e2ba 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -21,8 +21,6 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - #define elf_addr_to_cpuelf64_to_cpu #ifndef Elf_Rel -- 2.23.0.rc1
[PATCH v5 3/7] kexec_elf: remove parsing of section headers
We're not using them, so we can drop the parsing. Signed-off-by: Sven Schnelle Reviewed-by: Thiago Jung Bauermann --- include/linux/kexec.h | 1 - kernel/kexec_elf.c| 137 -- 2 files changed, 138 deletions(-) diff --git a/include/linux/kexec.h b/include/linux/kexec.h index da2a6b1d69e7..f0b809258ed3 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -226,7 +226,6 @@ struct kexec_elf_info { const struct elfhdr *ehdr; const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; }; int kexec_build_elf_info(const char *buf, size_t len, struct elfhdr *ehdr, diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 34376fbc55be..137037603117 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -257,134 +257,6 @@ static int elf_read_phdrs(const char *buf, size_t len, return 0; } -/** - * elf_is_shdr_sane - check that it is safe to use the section header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_shdr_sane(const struct elf_shdr *shdr, size_t buf_len) -{ - bool size_ok; - - /* SHT_NULL headers have undefined values, so we can't check them. */ - if (shdr->sh_type == SHT_NULL) - return true; - - /* Now verify sh_entsize */ - switch (shdr->sh_type) { - case SHT_SYMTAB: - size_ok = shdr->sh_entsize == sizeof(Elf_Sym); - break; - case SHT_RELA: - size_ok = shdr->sh_entsize == sizeof(Elf_Rela); - break; - case SHT_DYNAMIC: - size_ok = shdr->sh_entsize == sizeof(Elf_Dyn); - break; - case SHT_REL: - size_ok = shdr->sh_entsize == sizeof(Elf_Rel); - break; - case SHT_NOTE: - case SHT_PROGBITS: - case SHT_HASH: - case SHT_NOBITS: - default: - /* -* This is a section whose entsize requirements -* I don't care about. If I don't know about -* the section I can't care about it's entsize -* requirements. -*/ - size_ok = true; - break; - } - - if (!size_ok) { - pr_debug("ELF section with wrong entry size.\n"); - return false; - } else if (shdr->sh_addr + shdr->sh_size < shdr->sh_addr) { - pr_debug("ELF section address wraps around.\n"); - return false; - } - - if (shdr->sh_type != SHT_NOBITS) { - if (shdr->sh_offset + shdr->sh_size < shdr->sh_offset) { - pr_debug("ELF section location wraps around.\n"); - return false; - } else if (shdr->sh_offset + shdr->sh_size > buf_len) { - pr_debug("ELF section not in file.\n"); - return false; - } - } - - return true; -} - -static int elf_read_shdr(const char *buf, size_t len, -struct kexec_elf_info *elf_info, -int idx) -{ - struct elf_shdr *shdr = &elf_info->sechdrs[idx]; - const struct elfhdr *ehdr = elf_info->ehdr; - const char *sbuf; - struct elf_shdr *buf_shdr; - - sbuf = buf + ehdr->e_shoff + idx * sizeof(*buf_shdr); - buf_shdr = (struct elf_shdr *) sbuf; - - shdr->sh_name = elf32_to_cpu(ehdr, buf_shdr->sh_name); - shdr->sh_type = elf32_to_cpu(ehdr, buf_shdr->sh_type); - shdr->sh_addr = elf_addr_to_cpu(ehdr, buf_shdr->sh_addr); - shdr->sh_offset= elf_addr_to_cpu(ehdr, buf_shdr->sh_offset); - shdr->sh_link = elf32_to_cpu(ehdr, buf_shdr->sh_link); - shdr->sh_info = elf32_to_cpu(ehdr, buf_shdr->sh_info); - - /* -* The following fields have a type equivalent to Elf_Addr -* both in 32 bit and 64 bit ELF. -*/ - shdr->sh_flags = elf_addr_to_cpu(ehdr, buf_shdr->sh_flags); - shdr->sh_size = elf_addr_to_cpu(ehdr, buf_shdr->sh_size); - shdr->sh_addralign = elf_addr_to_cpu(ehdr, buf_shdr->sh_addralign); - shdr->sh_entsize = elf_addr_to_cpu(ehdr, buf_shdr->sh_entsize); - - return elf_is_shdr_sane(shdr, len) ? 0 : -ENOEXEC; -} - -/** - * elf_read_shdrs - read the section headers from the buffer - * - * This function assumes that the section header table was checked for sanity. - * Use elf_is_ehdr_sane() if it wasn't. - */ -static int elf_read_shdrs(const char *buf, size_t len, - struct kexec_elf_info *elf_info) -{ - size_t shdr_size, i; - - /* -* e_shnum is at most 65536 so calculating -* the size of the secti
[PATCH v5 1/7] kexec: add KEXEC_ELF
Right now powerpc provides an implementation to read elf files with the kexec_file_load() syscall. Make that available as a public kexec interface so it can be re-used on other architectures. Signed-off-by: Sven Schnelle --- arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c| 545 +- include/linux/kexec.h | 24 + kernel/Makefile | 1 + .../kexec_elf_64.c => kernel/kexec_elf.c | 183 ++ 6 files changed, 70 insertions(+), 687 deletions(-) copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_elf.c (75%) diff --git a/arch/Kconfig b/arch/Kconfig index a7b57dd42c26..01244412f393 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 7cb51bba4985..fbb0b305e1be 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -511,6 +511,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index 83cf7b852876..3072fd6dbe94 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -23,541 +23,6 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - -#define elf_addr_to_cpuelf64_to_cpu - -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le64_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be64_to_cpu(value); - - return value; -} - -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); - - return value; -} - -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); - - return value; -} - -/** - * elf_is_ehdr_sane - check that it is safe to use the ELF header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_t buf_len) -{ - if (ehdr->e_phnum > 0 && ehdr->e_phentsize != sizeof(struct elf_phdr)) { - pr_debug("Bad program header size.\n"); - return false; - } else if (ehdr->e_shnum > 0 && - ehdr->e_shentsize != sizeof(struct elf_shdr)) { - pr_debug("Bad section header size.\n"); - return false; - } else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT || - ehdr->e_version != EV_CURRENT) { - pr_debug("Unknown ELF version.\n"); - return false; - } - - if (ehdr->e_phoff > 0 && ehdr->e_phnum > 0) { - size_t phdr_size; - - /* -* e_phnum is at most 65535 so calculating the size of the -* program header cannot overflow. -*/ - phdr_size = sizeof(struct elf_phdr) * ehdr->e_phnum; - - /* Sanity check the program header table location. */ - if (ehdr->e_phoff + phdr_size < ehdr->e_phoff) { - pr_debug("Program headers at invalid location.\n"); - return false; - } else if (ehdr->e_phoff + phdr_size > buf_len) { - pr_debug("Program headers truncated.\n"); - return false; - } - } - - if (ehdr->e_shoff > 0 && ehdr->e_shnum > 0) { - size_t shdr_size; - - /* -* e_shnum is at most
[PATCH v5 6/7] kexec_elf: remove unused variable in kexec_elf_load()
base was never assigned, so we can remove it. Reviewed-by: Christophe Leroy Reviewed-by: Thiago Jung Bauermann Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 6c806ce96ac1..85f2bd177d6e 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -363,7 +363,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, struct kexec_buf *kbuf, unsigned long *lowest_load_addr) { - unsigned long base = 0, lowest_addr = UINT_MAX; + unsigned long lowest_addr = UINT_MAX; int ret; size_t i; @@ -385,7 +385,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, kbuf->bufsz = size; kbuf->memsz = phdr->p_memsz; kbuf->buf_align = phdr->p_align; - kbuf->buf_min = phdr->p_paddr + base; + kbuf->buf_min = phdr->p_paddr; kbuf->mem = KEXEC_BUF_MEM_UNKNOWN; ret = kexec_add_buffer(kbuf); if (ret) @@ -396,9 +396,6 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, lowest_addr = load_addr; } - /* Update entry point to reflect new load address. */ - ehdr->e_entry += base; - *lowest_load_addr = lowest_addr; ret = 0; out: -- 2.23.0.rc1
[PATCH v5 2/7] kexec_elf: change order of elf_*_to_cpu() functions
Change the order to have a 64/32/16 order, no functional change. Signed-off-by: Sven Schnelle Reviewed-by: Thiago Jung Bauermann --- kernel/kexec_elf.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 26c6310167a0..34376fbc55be 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -44,22 +44,22 @@ static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) return value; } -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) +static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) { if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); + value = le32_to_cpu(value); else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); + value = be32_to_cpu(value); return value; } -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) +static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) { if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); + value = le16_to_cpu(value); else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); + value = be16_to_cpu(value); return value; } -- 2.23.0.rc1
[PATCH v5 0/7] kexec: add generic support for elf kernel images
Changes to v4: - rebase on current powerpc/merge tree - fix syscall name in commit message - remove a few unused #defines in arch/powerpc/kernel/kexec_elf_64.c Changes to v3: - add support for 32-bit ELF files Changes to v2: - use git format-patch -C Changes to v1: - split up patch into smaller pieces - rebase onto powerpc/next - remove unused variable in kexec_elf_load() Changes to RFC version: - remove unused Elf_Rel macro - remove section header parsing - remove PURGATORY_STACK_SIZE - change order of elf_*_to_cpu() functions - remove elf_addr_to_cpu macro Sven Schnelle (7): kexec: add KEXEC_ELF kexec_elf: change order of elf_*_to_cpu() functions kexec_elf: remove parsing of section headers kexec_elf: remove PURGATORY_STACK_SIZE kexec_elf: remove Elf_Rel macro kexec_elf: remove unused variable in kexec_elf_load() kexec_elf: support 32 bit ELF files arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c| 545 +- include/linux/kexec.h | 23 + kernel/Makefile | 1 + .../kexec_elf_64.c => kernel/kexec_elf.c | 394 +++-- 6 files changed, 115 insertions(+), 852 deletions(-) copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_elf.c (50%) -- 2.23.0.rc1
Re: [PATCH 1/2] arch: mark syscall number 435 reserved for clone3
Hi, [Adding Helge to CC list] On Tue, Jul 16, 2019 at 03:06:33PM +0200, Christian Brauner wrote: > On Mon, Jul 15, 2019 at 03:56:04PM +0200, Christian Borntraeger wrote: > > I think Vasily already has a clone3 patch for s390x with 435. > > A quick follow-up on this. Helge and Michael have asked whether there > are any tests for clone3. Yes, there will be and I try to have them > ready by the end of the this or next week for review. In the meantime I > hope the following minimalistic test program that just verifies very > very basic functionality (It's not pretty.) will help you test: > [..] On PA-RISC this seems to work fine with Helge's patch to wire up the clone3 syscall. root@c3750:/# clonetest Parent process received child's pid 84 as return value Parent process received child's pidfd 3 Parent process received child's pid 84 as return argument Child process with pid 84 root@c3750:/# echo $? 0 Regards Sven
[PATCH v4 1/7] kexec: add KEXEC_ELF
Right now powerpc provides an implementation to read elf files with the kexec_file() syscall. Make that available as a public kexec interface so it can be re-used on other architectures. Signed-off-by: Sven Schnelle --- arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c| 551 +- include/linux/kexec.h | 24 + kernel/Makefile | 1 + .../kexec_elf_64.c => kernel/kexec_elf.c | 199 ++- 6 files changed, 75 insertions(+), 704 deletions(-) copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_elf.c (71%) diff --git a/arch/Kconfig b/arch/Kconfig index c47b328eada0..30694aca4316 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 12cee37f15c4..addc2dad78e0 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -510,6 +510,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..30bd57a93c17 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0-only /* * Load ELF vmlinux file for the kexec_file_load syscall. * @@ -10,15 +11,6 @@ * Based on kexec-tools' kexec-elf-exec.c and kexec-elf-ppc64.c. * Heavily modified for the kernel by * Thiago Jung Bauermann . - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation (version 2 of the License). - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #define pr_fmt(fmt)"kexec_elf: " fmt @@ -39,532 +31,6 @@ #define Elf_RelElf64_Rel #endif /* Elf_Rel */ -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le64_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be64_to_cpu(value); - - return value; -} - -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); - - return value; -} - -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); - - return value; -} - -/** - * elf_is_ehdr_sane - check that it is safe to use the ELF header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_t buf_len) -{ - if (ehdr->e_phnum > 0 && ehdr->e_phentsize != sizeof(struct elf_phdr)) { - pr_debug("Bad program header size.\n"); - return false; - } else if (ehdr->e_shnum > 0 && - ehdr->e_shentsize != sizeof(struct elf_shdr)) { - pr_debug("Bad section header size.\n"); - return false; - } else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT || - ehdr->e_version != EV_CURRENT) { - pr_debug("Unknown ELF version.\n"); - return false; - } - - if (ehdr->e_phoff > 0 && ehdr->e_phnum > 0) { - size_t phdr_size; - - /* -* e_phnum is at most 65535 so calculating the size of the -* program header cannot overflow. -*/ - phdr_size = sizeof(struct elf_phdr) * ehdr-&g
[PATCH v4 0/7] kexec: add generic support for elf kernel images
Changes to v3: - add support for 32-bit ELF files Changes to v2: - use git format-patch -C Changes to v1: - split up patch into smaller pieces - rebase onto powerpc/next - remove unused variable in kexec_elf_load() Changes to RFC version: - remove unused Elf_Rel macro - remove section header parsing - remove PURGATORY_STACK_SIZE - change order of elf_*_to_cpu() functions - remove elf_addr_to_cpu macro Sven Schnelle (7): kexec: add KEXEC_ELF kexec_elf: change order of elf_*_to_cpu() functions kexec_elf: remove parsing of section headers kexec_elf: remove PURGATORY_STACK_SIZE kexec_elf: remove Elf_Rel macro kexec_elf: remove unused variable in kexec_elf_load() kexec_elf: support 32 bit ELF files arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 551 + include/linux/kexec.h | 23 ++ kernel/Makefile| 1 + kernel/kexec_elf.c | 418 ++ 6 files changed, 456 insertions(+), 541 deletions(-) create mode 100644 kernel/kexec_elf.c -- 2.20.1
[PATCH v4 3/7] kexec_elf: remove parsing of section headers
We're not using them, so we can drop the parsing. Signed-off-by: Sven Schnelle --- include/linux/kexec.h | 1 - kernel/kexec_elf.c| 137 -- 2 files changed, 138 deletions(-) diff --git a/include/linux/kexec.h b/include/linux/kexec.h index da2a6b1d69e7..f0b809258ed3 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -226,7 +226,6 @@ struct kexec_elf_info { const struct elfhdr *ehdr; const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; }; int kexec_build_elf_info(const char *buf, size_t len, struct elfhdr *ehdr, diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 76e7df64d715..effe9dc0b055 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -244,134 +244,6 @@ static int elf_read_phdrs(const char *buf, size_t len, return 0; } -/** - * elf_is_shdr_sane - check that it is safe to use the section header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_shdr_sane(const struct elf_shdr *shdr, size_t buf_len) -{ - bool size_ok; - - /* SHT_NULL headers have undefined values, so we can't check them. */ - if (shdr->sh_type == SHT_NULL) - return true; - - /* Now verify sh_entsize */ - switch (shdr->sh_type) { - case SHT_SYMTAB: - size_ok = shdr->sh_entsize == sizeof(Elf_Sym); - break; - case SHT_RELA: - size_ok = shdr->sh_entsize == sizeof(Elf_Rela); - break; - case SHT_DYNAMIC: - size_ok = shdr->sh_entsize == sizeof(Elf_Dyn); - break; - case SHT_REL: - size_ok = shdr->sh_entsize == sizeof(Elf_Rel); - break; - case SHT_NOTE: - case SHT_PROGBITS: - case SHT_HASH: - case SHT_NOBITS: - default: - /* -* This is a section whose entsize requirements -* I don't care about. If I don't know about -* the section I can't care about it's entsize -* requirements. -*/ - size_ok = true; - break; - } - - if (!size_ok) { - pr_debug("ELF section with wrong entry size.\n"); - return false; - } else if (shdr->sh_addr + shdr->sh_size < shdr->sh_addr) { - pr_debug("ELF section address wraps around.\n"); - return false; - } - - if (shdr->sh_type != SHT_NOBITS) { - if (shdr->sh_offset + shdr->sh_size < shdr->sh_offset) { - pr_debug("ELF section location wraps around.\n"); - return false; - } else if (shdr->sh_offset + shdr->sh_size > buf_len) { - pr_debug("ELF section not in file.\n"); - return false; - } - } - - return true; -} - -static int elf_read_shdr(const char *buf, size_t len, -struct kexec_elf_info *elf_info, -int idx) -{ - struct elf_shdr *shdr = &elf_info->sechdrs[idx]; - const struct elfhdr *ehdr = elf_info->ehdr; - const char *sbuf; - struct elf_shdr *buf_shdr; - - sbuf = buf + ehdr->e_shoff + idx * sizeof(*buf_shdr); - buf_shdr = (struct elf_shdr *) sbuf; - - shdr->sh_name = elf32_to_cpu(ehdr, buf_shdr->sh_name); - shdr->sh_type = elf32_to_cpu(ehdr, buf_shdr->sh_type); - shdr->sh_addr = elf_addr_to_cpu(ehdr, buf_shdr->sh_addr); - shdr->sh_offset= elf_addr_to_cpu(ehdr, buf_shdr->sh_offset); - shdr->sh_link = elf32_to_cpu(ehdr, buf_shdr->sh_link); - shdr->sh_info = elf32_to_cpu(ehdr, buf_shdr->sh_info); - - /* -* The following fields have a type equivalent to Elf_Addr -* both in 32 bit and 64 bit ELF. -*/ - shdr->sh_flags = elf_addr_to_cpu(ehdr, buf_shdr->sh_flags); - shdr->sh_size = elf_addr_to_cpu(ehdr, buf_shdr->sh_size); - shdr->sh_addralign = elf_addr_to_cpu(ehdr, buf_shdr->sh_addralign); - shdr->sh_entsize = elf_addr_to_cpu(ehdr, buf_shdr->sh_entsize); - - return elf_is_shdr_sane(shdr, len) ? 0 : -ENOEXEC; -} - -/** - * elf_read_shdrs - read the section headers from the buffer - * - * This function assumes that the section header table was checked for sanity. - * Use elf_is_ehdr_sane() if it wasn't. - */ -static int elf_read_shdrs(const char *buf, size_t len, - struct kexec_elf_info *elf_info) -{ - size_t shdr_size, i; - - /* -* e_shnum is at most 65536 so calculating -* the size of the section header cannot overflow. -*/ - shd
[PATCH v4 2/7] kexec_elf: change order of elf_*_to_cpu() functions
Change the order to have a 64/32/16 order, no functional change. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 6e9f52171ede..76e7df64d715 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -31,22 +31,22 @@ static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) return value; } -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) +static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) { if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); + value = le32_to_cpu(value); else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); + value = be32_to_cpu(value); return value; } -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) +static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) { if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); + value = le16_to_cpu(value); else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); + value = be16_to_cpu(value); return value; } -- 2.20.1
[PATCH v4 5/7] kexec_elf: remove Elf_Rel macro
It wasn't used anywhere, so lets drop it. Reviewed-by: Christophe Leroy Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 4 1 file changed, 4 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 70d31b8feeae..e346659af324 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -10,10 +10,6 @@ #define elf_addr_to_cpuelf64_to_cpu -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - static inline bool elf_is_elf_file(const struct elfhdr *ehdr) { return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -- 2.20.1
[PATCH v4 6/7] kexec_elf: remove unused variable in kexec_elf_load()
base was never assigned, so we can remove it. Reviewed-by: Christophe Leroy Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index e346659af324..9421eebbacf0 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -350,7 +350,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, struct kexec_buf *kbuf, unsigned long *lowest_load_addr) { - unsigned long base = 0, lowest_addr = UINT_MAX; + unsigned long lowest_addr = UINT_MAX; int ret; size_t i; @@ -372,7 +372,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, kbuf->bufsz = size; kbuf->memsz = phdr->p_memsz; kbuf->buf_align = phdr->p_align; - kbuf->buf_min = phdr->p_paddr + base; + kbuf->buf_min = phdr->p_paddr; ret = kexec_add_buffer(kbuf); if (ret) goto out; @@ -382,9 +382,6 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, lowest_addr = load_addr; } - /* Update entry point to reflect new load address. */ - ehdr->e_entry += base; - *lowest_load_addr = lowest_addr; ret = 0; out: -- 2.20.1
[PATCH v4 4/7] kexec_elf: remove PURGATORY_STACK_SIZE
It's not used anywhere so just drop it. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index effe9dc0b055..70d31b8feeae 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,8 +8,6 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - #define elf_addr_to_cpuelf64_to_cpu #ifndef Elf_Rel -- 2.20.1
[PATCH v4 7/7] kexec_elf: support 32 bit ELF files
The powerpc version only supported 64 bit. Add some code to switch decoding of fields during runtime so we can kexec a 32 bit kernel from a 64 bit kernel and vice versa. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 57 ++ 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 9421eebbacf0..a39d01154829 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,8 +8,6 @@ #include #include -#define elf_addr_to_cpuelf64_to_cpu - static inline bool elf_is_elf_file(const struct elfhdr *ehdr) { return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; @@ -139,9 +137,6 @@ static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr) ehdr->e_type = elf16_to_cpu(ehdr, buf_ehdr->e_type); ehdr->e_machine = elf16_to_cpu(ehdr, buf_ehdr->e_machine); ehdr->e_version = elf32_to_cpu(ehdr, buf_ehdr->e_version); - ehdr->e_entry = elf_addr_to_cpu(ehdr, buf_ehdr->e_entry); - ehdr->e_phoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_phoff); - ehdr->e_shoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_shoff); ehdr->e_flags = elf32_to_cpu(ehdr, buf_ehdr->e_flags); ehdr->e_phentsize = elf16_to_cpu(ehdr, buf_ehdr->e_phentsize); ehdr->e_phnum = elf16_to_cpu(ehdr, buf_ehdr->e_phnum); @@ -149,6 +144,24 @@ static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr) ehdr->e_shnum = elf16_to_cpu(ehdr, buf_ehdr->e_shnum); ehdr->e_shstrndx = elf16_to_cpu(ehdr, buf_ehdr->e_shstrndx); + switch (ehdr->e_ident[EI_CLASS]) { + case ELFCLASS64: + ehdr->e_entry = elf64_to_cpu(ehdr, buf_ehdr->e_entry); + ehdr->e_phoff = elf64_to_cpu(ehdr, buf_ehdr->e_phoff); + ehdr->e_shoff = elf64_to_cpu(ehdr, buf_ehdr->e_shoff); + break; + + case ELFCLASS32: + ehdr->e_entry = elf32_to_cpu(ehdr, buf_ehdr->e_entry); + ehdr->e_phoff = elf32_to_cpu(ehdr, buf_ehdr->e_phoff); + ehdr->e_shoff = elf32_to_cpu(ehdr, buf_ehdr->e_shoff); + break; + + default: + pr_debug("Unknown ELF class.\n"); + return -EINVAL; + } + return elf_is_ehdr_sane(ehdr, len) ? 0 : -ENOEXEC; } @@ -179,6 +192,7 @@ static int elf_read_phdr(const char *buf, size_t len, { /* Override the const in proghdrs, we are the ones doing the loading. */ struct elf_phdr *phdr = (struct elf_phdr *) &elf_info->proghdrs[idx]; + const struct elfhdr *ehdr = elf_info->ehdr; const char *pbuf; struct elf_phdr *buf_phdr; @@ -186,18 +200,31 @@ static int elf_read_phdr(const char *buf, size_t len, buf_phdr = (struct elf_phdr *) pbuf; phdr->p_type = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_type); - phdr->p_offset = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_offset); - phdr->p_paddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_paddr); - phdr->p_vaddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr); phdr->p_flags = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_flags); - /* -* The following fields have a type equivalent to Elf_Addr -* both in 32 bit and 64 bit ELF. -*/ - phdr->p_filesz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_filesz); - phdr->p_memsz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_memsz); - phdr->p_align = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_align); + switch (ehdr->e_ident[EI_CLASS]) { + case ELFCLASS64: + phdr->p_offset = elf64_to_cpu(ehdr, buf_phdr->p_offset); + phdr->p_paddr = elf64_to_cpu(ehdr, buf_phdr->p_paddr); + phdr->p_vaddr = elf64_to_cpu(ehdr, buf_phdr->p_vaddr); + phdr->p_filesz = elf64_to_cpu(ehdr, buf_phdr->p_filesz); + phdr->p_memsz = elf64_to_cpu(ehdr, buf_phdr->p_memsz); + phdr->p_align = elf64_to_cpu(ehdr, buf_phdr->p_align); + break; + + case ELFCLASS32: + phdr->p_offset = elf32_to_cpu(ehdr, buf_phdr->p_offset); + phdr->p_paddr = elf32_to_cpu(ehdr, buf_phdr->p_paddr); + phdr->p_vaddr = elf32_to_cpu(ehdr, buf_phdr->p_vaddr); + phdr->p_filesz = elf32_to_cpu(ehdr, buf_phdr->p_filesz); + phdr->p_memsz = elf32_to_cpu(ehdr, buf_phdr->p_memsz); + phdr->p_align = elf32_to_cpu(ehdr, buf_phdr->p_align); + break; + + default: + pr_debug("Unknown ELF class.\n"); + return -EINVAL; + } return elf_is_phdr_sane(phdr, len) ? 0 : -ENOEXEC; } -- 2.20.1
Re: [PATCH v3 5/7] kexec_elf: remove elf_addr_to_cpu macro
Hi Michael, On Thu, Jul 11, 2019 at 09:08:51PM +1000, Michael Ellerman wrote: > Sven Schnelle writes: > > On Wed, Jul 10, 2019 at 05:09:29PM +0200, Christophe Leroy wrote: > >> Le 10/07/2019 à 16:29, Sven Schnelle a écrit : > >> > It had only one definition, so just use the function directly. > >> > >> It had only one definition because it was for ppc64 only. > >> But as far as I understand (at least from the name of the new file), you > >> want it to be generic, don't you ? Therefore I get on 32 bits it would be > >> elf32_to_cpu(). > > > > That brings up the question whether we need those endianess conversions. I > > would > > assume that the ELF file has always the same endianess as the running > > kernel. So > > i think we could just drop them. What do you think? > > We should be able to kexec from big to little endian or vice versa, so > they are necessary. I'll update the patch to check for a needed 32/64 bit conversion during runtime, so we can also kexec from 32 to 64 bit kernels and vice versa. Don't know whether that's possible on powerpc, but at least on parisc it is. Regards Sven
Re: [PATCH v3 5/7] kexec_elf: remove elf_addr_to_cpu macro
Hi Christophe, On Wed, Jul 10, 2019 at 05:09:29PM +0200, Christophe Leroy wrote: > > > Le 10/07/2019 à 16:29, Sven Schnelle a écrit : > > It had only one definition, so just use the function directly. > > It had only one definition because it was for ppc64 only. > But as far as I understand (at least from the name of the new file), you > want it to be generic, don't you ? Therefore I get on 32 bits it would be > elf32_to_cpu(). That brings up the question whether we need those endianess conversions. I would assume that the ELF file has always the same endianess as the running kernel. So i think we could just drop them. What do you think? Regards Sven
[PATCH v3 3/7] kexec_elf: remove parsing of section headers
We're not using them, so we can drop the parsing. Signed-off-by: Sven Schnelle --- include/linux/kexec.h | 1 - kernel/kexec_elf.c| 137 -- 2 files changed, 138 deletions(-) diff --git a/include/linux/kexec.h b/include/linux/kexec.h index da2a6b1d69e7..f0b809258ed3 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -226,7 +226,6 @@ struct kexec_elf_info { const struct elfhdr *ehdr; const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; }; int kexec_build_elf_info(const char *buf, size_t len, struct elfhdr *ehdr, diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 76e7df64d715..effe9dc0b055 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -244,134 +244,6 @@ static int elf_read_phdrs(const char *buf, size_t len, return 0; } -/** - * elf_is_shdr_sane - check that it is safe to use the section header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_shdr_sane(const struct elf_shdr *shdr, size_t buf_len) -{ - bool size_ok; - - /* SHT_NULL headers have undefined values, so we can't check them. */ - if (shdr->sh_type == SHT_NULL) - return true; - - /* Now verify sh_entsize */ - switch (shdr->sh_type) { - case SHT_SYMTAB: - size_ok = shdr->sh_entsize == sizeof(Elf_Sym); - break; - case SHT_RELA: - size_ok = shdr->sh_entsize == sizeof(Elf_Rela); - break; - case SHT_DYNAMIC: - size_ok = shdr->sh_entsize == sizeof(Elf_Dyn); - break; - case SHT_REL: - size_ok = shdr->sh_entsize == sizeof(Elf_Rel); - break; - case SHT_NOTE: - case SHT_PROGBITS: - case SHT_HASH: - case SHT_NOBITS: - default: - /* -* This is a section whose entsize requirements -* I don't care about. If I don't know about -* the section I can't care about it's entsize -* requirements. -*/ - size_ok = true; - break; - } - - if (!size_ok) { - pr_debug("ELF section with wrong entry size.\n"); - return false; - } else if (shdr->sh_addr + shdr->sh_size < shdr->sh_addr) { - pr_debug("ELF section address wraps around.\n"); - return false; - } - - if (shdr->sh_type != SHT_NOBITS) { - if (shdr->sh_offset + shdr->sh_size < shdr->sh_offset) { - pr_debug("ELF section location wraps around.\n"); - return false; - } else if (shdr->sh_offset + shdr->sh_size > buf_len) { - pr_debug("ELF section not in file.\n"); - return false; - } - } - - return true; -} - -static int elf_read_shdr(const char *buf, size_t len, -struct kexec_elf_info *elf_info, -int idx) -{ - struct elf_shdr *shdr = &elf_info->sechdrs[idx]; - const struct elfhdr *ehdr = elf_info->ehdr; - const char *sbuf; - struct elf_shdr *buf_shdr; - - sbuf = buf + ehdr->e_shoff + idx * sizeof(*buf_shdr); - buf_shdr = (struct elf_shdr *) sbuf; - - shdr->sh_name = elf32_to_cpu(ehdr, buf_shdr->sh_name); - shdr->sh_type = elf32_to_cpu(ehdr, buf_shdr->sh_type); - shdr->sh_addr = elf_addr_to_cpu(ehdr, buf_shdr->sh_addr); - shdr->sh_offset= elf_addr_to_cpu(ehdr, buf_shdr->sh_offset); - shdr->sh_link = elf32_to_cpu(ehdr, buf_shdr->sh_link); - shdr->sh_info = elf32_to_cpu(ehdr, buf_shdr->sh_info); - - /* -* The following fields have a type equivalent to Elf_Addr -* both in 32 bit and 64 bit ELF. -*/ - shdr->sh_flags = elf_addr_to_cpu(ehdr, buf_shdr->sh_flags); - shdr->sh_size = elf_addr_to_cpu(ehdr, buf_shdr->sh_size); - shdr->sh_addralign = elf_addr_to_cpu(ehdr, buf_shdr->sh_addralign); - shdr->sh_entsize = elf_addr_to_cpu(ehdr, buf_shdr->sh_entsize); - - return elf_is_shdr_sane(shdr, len) ? 0 : -ENOEXEC; -} - -/** - * elf_read_shdrs - read the section headers from the buffer - * - * This function assumes that the section header table was checked for sanity. - * Use elf_is_ehdr_sane() if it wasn't. - */ -static int elf_read_shdrs(const char *buf, size_t len, - struct kexec_elf_info *elf_info) -{ - size_t shdr_size, i; - - /* -* e_shnum is at most 65536 so calculating -* the size of the section header cannot overflow. -*/ - shd
[PATCH v3 7/7] kexec_elf: remove unused variable in kexec_elf_load()
base was never unsigned, so we can remove it. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index b7e47ddd7cad..a56ec5481e71 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -348,7 +348,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, struct kexec_buf *kbuf, unsigned long *lowest_load_addr) { - unsigned long base = 0, lowest_addr = UINT_MAX; + unsigned long lowest_addr = UINT_MAX; int ret; size_t i; @@ -370,7 +370,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, kbuf->bufsz = size; kbuf->memsz = phdr->p_memsz; kbuf->buf_align = phdr->p_align; - kbuf->buf_min = phdr->p_paddr + base; + kbuf->buf_min = phdr->p_paddr; ret = kexec_add_buffer(kbuf); if (ret) goto out; @@ -380,9 +380,6 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, lowest_addr = load_addr; } - /* Update entry point to reflect new load address. */ - ehdr->e_entry += base; - *lowest_load_addr = lowest_addr; ret = 0; out: -- 2.20.1
[PATCH v3 5/7] kexec_elf: remove elf_addr_to_cpu macro
It had only one definition, so just use the function directly. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 70d31b8feeae..99e6d63b5dfc 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,8 +8,6 @@ #include #include -#define elf_addr_to_cpuelf64_to_cpu - #ifndef Elf_Rel #define Elf_RelElf64_Rel #endif /* Elf_Rel */ @@ -143,9 +141,9 @@ static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr) ehdr->e_type = elf16_to_cpu(ehdr, buf_ehdr->e_type); ehdr->e_machine = elf16_to_cpu(ehdr, buf_ehdr->e_machine); ehdr->e_version = elf32_to_cpu(ehdr, buf_ehdr->e_version); - ehdr->e_entry = elf_addr_to_cpu(ehdr, buf_ehdr->e_entry); - ehdr->e_phoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_phoff); - ehdr->e_shoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_shoff); + ehdr->e_entry = elf64_to_cpu(ehdr, buf_ehdr->e_entry); + ehdr->e_phoff = elf64_to_cpu(ehdr, buf_ehdr->e_phoff); + ehdr->e_shoff = elf64_to_cpu(ehdr, buf_ehdr->e_shoff); ehdr->e_flags = elf32_to_cpu(ehdr, buf_ehdr->e_flags); ehdr->e_phentsize = elf16_to_cpu(ehdr, buf_ehdr->e_phentsize); ehdr->e_phnum = elf16_to_cpu(ehdr, buf_ehdr->e_phnum); @@ -190,18 +188,18 @@ static int elf_read_phdr(const char *buf, size_t len, buf_phdr = (struct elf_phdr *) pbuf; phdr->p_type = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_type); - phdr->p_offset = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_offset); - phdr->p_paddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_paddr); - phdr->p_vaddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr); + phdr->p_offset = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_offset); + phdr->p_paddr = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_paddr); + phdr->p_vaddr = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr); phdr->p_flags = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_flags); /* * The following fields have a type equivalent to Elf_Addr * both in 32 bit and 64 bit ELF. */ - phdr->p_filesz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_filesz); - phdr->p_memsz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_memsz); - phdr->p_align = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_align); + phdr->p_filesz = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_filesz); + phdr->p_memsz = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_memsz); + phdr->p_align = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_align); return elf_is_phdr_sane(phdr, len) ? 0 : -ENOEXEC; } -- 2.20.1
[PATCH v3 4/7] kexec_elf: remove PURGATORY_STACK_SIZE
It's not used anywhere so just drop it. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index effe9dc0b055..70d31b8feeae 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,8 +8,6 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - #define elf_addr_to_cpuelf64_to_cpu #ifndef Elf_Rel -- 2.20.1
[PATCH v3 2/7] kexec_elf: change order of elf_*_to_cpu() functions
Change the order to have a 64/32/16 order, no functional change. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 6e9f52171ede..76e7df64d715 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -31,22 +31,22 @@ static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) return value; } -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) +static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) { if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); + value = le32_to_cpu(value); else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); + value = be32_to_cpu(value); return value; } -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) +static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) { if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); + value = le16_to_cpu(value); else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); + value = be16_to_cpu(value); return value; } -- 2.20.1
[PATCH v3 1/7] kexec: add KEXEC_ELF
Right now powerpc provides an implementation to read elf files with the kexec_file() syscall. Make that available as a public kexec interface so it can be re-used on other architectures. Signed-off-by: Sven Schnelle --- arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c| 551 +- include/linux/kexec.h | 24 + kernel/Makefile | 1 + .../kexec_elf_64.c => kernel/kexec_elf.c | 199 ++- 6 files changed, 75 insertions(+), 704 deletions(-) copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_elf.c (71%) diff --git a/arch/Kconfig b/arch/Kconfig index c47b328eada0..30694aca4316 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 12cee37f15c4..addc2dad78e0 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -510,6 +510,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..30bd57a93c17 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0-only /* * Load ELF vmlinux file for the kexec_file_load syscall. * @@ -10,15 +11,6 @@ * Based on kexec-tools' kexec-elf-exec.c and kexec-elf-ppc64.c. * Heavily modified for the kernel by * Thiago Jung Bauermann . - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation (version 2 of the License). - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #define pr_fmt(fmt)"kexec_elf: " fmt @@ -39,532 +31,6 @@ #define Elf_RelElf64_Rel #endif /* Elf_Rel */ -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le64_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be64_to_cpu(value); - - return value; -} - -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); - - return value; -} - -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); - - return value; -} - -/** - * elf_is_ehdr_sane - check that it is safe to use the ELF header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_t buf_len) -{ - if (ehdr->e_phnum > 0 && ehdr->e_phentsize != sizeof(struct elf_phdr)) { - pr_debug("Bad program header size.\n"); - return false; - } else if (ehdr->e_shnum > 0 && - ehdr->e_shentsize != sizeof(struct elf_shdr)) { - pr_debug("Bad section header size.\n"); - return false; - } else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT || - ehdr->e_version != EV_CURRENT) { - pr_debug("Unknown ELF version.\n"); - return false; - } - - if (ehdr->e_phoff > 0 && ehdr->e_phnum > 0) { - size_t phdr_size; - - /* -* e_phnum is at most 65535 so calculating the size of the -* program header cannot overflow. -*/ - phdr_size = sizeof(struct elf_phdr) * ehdr-&g
[PATCH v3 6/7] kexec_elf: remove Elf_Rel macro
It wasn't used anywhere, so lets drop it. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 4 1 file changed, 4 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 99e6d63b5dfc..b7e47ddd7cad 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,10 +8,6 @@ #include #include -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - static inline bool elf_is_elf_file(const struct elfhdr *ehdr) { return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -- 2.20.1
[PATCH v3 0/7] kexec: add generic support for elf kernel images
Hi List, this is the same changeset as v2, but (hopefully) with git format-patch -C. Changes to v2: - use git format-patch -C Changes to v1: - split up patch into smaller pieces - rebase onto powerpc/next - remove unused variable in kexec_elf_load() Changes to RFC version: - remove unused Elf_Rel macro - remove section header parsing - remove PURGATORY_STACK_SIZE - change order of elf_*_to_cpu() functions - remove elf_addr_to_cpu macro Sven Schnelle (7): kexec: add KEXEC_ELF kexec_elf: change order of elf_*_to_cpu() functions kexec_elf: remove parsing of section headers kexec_elf: remove PURGATORY_STACK_SIZE kexec_elf: remove elf_addr_to_cpu macro kexec_elf: remove Elf_Rel macro kexec_elf: remove unused variable in kexec_elf_load() arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 551 + include/linux/kexec.h | 23 ++ kernel/Makefile| 1 + kernel/kexec_elf.c | 389 6 files changed, 427 insertions(+), 541 deletions(-) create mode 100644 kernel/kexec_elf.c -- 2.20.1
Re: [PATCH v2 1/7] kexec: add KEXEC_ELF
Hi Christophe, On Wed, Jul 10, 2019 at 01:19:13PM +, Christophe Leroy wrote: > Hi Sven, > > On 07/09/2019 07:43 PM, Sven Schnelle wrote: > > Right now powerpc provides an implementation to read elf files > > with the kexec_file() syscall. Make that available as a public > > kexec interface so it can be re-used on other architectures. > > > > Signed-off-by: Sven Schnelle > > --- > > arch/Kconfig | 3 + > > arch/powerpc/Kconfig | 1 + > > arch/powerpc/kernel/kexec_elf_64.c | 551 + > > include/linux/kexec.h | 24 ++ > > kernel/Makefile| 1 + > > kernel/kexec_elf.c | 537 > > 6 files changed, 576 insertions(+), 541 deletions(-) > > create mode 100644 kernel/kexec_elf.c > > Why are you persisting at not using -C when creating your patch ? Do you > want to hide the changes you did while copying > arch/powerpc/kernel/kexec_elf_64.c to kernel/kexec_elf.c ? > Or you want to make life harder for the reviewers ? Sorry, never used -C before. I used: git send-email --annotate -v2 -7 --to ke...@lists.infradead.org --cc del...@gmx.de,linuxppc-dev@lists.ozlabs.org -v --format-patch -C -M However, it looks like it only works when started this way: git send-email --format-patch -M -C --annotate -v2 -7 --to ke...@lists.infradead.org --cc del...@gmx.de,linuxppc-dev@lists.ozlabs.org -v I'll resend v2. Best Regards, Sven
[PATCH v2 3/7] kexec_elf: remove parsing of section headers
We're not using them, so we can drop the parsing. Signed-off-by: Sven Schnelle --- include/linux/kexec.h | 1 - kernel/kexec_elf.c| 137 -- 2 files changed, 138 deletions(-) diff --git a/include/linux/kexec.h b/include/linux/kexec.h index da2a6b1d69e7..f0b809258ed3 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -226,7 +226,6 @@ struct kexec_elf_info { const struct elfhdr *ehdr; const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; }; int kexec_build_elf_info(const char *buf, size_t len, struct elfhdr *ehdr, diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 76e7df64d715..effe9dc0b055 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -244,134 +244,6 @@ static int elf_read_phdrs(const char *buf, size_t len, return 0; } -/** - * elf_is_shdr_sane - check that it is safe to use the section header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_shdr_sane(const struct elf_shdr *shdr, size_t buf_len) -{ - bool size_ok; - - /* SHT_NULL headers have undefined values, so we can't check them. */ - if (shdr->sh_type == SHT_NULL) - return true; - - /* Now verify sh_entsize */ - switch (shdr->sh_type) { - case SHT_SYMTAB: - size_ok = shdr->sh_entsize == sizeof(Elf_Sym); - break; - case SHT_RELA: - size_ok = shdr->sh_entsize == sizeof(Elf_Rela); - break; - case SHT_DYNAMIC: - size_ok = shdr->sh_entsize == sizeof(Elf_Dyn); - break; - case SHT_REL: - size_ok = shdr->sh_entsize == sizeof(Elf_Rel); - break; - case SHT_NOTE: - case SHT_PROGBITS: - case SHT_HASH: - case SHT_NOBITS: - default: - /* -* This is a section whose entsize requirements -* I don't care about. If I don't know about -* the section I can't care about it's entsize -* requirements. -*/ - size_ok = true; - break; - } - - if (!size_ok) { - pr_debug("ELF section with wrong entry size.\n"); - return false; - } else if (shdr->sh_addr + shdr->sh_size < shdr->sh_addr) { - pr_debug("ELF section address wraps around.\n"); - return false; - } - - if (shdr->sh_type != SHT_NOBITS) { - if (shdr->sh_offset + shdr->sh_size < shdr->sh_offset) { - pr_debug("ELF section location wraps around.\n"); - return false; - } else if (shdr->sh_offset + shdr->sh_size > buf_len) { - pr_debug("ELF section not in file.\n"); - return false; - } - } - - return true; -} - -static int elf_read_shdr(const char *buf, size_t len, -struct kexec_elf_info *elf_info, -int idx) -{ - struct elf_shdr *shdr = &elf_info->sechdrs[idx]; - const struct elfhdr *ehdr = elf_info->ehdr; - const char *sbuf; - struct elf_shdr *buf_shdr; - - sbuf = buf + ehdr->e_shoff + idx * sizeof(*buf_shdr); - buf_shdr = (struct elf_shdr *) sbuf; - - shdr->sh_name = elf32_to_cpu(ehdr, buf_shdr->sh_name); - shdr->sh_type = elf32_to_cpu(ehdr, buf_shdr->sh_type); - shdr->sh_addr = elf_addr_to_cpu(ehdr, buf_shdr->sh_addr); - shdr->sh_offset= elf_addr_to_cpu(ehdr, buf_shdr->sh_offset); - shdr->sh_link = elf32_to_cpu(ehdr, buf_shdr->sh_link); - shdr->sh_info = elf32_to_cpu(ehdr, buf_shdr->sh_info); - - /* -* The following fields have a type equivalent to Elf_Addr -* both in 32 bit and 64 bit ELF. -*/ - shdr->sh_flags = elf_addr_to_cpu(ehdr, buf_shdr->sh_flags); - shdr->sh_size = elf_addr_to_cpu(ehdr, buf_shdr->sh_size); - shdr->sh_addralign = elf_addr_to_cpu(ehdr, buf_shdr->sh_addralign); - shdr->sh_entsize = elf_addr_to_cpu(ehdr, buf_shdr->sh_entsize); - - return elf_is_shdr_sane(shdr, len) ? 0 : -ENOEXEC; -} - -/** - * elf_read_shdrs - read the section headers from the buffer - * - * This function assumes that the section header table was checked for sanity. - * Use elf_is_ehdr_sane() if it wasn't. - */ -static int elf_read_shdrs(const char *buf, size_t len, - struct kexec_elf_info *elf_info) -{ - size_t shdr_size, i; - - /* -* e_shnum is at most 65536 so calculating -* the size of the section header cannot overflow. -*/ - shd
[PATCH v2 6/7] kexec_elf: remove Elf_Rel macro
It wasn't used anywhere, so lets drop it. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 4 1 file changed, 4 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 99e6d63b5dfc..b7e47ddd7cad 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,10 +8,6 @@ #include #include -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - static inline bool elf_is_elf_file(const struct elfhdr *ehdr) { return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -- 2.20.1
[PATCH v2 2/7] kexec_elf: change order of elf_*_to_cpu() functions
Change the order to have a 64/32/16 order, no functional change. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 6e9f52171ede..76e7df64d715 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -31,22 +31,22 @@ static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) return value; } -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) +static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) { if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); + value = le32_to_cpu(value); else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); + value = be32_to_cpu(value); return value; } -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) +static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) { if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); + value = le16_to_cpu(value); else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); + value = be16_to_cpu(value); return value; } -- 2.20.1
[PATCH v2 1/7] kexec: add KEXEC_ELF
Right now powerpc provides an implementation to read elf files with the kexec_file() syscall. Make that available as a public kexec interface so it can be re-used on other architectures. Signed-off-by: Sven Schnelle --- arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 551 + include/linux/kexec.h | 24 ++ kernel/Makefile| 1 + kernel/kexec_elf.c | 537 6 files changed, 576 insertions(+), 541 deletions(-) create mode 100644 kernel/kexec_elf.c diff --git a/arch/Kconfig b/arch/Kconfig index c47b328eada0..30694aca4316 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 12cee37f15c4..addc2dad78e0 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -510,6 +510,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..30bd57a93c17 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0-only /* * Load ELF vmlinux file for the kexec_file_load syscall. * @@ -10,15 +11,6 @@ * Based on kexec-tools' kexec-elf-exec.c and kexec-elf-ppc64.c. * Heavily modified for the kernel by * Thiago Jung Bauermann . - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation (version 2 of the License). - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #define pr_fmt(fmt)"kexec_elf: " fmt @@ -39,532 +31,6 @@ #define Elf_RelElf64_Rel #endif /* Elf_Rel */ -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le64_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be64_to_cpu(value); - - return value; -} - -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); - - return value; -} - -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); - - return value; -} - -/** - * elf_is_ehdr_sane - check that it is safe to use the ELF header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_t buf_len) -{ - if (ehdr->e_phnum > 0 && ehdr->e_phentsize != sizeof(struct elf_phdr)) { - pr_debug("Bad program header size.\n"); - return false; - } else if (ehdr->e_shnum > 0 && - ehdr->e_shentsize != sizeof(struct elf_shdr)) { - pr_debug("Bad section header size.\n"); - return false; - } else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT || - ehdr->e_version != EV_CURRENT) { - pr_debug("Unknown ELF version.\n"); - return false; - } - - if (ehdr->e_phoff > 0 && ehdr->e_phnum > 0) { - size_t phdr_size; - - /* -* e_phnum is at most 65535 so calculating the size of the -* program header cannot overflow. -*/ - phdr_size = sizeof(struct elf_phdr) * ehdr->e_phnum; - - /* Sanity check the program header table l
[PATCH v2 4/7] kexec_elf: remove PURGATORY_STACK_SIZE
It's not used anywhere so just drop it. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index effe9dc0b055..70d31b8feeae 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,8 +8,6 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - #define elf_addr_to_cpuelf64_to_cpu #ifndef Elf_Rel -- 2.20.1
[PATCH v2 5/7] kexec_elf: remove elf_addr_to_cpu macro
It had only one definition, so just use the function directly. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 70d31b8feeae..99e6d63b5dfc 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,8 +8,6 @@ #include #include -#define elf_addr_to_cpuelf64_to_cpu - #ifndef Elf_Rel #define Elf_RelElf64_Rel #endif /* Elf_Rel */ @@ -143,9 +141,9 @@ static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr) ehdr->e_type = elf16_to_cpu(ehdr, buf_ehdr->e_type); ehdr->e_machine = elf16_to_cpu(ehdr, buf_ehdr->e_machine); ehdr->e_version = elf32_to_cpu(ehdr, buf_ehdr->e_version); - ehdr->e_entry = elf_addr_to_cpu(ehdr, buf_ehdr->e_entry); - ehdr->e_phoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_phoff); - ehdr->e_shoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_shoff); + ehdr->e_entry = elf64_to_cpu(ehdr, buf_ehdr->e_entry); + ehdr->e_phoff = elf64_to_cpu(ehdr, buf_ehdr->e_phoff); + ehdr->e_shoff = elf64_to_cpu(ehdr, buf_ehdr->e_shoff); ehdr->e_flags = elf32_to_cpu(ehdr, buf_ehdr->e_flags); ehdr->e_phentsize = elf16_to_cpu(ehdr, buf_ehdr->e_phentsize); ehdr->e_phnum = elf16_to_cpu(ehdr, buf_ehdr->e_phnum); @@ -190,18 +188,18 @@ static int elf_read_phdr(const char *buf, size_t len, buf_phdr = (struct elf_phdr *) pbuf; phdr->p_type = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_type); - phdr->p_offset = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_offset); - phdr->p_paddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_paddr); - phdr->p_vaddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr); + phdr->p_offset = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_offset); + phdr->p_paddr = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_paddr); + phdr->p_vaddr = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr); phdr->p_flags = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_flags); /* * The following fields have a type equivalent to Elf_Addr * both in 32 bit and 64 bit ELF. */ - phdr->p_filesz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_filesz); - phdr->p_memsz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_memsz); - phdr->p_align = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_align); + phdr->p_filesz = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_filesz); + phdr->p_memsz = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_memsz); + phdr->p_align = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_align); return elf_is_phdr_sane(phdr, len) ? 0 : -ENOEXEC; } -- 2.20.1
[PATCH v2 0/7] kexec: add generic support for elf kernel images
Hi List, i've split up the patch a bit more. The first one move the generic elf stuff out of arch/powerpc to linux/kexec_elf.c, and prefixes the exposed functions with kexec_. That other patches remove stuff that is not used as proposed in the review. Changes to v1: - split up patch into smaller pieces - rebase onto powerpc/next - remove unused variable in kexec_elf_load() Changes to RFC version: - remove unused Elf_Rel macro - remove section header parsing - remove PURGATORY_STACK_SIZE - change order of elf_*_to_cpu() functions - remove elf_addr_to_cpu macro Sven Schnelle (7): kexec: add KEXEC_ELF kexec_elf: change order of elf_*_to_cpu() functions kexec_elf: remove parsing of section headers kexec_elf: remove PURGATORY_STACK_SIZE kexec_elf: remove elf_addr_to_cpu macro kexec_elf: remove Elf_Rel macro kexec_elf: remove unused variable in kexec_elf_load() arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 551 + include/linux/kexec.h | 23 ++ kernel/Makefile| 1 + kernel/kexec_elf.c | 389 6 files changed, 427 insertions(+), 541 deletions(-) create mode 100644 kernel/kexec_elf.c -- 2.20.1
[PATCH v2 7/7] kexec_elf: remove unused variable in kexec_elf_load()
base was never unsigned, so we can remove it. Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index b7e47ddd7cad..a56ec5481e71 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -348,7 +348,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, struct kexec_buf *kbuf, unsigned long *lowest_load_addr) { - unsigned long base = 0, lowest_addr = UINT_MAX; + unsigned long lowest_addr = UINT_MAX; int ret; size_t i; @@ -370,7 +370,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, kbuf->bufsz = size; kbuf->memsz = phdr->p_memsz; kbuf->buf_align = phdr->p_align; - kbuf->buf_min = phdr->p_paddr + base; + kbuf->buf_min = phdr->p_paddr; ret = kexec_add_buffer(kbuf); if (ret) goto out; @@ -380,9 +380,6 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, lowest_addr = load_addr; } - /* Update entry point to reflect new load address. */ - ehdr->e_entry += base; - *lowest_load_addr = lowest_addr; ret = 0; out: -- 2.20.1
Re: [PATCH using git format-patch -C] kexec: add generic support for elf kernel images
Hi Christophe, On Mon, Jul 08, 2019 at 12:40:52PM +0200, Christophe Leroy wrote: > > From: Sven Schnelle > > Please describe you patch. > > All the changes you are doing to the ppc64 version in order to make it > generic should be described. > > Those changes are also maybe worth splitting the patch in several parts, > either preparing the ppc64 for generic then moving to kernel/kexec_elf.c or > moving to kernel/kexec_elf.c without modifications, then modifying it to > make it generic. > > Note that your patch only applies on Linux 5.1, it doesn't apply on > powerpc/next. > > I think it also be worth taking the opportunity to fix the sparse warnings, > see https://patchwork.ozlabs.org/patch/1128720/ > > checkpatch comments should also be fixed, see > https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/8044//artifact/linux/checkpatch.log Actually this patch shouldn't have been sent out, that was my fault. Appologies for that. Thanks for your review, i will split up the patch and resent it. Regards Sven
[PATCH] kexec: add generic support for elf kernel images
Signed-off-by: Sven Schnelle --- arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 547 + include/linux/kexec.h | 35 ++ kernel/Makefile| 1 + kernel/kexec_elf.c | 540 6 files changed, 588 insertions(+), 539 deletions(-) create mode 100644 kernel/kexec_elf.c diff --git a/arch/Kconfig b/arch/Kconfig index c47b328eada0..30694aca4316 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 8c1c636308c8..97aa81622452 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -502,6 +502,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..d062c5991722 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -31,539 +31,7 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - -#define elf_addr_to_cpuelf64_to_cpu - -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le64_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be64_to_cpu(value); - - return value; -} - -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); - - return value; -} - -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); - - return value; -} - -/** - * elf_is_ehdr_sane - check that it is safe to use the ELF header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_t buf_len) -{ - if (ehdr->e_phnum > 0 && ehdr->e_phentsize != sizeof(struct elf_phdr)) { - pr_debug("Bad program header size.\n"); - return false; - } else if (ehdr->e_shnum > 0 && - ehdr->e_shentsize != sizeof(struct elf_shdr)) { - pr_debug("Bad section header size.\n"); - return false; - } else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT || - ehdr->e_version != EV_CURRENT) { - pr_debug("Unknown ELF version.\n"); - return false; - } - - if (ehdr->e_phoff > 0 && ehdr->e_phnum > 0) { - size_t phdr_size; - - /* -* e_phnum is at most 65535 so calculating the size of the -* program header cannot overflow. -*/ - phdr_size = sizeof(struct elf_phdr) * ehdr->e_phnum; - - /* Sanity check the program header table location. */ - if (ehdr->e_phoff + phdr_size < ehdr->e_phoff) { - pr_debug("Program headers at invalid location.\n"); - return false; - } else if (ehdr->e_phoff + phdr_size > buf_len) { - pr_debug("Program headers truncated.\n"); - return false; - } - } - - if (ehdr->e_shoff > 0 && ehdr->e_shnum > 0) { - size_t shdr_size; - - /* -* e_shnum is at most 65536 so calculating -* the size of the section header cannot overflow. -*/ - shdr_size = sizeof(struct elf_shdr) * ehdr->e_shnum; - - /* Sanity check the section header table location. */ -
Re: [PATCH RFC] generic ELF support for kexec
Hi Philipp, On Mon, Jul 01, 2019 at 02:31:20PM +0200, Philipp Rudo wrote: > Sven Schnelle wrote: > > > I'm attaching the patch to this Mail. What do you think about that change? > > s390 also uses ELF files, and (maybe?) could also switch to this > > implementation. > > But i don't know anything about S/390 and don't have one in my basement. So > > i'll leave s390 to the IBM folks. > > I'm afraid there isn't much code here s390 can reuse. I see multiple problems > in kexec_elf_load: > > * while loading the phdrs we also need to setup some data structures to pass > to the next kernel > * the s390 kernel needs to be loaded to a fixed address > * there is no support to load a crash kernel > > Of course that could all be fixed/worked around by introducing some arch > hooks. > But when you take into account that the whole elf loader on s390 is ~100 lines > of code, I don't think it is worth it. That's fine. I didn't really look into the S/390 Loader, and just wanted to let the IBM people know. > More comments below. > > [...] > > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > > index b9b1bc5f9669..49b23b425f84 100644 > > --- a/include/linux/kexec.h > > +++ b/include/linux/kexec.h > > @@ -216,6 +216,41 @@ extern int crash_prepare_elf64_headers(struct > > crash_mem *mem, int kernel_map, > >void **addr, unsigned long *sz); > > #endif /* CONFIG_KEXEC_FILE */ > > > > +#ifdef CONFIG_KEXEC_FILE_ELF > > + > > +struct kexec_elf_info { > > + /* > > +* Where the ELF binary contents are kept. > > +* Memory managed by the user of the struct. > > +*/ > > + const char *buffer; > > + > > + const struct elfhdr *ehdr; > > + const struct elf_phdr *proghdrs; > > + struct elf_shdr *sechdrs; > > +}; > > Do i understand this right? elf_info->buffer contains the full elf file and > elf_info->ehdr is a (endianness translated) copy of the files ehdr? > > If so ... > > > +void kexec_free_elf_info(struct kexec_elf_info *elf_info); > > + > > +int kexec_build_elf_info(const char *buf, size_t len, struct elfhdr *ehdr, > > + struct kexec_elf_info *elf_info); > > + > > +int kexec_elf_kernel_load(struct kimage *image, struct kexec_buf *kbuf, > > + char *kernel_buf, unsigned long kernel_len, > > + unsigned long *kernel_load_addr); > > + > > +int kexec_elf_probe(const char *buf, unsigned long len); > > + > > +int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, > > +struct kexec_elf_info *elf_info, > > +struct kexec_buf *kbuf, > > +unsigned long *lowest_load_addr); > > + > > +int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, > > +struct kexec_elf_info *elf_info, > > +struct kexec_buf *kbuf, > > +unsigned long *lowest_load_addr); > > ... you could simplify the arguments by dropping the ehdr argument. The > elf_info should contain all the information needed. Furthermore the kexec_buf > also contains a pointer to its kimage. So you can drop that argument as well. > > An other thing is that you kzalloc the memory needed for proghdrs and sechdrs > but expect the user of those functions to provide the memory needed for ehdr. > Wouldn't it be more consistent to also kzalloc the ehdr? > Good point. I'll think about it. I would like to do that in an extra patch, as it is not a small style change. > > > diff --git a/kernel/kexec_file_elf.c b/kernel/kexec_file_elf.c > > new file mode 100644 > > index ..bb966c93492c > > --- /dev/null > > +++ b/kernel/kexec_file_elf.c > > @@ -0,0 +1,574 @@ > > [...] > > > +static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) > > +{ > > + if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) > > + value = le64_to_cpu(value); > > + else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) > > + value = be64_to_cpu(value); > > + > > + return value; > > +} > > + > > +static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) > > +{ > > + if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) > > + value = le16_to_cpu(value); > > + else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) > > + value = be16_to_cpu(value); > > + > > + return value; > > +} > >
Re: [PATCH RFC] generic ELF support for kexec
Hi Michael, On Fri, Jun 28, 2019 at 12:04:11PM +1000, Michael Ellerman wrote: > Sven Schnelle writes: > https://github.com/linuxppc/wiki/wiki/Booting-with-Qemu > > But I'm not sure where you get a version of kexec that uses kexec_file(). kexec-tools HEAD supports it, so that's not a problem. > > If that change is acceptable i would finish the patch and submit it. I think > > best would be to push this change through Helge's parisc tree, so we don't > > have any dependencies to sort out. > > That will work for you but could cause us problems if we have any > changes that touch that code. > > It's easy enough to create a topic branch with just that patch that both > of us merge. What should be the base branch for that patch? Christophe suggested the powerpc/merge branch? > > #include > > #include > > #include > > @@ -31,540 +29,6 @@ > > #include > > #include > > > > -#define PURGATORY_STACK_SIZE (16 * 1024) > > This is unused AFAICS. We should probably remove it explicitly rather > than as part of this patch. I have one patch right now. If wanted i can split up all the changes suggested during the review into smaller pieces, whatever you prefer. > Or that. > > > +#include > > +#include > > + > > +#define elf_addr_to_cpuelf64_to_cpu > > Why are we doing that rather than just using elf64_to_cpu directly? > > > +#ifndef Elf_Rel > > +#define Elf_RelElf64_Rel > > +#endif /* Elf_Rel */ > > And that? Don't know - ask the PPC people :-) Regards Sven
[PATCH RFC] generic ELF support for kexec
Hi List, i recently started working on kexec for PA-RISC. While doing so, i figured that powerpc already has support for reading ELF images inside of the Kernel. My first attempt was to steal the source code and modify it for PA-RISC, but it turned out that i didn't had to change much. Only ARM specific stuff like fdt blob fetching had to be removed. So instead of duplicating the code, i thought about moving the ELF stuff to the core kexec code, and exposing several function to use that code from the arch specific code. I'm attaching the patch to this Mail. What do you think about that change? s390 also uses ELF files, and (maybe?) could also switch to this implementation. But i don't know anything about S/390 and don't have one in my basement. So i'll leave s390 to the IBM folks. I haven't really tested PowerPC yet. Can anyone give me a helping hand what would be a good target to test this code in QEMU? Or even better, test this code on real Hardware? If that change is acceptable i would finish the patch and submit it. I think best would be to push this change through Helge's parisc tree, so we don't have any dependencies to sort out. Regards, Sven [PATCH] kexec: add generic support for elf kernel images Signed-off-by: Sven Schnelle --- arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 547 +-- include/linux/kexec.h | 35 ++ kernel/Makefile| 1 + kernel/kexec_file_elf.c| 574 + 6 files changed, 619 insertions(+), 542 deletions(-) create mode 100644 kernel/kexec_file_elf.c diff --git a/arch/Kconfig b/arch/Kconfig index c47b328eada0..de7520100136 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_FILE_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 8c1c636308c8..48241260b6ae 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -502,6 +502,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_FILE_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..0059e36913e9 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -21,8 +21,6 @@ * GNU General Public License for more details. */ -#define pr_fmt(fmt)"kexec_elf: " fmt - #include #include #include @@ -31,540 +29,6 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - -#define elf_addr_to_cpuelf64_to_cpu - -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le64_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be64_to_cpu(value); - - return value; -} - -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); - - return value; -} - -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); - - return value; -} - -/** - * elf_is_ehdr_sane - check that it is safe to use the ELF header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_t buf_len) -{ - if (ehdr->e_phnum > 0 && ehdr->e_phentsize != sizeof(struct elf_phdr)) { - pr_debug("Bad program header size.\n"); - return false; - } else if (ehdr->e_shnum > 0 && - ehdr->e_shentsize != sizeof(struct elf_shdr)) { - pr_debug("Bad section header size.\n"); - return false; - } else if (ehdr->e_ident[E