From patchwork Mon Nov 21 23:49:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thiago Jung Bauermann X-Patchwork-Id: 697457 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tN52Z6LHpz9snk for ; Tue, 22 Nov 2016 10:50:46 +1100 (AEDT) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3tN52Z5Ng7zDvkb for ; Tue, 22 Nov 2016 10:50:46 +1100 (AEDT) X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3tN51G3Z9yzDvkP for ; Tue, 22 Nov 2016 10:49:38 +1100 (AEDT) Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uALNnLfU085466 for ; Mon, 21 Nov 2016 18:49:36 -0500 Received: from e24smtp02.br.ibm.com (e24smtp02.br.ibm.com [32.104.18.86]) by mx0a-001b2d01.pphosted.com with ESMTP id 26v5eyxgd0-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 21 Nov 2016 18:49:36 -0500 Received: from localhost by e24smtp02.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 21 Nov 2016 21:49:33 -0200 Received: from d24dlp01.br.ibm.com (9.18.248.204) by e24smtp02.br.ibm.com (10.172.0.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 21 Nov 2016 21:49:30 -0200 Received: from d24relay01.br.ibm.com (d24relay01.br.ibm.com [9.8.31.16]) by d24dlp01.br.ibm.com (Postfix) with ESMTP id 4496A352005C for ; Mon, 21 Nov 2016 18:49:01 -0500 (EST) Received: from d24av01.br.ibm.com (d24av01.br.ibm.com [9.8.31.91]) by d24relay01.br.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id uALNnUi44784318 for ; Mon, 21 Nov 2016 21:49:30 -0200 Received: from d24av01.br.ibm.com (localhost [127.0.0.1]) by d24av01.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id uALNnTFJ007073 for ; Mon, 21 Nov 2016 21:49:30 -0200 Received: from morokweng.localnet ([9.80.194.198]) by d24av01.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id uALNnR8J007062; Mon, 21 Nov 2016 21:49:28 -0200 From: Thiago Jung Bauermann To: Dave Young Subject: Re: [PATCH v10 04/10] kexec_file: Add support for purgatory built as PIE. Date: Mon, 21 Nov 2016 21:49:23 -0200 User-Agent: KMail/5.1.3 (Linux/4.4.0-47-generic; KDE/5.23.0; x86_64; ; ) In-Reply-To: <20161120024546.GA4413@dhcp-128-65.nay.redhat.com> References: <1478748449-3894-1-git-send-email-bauerman@linux.vnet.ibm.com> <1478748449-3894-5-git-send-email-bauerman@linux.vnet.ibm.com> <20161120024546.GA4413@dhcp-128-65.nay.redhat.com> MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16112123-0020-0000-0000-0000026707FB X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16112123-0021-0000-0000-0000307D0806 Message-Id: <5009580.5GxAkTrMYA@morokweng> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-11-21_19:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1611210399 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stewart Smith , Stephen Rothwell , "H. Peter Anvin" , Baoquan He , x86@kernel.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Ingo Molnar , Paul Mackerras , Eric Biederman , Thomas Gleixner , Mimi Zohar , linuxppc-dev@lists.ozlabs.org, Andrew Morton , Vivek Goyal Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" Hello Dave, Thanks for your review. Am Sonntag, 20. November 2016, 10:45:46 BRST schrieb Dave Young: > On 11/10/16 at 01:27am, Thiago Jung Bauermann wrote: > > powerpc's purgatory.ro has 12 relocation types when built as > > a relocatable object. To implement support for them requires > > arch_kexec_apply_relocations_add to duplicate a lot of code with > > module_64.c:apply_relocate_add. > > > > When built as a Position Independent Executable there are only 4 > > relocation types in purgatory.ro, so it becomes practical for the powerpc > > implementation of kexec_file to have its own relocation implementation. > > > > Also, the purgatory is an executable and not an intermediary output from > > the compiler so it makes sense conceptually that it is easier to build > > it as a PIE than as a partially linked object. > > > > Apart from the greatly reduced number of relocations, there are two > > differences between a relocatable object and a PIE: > > > > 1. __kexec_load_purgatory needs to use the program headers rather than the > > > > section headers to figure out how to load the binary. > > > > 2. Symbol values are absolute addresses instead of relative to the > > > > start of the section. > > > > This patch adds the support needed in generic code for the differences > > above and allows powerpc to load and relocate a position independent > > purgatory. > > [snip] > > The kexec-tools machine_apply_elf_rel is pretty simple for ppc64, it is > not that complex. So could you look into simplify your kexec_file > implementation? I can try, but there is one fundamental issue here: powerpc position-dependent code relies more on relocations than x86 position-dependent code does, so there's a limit to how simple it can be made without switching to position- independent code. And it will always be more involved than it is on x86. BTW, building x86's purgatory as PIE results in it not having any relocation at all, so it's an advantage even in that architecture. Unfortunately, the machine locks up during reboot and I didn't have time to try to figure out what's going on. > kernel/kexec_file.c kexec_apply_relocations only do limited things > and some of the logic is in arch/x86, so move general code out of arch > code, then I guess the arch code will be simpler I agree that is a good idea. Is the patch below what you had in mind? > and then we probably do not need this PIE stuff anymore. If you are ok with the patch below I can post a new version of the series based on it and we can see if Michael Ellerman thinks it is enough. > BTW, __kexec_really_load_purgatory looks worse than > ___kexec_load_purgatory ;) Really? I find the special handling of bss makes the section-based loader a bit more confusing. diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c index 8c1f218926d7..f4860c408ece 100644 --- a/arch/x86/kernel/machine_kexec_64.c +++ b/arch/x86/kernel/machine_kexec_64.c @@ -401,143 +401,45 @@ int arch_kexec_kernel_verify_sig(struct kimage *image, void *kernel, } #endif -/* - * Apply purgatory relocations. - * - * ehdr: Pointer to elf headers - * sechdrs: Pointer to section headers. - * relsec: section index of SHT_RELA section. - * - * TODO: Some of the code belongs to generic code. Move that in kexec.c. - */ -int arch_kexec_apply_relocations_add(const Elf64_Ehdr *ehdr, - Elf64_Shdr *sechdrs, unsigned int relsec) +int arch_kexec_apply_relocation_add(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, + unsigned int reltype, Elf_Sym *sym, + const char *name, unsigned long *location, + unsigned long address, unsigned long value) { - unsigned int i; - Elf64_Rela *rel; - Elf64_Sym *sym; - void *location; - Elf64_Shdr *section, *symtabsec; - unsigned long address, sec_base, value; - const char *strtab, *name, *shstrtab; - - /* - * ->sh_offset has been modified to keep the pointer to section - * contents in memory - */ - rel = (void *)sechdrs[relsec].sh_offset; - - /* Section to which relocations apply */ - section = &sechdrs[sechdrs[relsec].sh_info]; - - pr_debug("Applying relocate section %u to %u\n", relsec, - sechdrs[relsec].sh_info); - - /* Associated symbol table */ - symtabsec = &sechdrs[sechdrs[relsec].sh_link]; - - /* String table */ - if (symtabsec->sh_link >= ehdr->e_shnum) { - /* Invalid strtab section number */ - pr_err("Invalid string table section index %d\n", - symtabsec->sh_link); + if (sym->st_shndx == SHN_UNDEF) { + pr_err("Undefined symbol: %s\n", name); return -ENOEXEC; } - strtab = (char *)sechdrs[symtabsec->sh_link].sh_offset; - - /* section header string table */ - shstrtab = (char *)sechdrs[ehdr->e_shstrndx].sh_offset; - - for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) { - - /* - * rel[i].r_offset contains byte offset from beginning - * of section to the storage unit affected. - * - * This is location to update (->sh_offset). This is temporary - * buffer where section is currently loaded. This will finally - * be loaded to a different address later, pointed to by - * ->sh_addr. kexec takes care of moving it - * (kexec_load_segment()). - */ - location = (void *)(section->sh_offset + rel[i].r_offset); - - /* Final address of the location */ - address = section->sh_addr + rel[i].r_offset; - - /* - * rel[i].r_info contains information about symbol table index - * w.r.t which relocation must be made and type of relocation - * to apply. ELF64_R_SYM() and ELF64_R_TYPE() macros get - * these respectively. - */ - sym = (Elf64_Sym *)symtabsec->sh_offset + - ELF64_R_SYM(rel[i].r_info); - - if (sym->st_name) - name = strtab + sym->st_name; - else - name = shstrtab + sechdrs[sym->st_shndx].sh_name; - - pr_debug("Symbol: %s info: %02x shndx: %02x value=%llx size: %llx\n", - name, sym->st_info, sym->st_shndx, sym->st_value, - sym->st_size); - - if (sym->st_shndx == SHN_UNDEF) { - pr_err("Undefined symbol: %s\n", name); - return -ENOEXEC; - } - - if (sym->st_shndx == SHN_COMMON) { - pr_err("symbol '%s' in common section\n", name); - return -ENOEXEC; - } - - if (sym->st_shndx == SHN_ABS) - sec_base = 0; - else if (sym->st_shndx >= ehdr->e_shnum) { - pr_err("Invalid section %d for symbol %s\n", - sym->st_shndx, name); - return -ENOEXEC; - } else - sec_base = sechdrs[sym->st_shndx].sh_addr; - - value = sym->st_value; - value += sec_base; - value += rel[i].r_addend; - - switch (ELF64_R_TYPE(rel[i].r_info)) { - case R_X86_64_NONE: - break; - case R_X86_64_64: - *(u64 *)location = value; - break; - case R_X86_64_32: - *(u32 *)location = value; - if (value != *(u32 *)location) - goto overflow; - break; - case R_X86_64_32S: - *(s32 *)location = value; - if ((s64)value != *(s32 *)location) - goto overflow; - break; - case R_X86_64_PC32: - value -= (u64)address; - *(u32 *)location = value; - break; - default: - pr_err("Unknown rela relocation: %llu\n", - ELF64_R_TYPE(rel[i].r_info)); - return -ENOEXEC; - } + switch (reltype) { + case R_X86_64_NONE: + break; + case R_X86_64_64: + *(u64 *)location = value; + break; + case R_X86_64_32: + *(u32 *)location = value; + if (value != *(u32 *)location) + goto overflow; + break; + case R_X86_64_32S: + *(s32 *)location = value; + if ((s64)value != *(s32 *)location) + goto overflow; + break; + case R_X86_64_PC32: + value -= (u64)address; + *(u32 *)location = value; + break; + default: + pr_err("Unknown rela relocation: %u\n", reltype); + return -ENOEXEC; } + return 0; overflow: - pr_err("Overflow in relocation type %d value 0x%lx\n", - (int)ELF64_R_TYPE(rel[i].r_info), value); + pr_err("Overflow in relocation type %u value 0x%lx\n", reltype, value); return -ENOEXEC; } #endif /* CONFIG_KEXEC_FILE */ diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 406c33dcae13..e171a083540d 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -320,8 +320,13 @@ void * __weak arch_kexec_kernel_image_load(struct kimage *image); int __weak arch_kimage_file_post_load_cleanup(struct kimage *image); int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf, unsigned long buf_len); -int __weak arch_kexec_apply_relocations_add(const Elf_Ehdr *ehdr, - Elf_Shdr *sechdrs, unsigned int relsec); +int __weak arch_kexec_apply_relocation_add(const Elf_Ehdr *ehdr, + Elf_Shdr *sechdrs, + unsigned int reltype, Elf_Sym *sym, + const char *name, + unsigned long *location, + unsigned long address, + unsigned long value); int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, unsigned int relsec); void arch_kexec_protect_crashkres(void); diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index 037c321c5618..1517f977cc25 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -61,8 +61,10 @@ int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf, /* Apply relocations of type RELA */ int __weak -arch_kexec_apply_relocations_add(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, - unsigned int relsec) +arch_kexec_apply_relocation_add(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, + unsigned int reltype, Elf_Sym *sym, + const char *name, unsigned long *location, + unsigned long address, unsigned long value) { pr_err("RELA relocation unsupported.\n"); return -ENOEXEC; @@ -793,6 +795,117 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min, return ret; } +/** + * kexec_apply_relocations_add - apply purgatory relocations + * @ehdr: Pointer to elf headers + * @sechdrs: Pointer to section headers. + * @relsec: Section index of SHT_RELA section. + */ +static int kexec_apply_relocations_add(const Elf64_Ehdr *ehdr, + Elf64_Shdr *sechdrs, unsigned int relsec) +{ + int ret; + unsigned int i; + Elf64_Rela *rel; + Elf64_Sym *sym; + void *location; + Elf64_Shdr *section, *symtabsec; + unsigned long address, sec_base, value; + const char *strtab, *name, *shstrtab; + + /* + * ->sh_offset has been modified to keep the pointer to section + * contents in memory + */ + rel = (void *)sechdrs[relsec].sh_offset; + + /* Section to which relocations apply */ + section = &sechdrs[sechdrs[relsec].sh_info]; + + pr_debug("Applying relocate section %u to %u\n", relsec, + sechdrs[relsec].sh_info); + + /* Associated symbol table */ + symtabsec = &sechdrs[sechdrs[relsec].sh_link]; + + /* String table */ + if (symtabsec->sh_link >= ehdr->e_shnum) { + /* Invalid strtab section number */ + pr_err("Invalid string table section index %d\n", + symtabsec->sh_link); + return -ENOEXEC; + } + + /* String table for the associated symbol table. */ + strtab = (char *)sechdrs[symtabsec->sh_link].sh_offset; + + /* section header string table */ + shstrtab = (char *)sechdrs[ehdr->e_shstrndx].sh_offset; + + for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) { + + /* + * rel[i].r_offset contains byte offset from beginning + * of section to the storage unit affected. + * + * This is location to update (->sh_offset). This is temporary + * buffer where section is currently loaded. This will finally + * be loaded to a different address later, pointed to by + * ->sh_addr. kexec takes care of moving it + * (kexec_load_segment()). + */ + location = (void *)(section->sh_offset + rel[i].r_offset); + + /* Final address of the location */ + address = section->sh_addr + rel[i].r_offset; + + /* + * rel[i].r_info contains information about symbol table index + * w.r.t which relocation must be made and type of relocation + * to apply. ELF64_R_SYM() and ELF64_R_TYPE() macros get + * these respectively. + */ + sym = (Elf64_Sym *)symtabsec->sh_offset + + ELF64_R_SYM(rel[i].r_info); + + if (sym->st_name) + name = strtab + sym->st_name; + else + name = shstrtab + sechdrs[sym->st_shndx].sh_name; + + pr_debug("Symbol: %s info: %02x shndx: %02x value=%llx size: %llx\n", + name, sym->st_info, sym->st_shndx, sym->st_value, + sym->st_size); + + if (sym->st_shndx == SHN_COMMON) { + pr_err("symbol '%s' in common section\n", name); + return -ENOEXEC; + } + + if (sym->st_shndx == SHN_ABS) + sec_base = 0; + else if (sym->st_shndx >= ehdr->e_shnum) { + pr_err("Invalid section %d for symbol %s\n", + sym->st_shndx, name); + return -ENOEXEC; + } else + sec_base = sechdrs[sym->st_shndx].sh_addr; + + value = sym->st_value; + value += sec_base; + value += rel[i].r_addend; + + ret = arch_kexec_apply_relocation_add(ehdr, sechdrs, + ELF64_R_TYPE(rel[i].r_info), + sym, name, location, + address, value); + if (ret) + return ret; + } + + return 0; +} + static int kexec_apply_relocations(struct kimage *image) { int i, ret; @@ -836,8 +949,7 @@ static int kexec_apply_relocations(struct kimage *image) * relocations of type SHT_RELA/SHT_REL. */ if (sechdrs[i].sh_type == SHT_RELA) - ret = arch_kexec_apply_relocations_add(pi->ehdr, - sechdrs, i); + ret = kexec_apply_relocations_add(pi->ehdr, sechdrs, i); else if (sechdrs[i].sh_type == SHT_REL) ret = arch_kexec_apply_relocations(pi->ehdr, sechdrs, i);