Re: [PATCH v4 1/7] kexec: add KEXEC_ELF
Thiago Jung Bauermann writes:
>> diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/kernel/kexec_elf.c
>> similarity index 71%
>> copy from arch/powerpc/kernel/kexec_elf_64.c
>> copy to kernel/kexec_elf.c
>> index ba4f18a43ee8..6e9f52171ede 100644
>> --- a/arch/powerpc/kernel/kexec_elf_64.c
>> +++ b/kernel/kexec_elf.c
>> @@ -1,33 +1,10 @@
>> -/*
>> - * Load ELF vmlinux file for the kexec_file_load syscall.
>> - *
>> - * Copyright (C) 2004 Adam Litke (a...@us.ibm.com)
>> - * Copyright (C) 2004 IBM Corp.
>> - * Copyright (C) 2005 R Sharada (shar...@in.ibm.com)
>> - * Copyright (C) 2006 Mohan Kumar M (mo...@in.ibm.com)
>> - * Copyright (C) 2016 IBM Corporation
>> - *
>> - * 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.
>> - */
>> +// SPDX-License-Identifier: GPL-2.0-only
>
> I may be wrong, but my understanding of the SPDX license identifier is
> that it substitutes the license text (i.e., the last two paragraphs
> above), but not the copyright statements. Is it ok to have a file with a
> SPDX license identifier but no copyright statement?
Answering my own question: I just came accross commit b24413180f56
("License cleanup: add SPDX GPL-2.0 license identifier to files with no
license") which adds SPDX license identifiers to a lot of files without
any copyright statement so I conclude that it is indeed ok to not have
copyright statements in a file.
In this instance the new file is heavily based on the old one though, so
IMHO it makes sense for it to inherit the copyright statements from the
original file.
--
Thiago Jung Bauermann
IBM Linux Technology Center
Re: [PATCH v4 1/7] kexec: add KEXEC_ELF
Hello Sven, Just a few small comments below. Regardless of them: Reviewed-by: Thiago Jung Bauermann Sven Schnelle writes: > Right now powerpc provides an implementation to read elf files > with the kexec_file() syscall. Make that available as a public Nit: the syscall is kexec_file_load() > 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/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_Rel Elf64_Rel > #endif /* Elf_Rel */ Perhaps this patch could remove the #define for elf_addr_to_cpu since it's not used anymore in this file? > diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/kernel/kexec_elf.c > similarity index 71% > copy from arch/powerpc/kernel/kexec_elf_64.c > copy to kernel/kexec_elf.c > index ba4f18a43ee8..6e9f52171ede 100644 > --- a/arch/powerpc/kernel/kexec_elf_64.c > +++ b/kernel/kexec_elf.c > @@ -1,33 +1,10 @@ > -/* > - * Load ELF vmlinux file for the kexec_file_load syscall. > - * > - * Copyright (C) 2004 Adam Litke (a...@us.ibm.com) > - * Copyright (C) 2004 IBM Corp. > - * Copyright (C) 2005 R Sharada (shar...@in.ibm.com) > - * Copyright (C) 2006 Mohan Kumar M (mo...@in.ibm.com) > - * Copyright (C) 2016 IBM Corporation > - * > - * 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. > - */ > +// SPDX-License-Identifier: GPL-2.0-only I may be wrong, but my understanding of the SPDX license identifier is that it substitutes the license text (i.e., the last two paragraphs above), but not the copyright statements. Is it ok to have a file with a SPDX license identifier but no copyright statement? -- Thiago Jung Bauermann IBM Linux Technology Center
[PATCH v4 1/7] kexec: add KEXEC_ELF
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_phnum;
-
- /* Sanity check the program header table location. */
- if (ehdr->e_phoff + phdr_size <
