When upreving llvm I realised that kexec stopped working on my test platform. This patch fixes it.
To: Eric Biederman ebiederm@xmission.com Cc: Baoquan He bhe@redhat.com Cc: Philipp Rudo prudo@redhat.com Cc: kexec@lists.infradead.org Cc: linux-kernel@vger.kernel.org Cc: Ross Zwisler zwisler@google.com Cc: Steven Rostedt rostedt@goodmis.org Signed-off-by: Ricardo Ribalda ribalda@chromium.org
--- Changes in v4: - Add Cc: stable - Add linker script for x86 - Add a warning when the kernel image has overlapping sections. - Link to v3: https://lore.kernel.org/r/20230321-kexec_clang16-v3-0-5f016c8d0e87@chromium....
Changes in v3: - Fix initial value. Thanks Ross! - Link to v2: https://lore.kernel.org/r/20230321-kexec_clang16-v2-0-d10e5d517869@chromium....
Changes in v2: - Fix if condition. Thanks Steven!. - Update Philipp email. Thanks Baoquan. - Link to v1: https://lore.kernel.org/r/20230321-kexec_clang16-v1-0-a768fc2c7c4d@chromium....
--- Ricardo Ribalda (2): kexec: Support purgatories with .text.hot sections x86/purgatory: Add linker script
arch/x86/purgatory/.gitignore | 2 ++ arch/x86/purgatory/Makefile | 20 +++++++++---- arch/x86/purgatory/kexec-purgatory.S | 2 +- arch/x86/purgatory/purgatory.lds.S | 57 ++++++++++++++++++++++++++++++++++++ kernel/kexec_file.c | 13 +++++++- 5 files changed, 86 insertions(+), 8 deletions(-) --- base-commit: 17214b70a159c6547df9ae204a6275d983146f6b change-id: 20230321-kexec_clang16-4510c23d129c
Best regards,
Clang16 links the purgatory text in two sections:
[ 1] .text PROGBITS 0000000000000000 00000040 00000000000011a1 0000000000000000 AX 0 0 16 [ 2] .rela.text RELA 0000000000000000 00003498 0000000000000648 0000000000000018 I 24 1 8 ... [17] .text.hot. PROGBITS 0000000000000000 00003220 000000000000020b 0000000000000000 AX 0 0 1 [18] .rela.text.hot. RELA 0000000000000000 00004428 0000000000000078 0000000000000018 I 24 17 8
And both of them have their range [sh_addr ... sh_addr+sh_size] on the area pointed by `e_entry`.
This causes that image->start is calculated twice, once for .text and another time for .text.hot. The second calculation leaves image->start in a random location.
Because of this, the system crashes inmediatly after:
kexec_core: Starting new kernel
Cc: stable@vger.kernel.org Reviewed-by: Ross Zwisler zwisler@google.com Signed-off-by: Ricardo Ribalda ribalda@chromium.org --- kernel/kexec_file.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index f1a0e4e3fb5c..25a37d8f113a 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -901,10 +901,21 @@ static int kexec_purgatory_setup_sechdrs(struct purgatory_info *pi, }
offset = ALIGN(offset, align); + + /* + * Check if the segment contains the entry point, if so, + * calculate the value of image->start based on it. + * If the compiler has produced more than one .text sections + * (Eg: .text.hot), they are generally after the main .text + * section, and they shall not be used to calculate + * image->start. So do not re-calculate image->start if it + * is not set to the initial value. + */ if (sechdrs[i].sh_flags & SHF_EXECINSTR && pi->ehdr->e_entry >= sechdrs[i].sh_addr && pi->ehdr->e_entry < (sechdrs[i].sh_addr - + sechdrs[i].sh_size)) { + + sechdrs[i].sh_size) && + kbuf->image->start == pi->ehdr->e_entry) { kbuf->image->start -= sechdrs[i].sh_addr; kbuf->image->start += kbuf->mem + offset; }
On Mon, Mar 27, 2023 at 05:06:53PM +0200, Ricardo Ribalda wrote:
Clang16 links the purgatory text in two sections:
[ 1] .text PROGBITS 0000000000000000 00000040 00000000000011a1 0000000000000000 AX 0 0 16 [ 2] .rela.text RELA 0000000000000000 00003498 0000000000000648 0000000000000018 I 24 1 8 ... [17] .text.hot. PROGBITS 0000000000000000 00003220 000000000000020b 0000000000000000 AX 0 0 1 [18] .rela.text.hot. RELA 0000000000000000 00004428 0000000000000078 0000000000000018 I 24 17 8
And both of them have their range [sh_addr ... sh_addr+sh_size] on the area pointed by `e_entry`.
This causes that image->start is calculated twice, once for .text and another time for .text.hot. The second calculation leaves image->start in a random location.
Because of this, the system crashes inmediatly after:
s/inmediatly/immediately/
kexec_core: Starting new kernel
Cc: stable@vger.kernel.org
Maybe a fixes tag is warranted here.
Reviewed-by: Ross Zwisler zwisler@google.com Signed-off-by: Ricardo Ribalda ribalda@chromium.org
kernel/kexec_file.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index f1a0e4e3fb5c..25a37d8f113a 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -901,10 +901,21 @@ static int kexec_purgatory_setup_sechdrs(struct purgatory_info *pi, } offset = ALIGN(offset, align);
/** Check if the segment contains the entry point, if so,* calculate the value of image->start based on it.* If the compiler has produced more than one .text sections
nit: s/sections/section/
* (Eg: .text.hot), they are generally after the main .text
If this is the general case, then are there cases where this doesn't hold?
* section, and they shall not be used to calculate* image->start. So do not re-calculate image->start if it* is not set to the initial value.*/
+ sechdrs[i].sh_size)) {
+ sechdrs[i].sh_size) &&kbuf->image->start == pi->ehdr->e_entry) { kbuf->image->start -= sechdrs[i].sh_addr; kbuf->image->start += kbuf->mem + offset;-- 2.40.0.348.gf938b09366-goog-b4-0.11.0-dev-696ae
kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Hi Simon
Thanks for your review!
On Thu, 30 Mar 2023 at 09:49, Simon Horman horms@kernel.org wrote:
On Mon, Mar 27, 2023 at 05:06:53PM +0200, Ricardo Ribalda wrote:
Clang16 links the purgatory text in two sections:
[ 1] .text PROGBITS 0000000000000000 00000040 00000000000011a1 0000000000000000 AX 0 0 16 [ 2] .rela.text RELA 0000000000000000 00003498 0000000000000648 0000000000000018 I 24 1 8 ... [17] .text.hot. PROGBITS 0000000000000000 00003220 000000000000020b 0000000000000000 AX 0 0 1 [18] .rela.text.hot. RELA 0000000000000000 00004428 0000000000000078 0000000000000018 I 24 17 8
And both of them have their range [sh_addr ... sh_addr+sh_size] on the area pointed by `e_entry`.
This causes that image->start is calculated twice, once for .text and another time for .text.hot. The second calculation leaves image->start in a random location.
Because of this, the system crashes inmediatly after:
s/inmediatly/immediately/
kexec_core: Starting new kernel
Cc: stable@vger.kernel.org
Maybe a fixes tag is warranted here.
Reviewed-by: Ross Zwisler zwisler@google.com Signed-off-by: Ricardo Ribalda ribalda@chromium.org
kernel/kexec_file.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index f1a0e4e3fb5c..25a37d8f113a 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -901,10 +901,21 @@ static int kexec_purgatory_setup_sechdrs(struct purgatory_info *pi, }
offset = ALIGN(offset, align);
/** Check if the segment contains the entry point, if so,* calculate the value of image->start based on it.* If the compiler has produced more than one .text sectionsnit: s/sections/section/
* (Eg: .text.hot), they are generally after the main .textIf this is the general case, then are there cases where this doesn't hold?
When looking at this issue, I have only seen .text.hot after .text. But I cannot warantee that future versions of llvm or gcc decide to swap the order.
I am going to add a WARN whenever there are two overlapping .text sections so the user has the chance to update their linker script.
* section, and they shall not be used to calculate* image->start. So do not re-calculate image->start if it* is not set to the initial value.*/ if (sechdrs[i].sh_flags & SHF_EXECINSTR && pi->ehdr->e_entry >= sechdrs[i].sh_addr && pi->ehdr->e_entry < (sechdrs[i].sh_addr
+ sechdrs[i].sh_size)) {
+ sechdrs[i].sh_size) &&kbuf->image->start == pi->ehdr->e_entry) { kbuf->image->start -= sechdrs[i].sh_addr; kbuf->image->start += kbuf->mem + offset; }-- 2.40.0.348.gf938b09366-goog-b4-0.11.0-dev-696ae
kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
-- Ricardo Ribalda
Make sure that the .text section is not divided in multiple overlapping sections. This is not supported by kexec_file.
Signed-off-by: Ricardo Ribalda ribalda@chromium.org --- arch/x86/purgatory/.gitignore | 2 ++ arch/x86/purgatory/Makefile | 20 +++++++++---- arch/x86/purgatory/kexec-purgatory.S | 2 +- arch/x86/purgatory/purgatory.lds.S | 57 ++++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/arch/x86/purgatory/.gitignore b/arch/x86/purgatory/.gitignore index d2be1500671d..1fe71fe5945d 100644 --- a/arch/x86/purgatory/.gitignore +++ b/arch/x86/purgatory/.gitignore @@ -1 +1,3 @@ purgatory.chk +purgatory.lds +purgatory diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile index 17f09dc26381..4dc96d409bec 100644 --- a/arch/x86/purgatory/Makefile +++ b/arch/x86/purgatory/Makefile @@ -16,10 +16,11 @@ CFLAGS_sha256.o := -D__DISABLE_EXPORTS
# When linking purgatory.ro with -r unresolved symbols are not checked, # also link a purgatory.chk binary without -r to check for unresolved symbols. -PURGATORY_LDFLAGS := -e purgatory_start -z nodefaultlib -LDFLAGS_purgatory.ro := -r $(PURGATORY_LDFLAGS) -LDFLAGS_purgatory.chk := $(PURGATORY_LDFLAGS) -targets += purgatory.ro purgatory.chk +PURGATORY_LDFLAGS := -nostdlib -z nodefaultlib +LDFLAGS_purgatory := -r $(PURGATORY_LDFLAGS) -T +LDFLAGS_purgatory.chk := -e purgatory_start $(PURGATORY_LDFLAGS) + +targets += purgatory.lds purgatory.ro purgatory.chk
# Sanitizer, etc. runtimes are unavailable and cannot be linked here. GCOV_PROFILE := n @@ -72,10 +73,17 @@ CFLAGS_string.o += $(PURGATORY_CFLAGS) AFLAGS_REMOVE_setup-x86_$(BITS).o += -Wa,-gdwarf-2 AFLAGS_REMOVE_entry64.o += -Wa,-gdwarf-2
-$(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE +OBJCOPYFLAGS_purgatory.ro := -O elf64-x86-64 +OBJCOPYFLAGS_purgatory.ro += --remove-section='*debug*' +OBJCOPYFLAGS_purgatory.ro += --remove-section='.comment' +OBJCOPYFLAGS_purgatory.ro += --remove-section='.note.*' +$(obj)/purgatory.ro: $(obj)/purgatory FORCE + $(call if_changed,objcopy) + +$(obj)/purgatory.chk: $(obj)/purgatory FORCE $(call if_changed,ld)
-$(obj)/purgatory.chk: $(obj)/purgatory.ro FORCE +$(obj)/purgatory: $(obj)/purgatory.lds $(PURGATORY_OBJS) FORCE $(call if_changed,ld)
$(obj)/kexec-purgatory.o: $(obj)/purgatory.ro $(obj)/purgatory.chk diff --git a/arch/x86/purgatory/kexec-purgatory.S b/arch/x86/purgatory/kexec-purgatory.S index 8530fe93b718..54b0d0b4dc42 100644 --- a/arch/x86/purgatory/kexec-purgatory.S +++ b/arch/x86/purgatory/kexec-purgatory.S @@ -5,7 +5,7 @@ .align 8 kexec_purgatory: .globl kexec_purgatory - .incbin "arch/x86/purgatory/purgatory.ro" + .incbin "arch/x86/purgatory/purgatory" .Lkexec_purgatory_end:
.align 8 diff --git a/arch/x86/purgatory/purgatory.lds.S b/arch/x86/purgatory/purgatory.lds.S new file mode 100644 index 000000000000..610da88aafa0 --- /dev/null +++ b/arch/x86/purgatory/purgatory.lds.S @@ -0,0 +1,57 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#include <asm-generic/vmlinux.lds.h> + +OUTPUT_FORMAT(CONFIG_OUTPUT_FORMAT) + +#undef i386 + +#include <asm/cache.h> +#include <asm/page_types.h> + +ENTRY(purgatory_start) + +SECTIONS +{ + . = 0; + .head.text : { + _head = . ; + HEAD_TEXT + _ehead = . ; + } + .rodata : { + _rodata = . ; + *(.rodata) /* read-only data */ + *(.rodata.*) + _erodata = . ; + } + .text : { + _text = .; /* Text */ + *(.text) + *(.text.*) + *(.noinstr.text) + _etext = . ; + } + .data : { + _data = . ; + *(.data) + *(.data.*) + *(.bss.efistub) + _edata = . ; + } + . = ALIGN(L1_CACHE_BYTES); + .bss : { + _bss = . ; + *(.bss) + *(.bss.*) + *(COMMON) + . = ALIGN(8); /* For convenience during zeroing */ + _ebss = .; + } + + /* Sections to be discarded */ + /DISCARD/ : { + *(.eh_frame) + *(*__ksymtab*) + *(___kcrctab*) + } +}
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
Rule: 'Cc: stable@vger.kernel.org' or 'commit <sha1> upstream.' Subject: [PATCH v4 2/2] x86/purgatory: Add linker script Link: https://lore.kernel.org/stable/20230321-kexec_clang16-v4-2-1340518f98e9%40ch...
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
linux-stable-mirror@lists.linaro.org
-
kernel test robot -
Ricardo Ribalda -
Simon Horman