On Mon, Dec 06, 2021 at 12:20:47PM +0100, Philipp Rudo wrote:
> arch_kexec_apply_relocations_add currently ignores all errors returned
> by arch_kexec_do_relocs. This means that every unknown relocation is
> silently skipped causing unpredictable behavior while the relocated code
> runs. Fix this by checking for errors and fail kexec_file_load if an
> unknown relocation type is encountered.
>
> The problem was found after gcc changed its behavior and used
> R_390_PLT32DBL relocations for brasl instruction and relied on ld to
> resolve the relocations in the final link in case direct calls are
> possible. As the purgatory code is only linked partially (option -r)
> ld didn't resolve the relocations leaving them for arch_kexec_do_relocs.
> But arch_kexec_do_relocs doesn't know how to handle R_390_PLT32DBL
> relocations so they were silently skipped. This ultimately caused an
> endless loop in the purgatory as the brasl instructions kept branching
> to itself.
>
> Fixes: 71406883fd35 ("s390/kexec_file: Add kexec_file_load system call")
> Reported-by: Tao Liu <ltao@xxxxxxxxxx>
> Signed-off-by: Philipp Rudo <prudo@xxxxxxxxxx>
> ---
> arch/s390/kernel/machine_kexec_file.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
> index 9975ad200d74..0e1d646207dc 100644
> --- a/arch/s390/kernel/machine_kexec_file.c
> +++ b/arch/s390/kernel/machine_kexec_file.c
> @@ -292,6 +292,7 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> {
> Elf_Rela *relas;
> int i, r_type;
> + int ret;
>
> relas = (void *)pi->ehdr + relsec->sh_offset;
>
> @@ -326,7 +327,9 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> addr = section->sh_addr + relas[i].r_offset;
>
> r_type = ELF64_R_TYPE(relas[i].r_info);
> - arch_kexec_do_relocs(r_type, loc, val, addr);
> + ret = arch_kexec_do_relocs(r_type, loc, val, addr);
> + if (ret)
> + return -EINVAL;
I'd prefer if this would return -ENOEXEC, just to be consistent with
x86. And _maybe_ it would also make sense to print an error message,
including the failing relocation type?
Thanks,
Heiko
