Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'

2022-05-06 Thread Sven Schnelle
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

2022-02-16 Thread Sven Schnelle
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

2022-01-28 Thread Sven Schnelle
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

2022-01-28 Thread Sven Schnelle
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)

2022-01-27 Thread Sven Schnelle
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

2022-01-27 Thread Sven Schnelle
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

2022-01-27 Thread Sven Schnelle
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

2019-08-23 Thread Sven Schnelle
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

2019-08-23 Thread Sven Schnelle
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 *) _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

2019-08-23 Thread Sven Schnelle
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

2019-08-23 Thread Sven Schnelle
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 = _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.
-*/

[PATCH v5 1/7] kexec: add KEXEC_ELF

2019-08-23 Thread Sven Schnelle
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()

2019-08-23 Thread Sven Schnelle
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

2019-08-23 Thread Sven Schnelle
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

2019-08-23 Thread Sven Schnelle
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

2019-07-16 Thread Sven Schnelle
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

2019-07-15 Thread Sven Schnelle
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->e_p

[PATCH v4 0/7] kexec: add generic support for elf kernel images

2019-07-15 Thread Sven Schnelle
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

2019-07-15 Thread Sven Schnelle
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 = _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.
-*/
-   shdr_size = sizeof(struct elf_shdr) * elf_info

[PATCH v4 2/7] kexec_elf: change order of elf_*_to_cpu() functions

2019-07-15 Thread Sven Schnelle
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

2019-07-15 Thread Sven Schnelle
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()

2019-07-15 Thread Sven Schnelle
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

2019-07-15 Thread Sven Schnelle
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

2019-07-15 Thread Sven Schnelle
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 *) _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

2019-07-15 Thread Sven Schnelle
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

2019-07-10 Thread Sven Schnelle
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

2019-07-10 Thread Sven Schnelle
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 = _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.
-*/
-   shdr_size = sizeof(struct elf_shdr) * elf_info

[PATCH v3 7/7] kexec_elf: remove unused variable in kexec_elf_load()

2019-07-10 Thread Sven Schnelle
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

2019-07-10 Thread Sven Schnelle
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

2019-07-10 Thread Sven Schnelle
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

2019-07-10 Thread Sven Schnelle
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

2019-07-10 Thread Sven Schnelle
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->e_p

[PATCH v3 6/7] kexec_elf: remove Elf_Rel macro

2019-07-10 Thread Sven Schnelle
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

2019-07-10 Thread Sven Schnelle
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

2019-07-10 Thread Sven Schnelle
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

2019-07-09 Thread Sven Schnelle
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 = _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.
-*/
-   shdr_size = sizeof(struct elf_shdr) * elf_info

[PATCH v2 6/7] kexec_elf: remove Elf_Rel macro

2019-07-09 Thread Sven Schnelle
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

2019-07-09 Thread Sven Schnelle
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

2019-07-09 Thread Sven Schnelle
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 loc

[PATCH v2 4/7] kexec_elf: remove PURGATORY_STACK_SIZE

2019-07-09 Thread Sven Schnelle
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

2019-07-09 Thread Sven Schnelle
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

2019-07-09 Thread Sven Schnelle
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()

2019-07-09 Thread Sven Schnelle
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

2019-07-08 Thread Sven Schnelle
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

2019-07-07 Thread Sven Schnelle
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

2019-07-01 Thread Sven Schnelle
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;
> > +}
> > +
> > +static uint32_t elf32_to_cpu(const struct

Re: [PATCH RFC] generic ELF support for kexec

2019-07-01 Thread Sven Schnelle
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

2019-06-25 Thread Sven Schnelle
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[EI_VERSION] != EV_CURRENT ||
-