[PATCH v3 0/2] s390/kexec_file: improve error handling and messages

[PATCH v3 0/2] s390/kexec_file: improve error handling and messages

[Philipp Rudo]

Hi everybody,

here is v3.

@Heiko: I hope you are happy now.

Thanks
Philipp

v3:
	- add the symbol name to the error messages

v2:
	- EINVAL -> ENOEXEC
	- print error message when encountering an unknown relocation
	- new patch to print error messages for all error cases in the function

Philipp Rudo (2):
  s390/kexec_file: print some more error messages
  s390/kexec_file: fix error handling when applying relocations

 arch/s390/kernel/machine_kexec_file.c | 34 +++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

-- 
2.31.1

[PATCH v3 1/2] s390/kexec_file: print some more error messages

[Philipp Rudo]

Be kind and give some more information on what went wrong.

Signed-off-by: Philipp Rudo <prudo@redhat.com>
---
 arch/s390/kernel/machine_kexec_file.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 9975ad200d74..a8bfa7c8cbba 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -7,6 +7,8 @@
  * Author(s): Philipp Rudo <prudo@linux.vnet.ibm.com>
  */
 
+#define pr_fmt(fmt)	"kexec: " fmt
+
 #include <linux/elf.h>
 #include <linux/errno.h>
 #include <linux/kexec.h>
@@ -290,9 +292,16 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
 				     const Elf_Shdr *relsec,
 				     const Elf_Shdr *symtab)
 {
+	const char *strtab, *name, *shstrtab;
+	const Elf_Shdr *sechdrs;
 	Elf_Rela *relas;
 	int i, r_type;
 
+	/* String & section header string table */
+	sechdrs = (void *)pi->ehdr + pi->ehdr->e_shoff;
+	strtab = (char *)pi->ehdr + sechdrs[symtab->sh_link].sh_offset;
+	shstrtab = (char *)pi->ehdr + sechdrs[pi->ehdr->e_shstrndx].sh_offset;
+
 	relas = (void *)pi->ehdr + relsec->sh_offset;
 
 	for (i = 0; i < relsec->sh_size / sizeof(*relas); i++) {
@@ -304,15 +313,27 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
 		sym = (void *)pi->ehdr + symtab->sh_offset;
 		sym += ELF64_R_SYM(relas[i].r_info);
 
-		if (sym->st_shndx == SHN_UNDEF)
+		if (sym->st_name)
+			name = strtab + sym->st_name;
+		else
+			name = shstrtab + sechdrs[sym->st_shndx].sh_name;
+
+		if (sym->st_shndx == SHN_UNDEF) {
+			pr_err("Undefined symbol: %s\n", name);
 			return -ENOEXEC;
+		}
 
-		if (sym->st_shndx == SHN_COMMON)
+		if (sym->st_shndx == SHN_COMMON) {
+			pr_err("symbol '%s' in common section\n", name);
 			return -ENOEXEC;
+		}
 
 		if (sym->st_shndx >= pi->ehdr->e_shnum &&
-		    sym->st_shndx != SHN_ABS)
+		    sym->st_shndx != SHN_ABS) {
+			pr_err("Invalid section %d for symbol %s\n",
+			       sym->st_shndx, name);
 			return -ENOEXEC;
+		}
 
 		loc = pi->purgatory_buf;
 		loc += section->sh_offset;
-- 
2.31.1

[PATCH v3 2/2] s390/kexec_file: fix error handling when applying relocations

[Philipp Rudo]

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@redhat.com>
Signed-off-by: Philipp Rudo <prudo@redhat.com>
---
 arch/s390/kernel/machine_kexec_file.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index a8bfa7c8cbba..876cdd3c994e 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -296,6 +296,7 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
 	const Elf_Shdr *sechdrs;
 	Elf_Rela *relas;
 	int i, r_type;
+	int ret;
 
 	/* String & section header string table */
 	sechdrs = (void *)pi->ehdr + pi->ehdr->e_shoff;
@@ -347,7 +348,11 @@ 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) {
+			pr_err("Unknown rela relocation: %d\n", r_type);
+			return -ENOEXEC;
+		}
 	}
 	return 0;
 }
-- 
2.31.1

Re: [PATCH v3 0/2] s390/kexec_file: improve error handling and messages

[Heiko Carstens]

On Wed, Dec 08, 2021 at 02:07:39PM +0100, Philipp Rudo wrote:
> Hi everybody,
> 
> here is v3.
> 
> @Heiko: I hope you are happy now.

Yes, very happy!

> v3:
> 	- add the symbol name to the error messages
> 
> v2:
> 	- EINVAL -> ENOEXEC
> 	- print error message when encountering an unknown relocation
> 	- new patch to print error messages for all error cases in the function
> 
> Philipp Rudo (2):
>   s390/kexec_file: print some more error messages
>   s390/kexec_file: fix error handling when applying relocations
> 
>  arch/s390/kernel/machine_kexec_file.c | 34 +++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)

Applied both. Thank you!