From: Arnd Bergmann <[email protected]>
The cleanup for the CONFIG_KEXEC Kconfig logic accidentally changed the
'depends on CRYPTO=y' dependency to a plain 'depends on CRYPTO', which
causes a link failure when all the crypto support is in a loadable module
and kexec_file support is built-in:
x86_64-linux-ld: vmlinux.o: in function `__x64_sys_kexec_file_load':
(.text+0x32e30a): undefined reference to `crypto_alloc_shash'
x86_64-linux-ld: (.text+0x32e58e): undefined reference to `crypto_shash_update'
x86_64-linux-ld: (.text+0x32e6ee): undefined reference to `crypto_shash_final'
Both s390 and x86 have this problem, while ppc64 and riscv have the
correct dependency already. On riscv, the dependency is only used
for the purgatory, not for the kexec_file code itself, which may
be a bit surprising as it means that with CONFIG_CRYPTO=m, it is
possible to enable KEXEC_FILE but then the purgatory code is silently
left out.
Move this into the common Kconfig.kexec file in a way that is
correct everywhere, using the dependency on CRYPTO_SHA256=y only
when the purgatory code is available. This requires reversing the
dependency between ARCH_SUPPORTS_KEXEC_PURGATORY and KEXEC_FILE,
but the effect remains the same, other than making riscv behave
like the other ones.
On s390, there is an additional dependency on CRYPTO_SHA256_S390, which
should technically not be required but gives better performance. Remove
this dependency here, noting that it was not present in the initial
Kconfig code but was brought in without an explanation in commit
71406883fd357 ("s390/kexec_file: Add kexec_file_load system call").
Fixes: 6af5138083005 ("x86/kexec: refactor for kernel/Kconfig.kexec")
Signed-off-by: Arnd Bergmann <[email protected]>
---
arch/powerpc/Kconfig | 4 ++--
arch/riscv/Kconfig | 4 +---
arch/s390/Kconfig | 4 ++--
arch/x86/Kconfig | 4 ++--
kernel/Kconfig.kexec | 1 +
5 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d5d5388973ac7..4640cee33f123 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
config ARCH_SUPPORTS_KEXEC_FILE
- def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
+ def_bool PPC64
config ARCH_SUPPORTS_KEXEC_PURGATORY
- def_bool KEXEC_FILE
+ def_bool y
config ARCH_SELECTS_KEXEC_FILE
def_bool y
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 25474f8c12b79..f571bad2d22d0 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
select KEXEC_ELF
config ARCH_SUPPORTS_KEXEC_PURGATORY
- def_bool KEXEC_FILE
- depends on CRYPTO=y
- depends on CRYPTO_SHA256=y
+ def_bool y
config ARCH_SUPPORTS_CRASH_DUMP
def_bool y
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b0d67ac8695f9..ec77106af4137 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
def_bool y
config ARCH_SUPPORTS_KEXEC_FILE
- def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+ def_bool y
config ARCH_SUPPORTS_KEXEC_SIG
def_bool MODULE_SIG_FORMAT
config ARCH_SUPPORTS_KEXEC_PURGATORY
- def_bool KEXEC_FILE
+ def_bool y
config ARCH_SUPPORTS_CRASH_DUMP
def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 94efde80ebf35..f9975b15ccd57 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
def_bool y
config ARCH_SUPPORTS_KEXEC_FILE
- def_bool X86_64 && CRYPTO && CRYPTO_SHA256
+ def_bool X86_64
config ARCH_SELECTS_KEXEC_FILE
def_bool y
@@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
select HAVE_IMA_KEXEC if IMA
config ARCH_SUPPORTS_KEXEC_PURGATORY
- def_bool KEXEC_FILE
+ def_bool y
config ARCH_SUPPORTS_KEXEC_SIG
def_bool y
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f48..bfc636d64ff2b 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,6 +36,7 @@ config KEXEC
config KEXEC_FILE
bool "Enable kexec file based system call"
depends on ARCH_SUPPORTS_KEXEC_FILE
+ depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
select KEXEC_CORE
help
This is new version of kexec system call. This system call is
--
2.39.2
From: Arnd Bergmann <[email protected]>
All other users of crypto code use 'select' instead of 'depends on',
so do the same thing with KEXEC_FILE for consistency.
In practice this makes very little difference as kernels with kexec
support are very likely to also include some other feature that already
selects both crypto and crypto_sha256, but being consistent here helps
for usability as well as to avoid potential circular dependencies.
This reverts the dependency back to what it was originally before commit
74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
new syscall"), which changed changed it with the comment "This should
be safer as "select" is not recursive", but that appears to have been
done in error, as "select" is indeed recursive, and there are no other
dependencies that prevent CRYPTO_SHA256 from being selected here.
Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
Cc: Herbert Xu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
Signed-off-by: Arnd Bergmann <[email protected]>
---
kernel/Kconfig.kexec | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index bfc636d64ff2b..51f719af10e79 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,7 +36,8 @@ config KEXEC
config KEXEC_FILE
bool "Enable kexec file based system call"
depends on ARCH_SUPPORTS_KEXEC_FILE
- depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
+ select CRYPTO
+ select CRYPTO_SHA256
select KEXEC_CORE
help
This is new version of kexec system call. This system call is
--
2.39.2
On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The cleanup for the CONFIG_KEXEC Kconfig logic accidentally changed the
> 'depends on CRYPTO=y' dependency to a plain 'depends on CRYPTO', which
> causes a link failure when all the crypto support is in a loadable module
> and kexec_file support is built-in:
>
> x86_64-linux-ld: vmlinux.o: in function `__x64_sys_kexec_file_load':
> (.text+0x32e30a): undefined reference to `crypto_alloc_shash'
> x86_64-linux-ld: (.text+0x32e58e): undefined reference to `crypto_shash_update'
> x86_64-linux-ld: (.text+0x32e6ee): undefined reference to `crypto_shash_final'
>
> Both s390 and x86 have this problem, while ppc64 and riscv have the
> correct dependency already. On riscv, the dependency is only used
> for the purgatory, not for the kexec_file code itself, which may
> be a bit surprising as it means that with CONFIG_CRYPTO=m, it is
> possible to enable KEXEC_FILE but then the purgatory code is silently
> left out.
>
> Move this into the common Kconfig.kexec file in a way that is
> correct everywhere, using the dependency on CRYPTO_SHA256=y only
> when the purgatory code is available. This requires reversing the
> dependency between ARCH_SUPPORTS_KEXEC_PURGATORY and KEXEC_FILE,
> but the effect remains the same, other than making riscv behave
> like the other ones.
>
> On s390, there is an additional dependency on CRYPTO_SHA256_S390, which
> should technically not be required but gives better performance. Remove
> this dependency here, noting that it was not present in the initial
> Kconfig code but was brought in without an explanation in commit
> 71406883fd357 ("s390/kexec_file: Add kexec_file_load system call").
>
> Fixes: 6af5138083005 ("x86/kexec: refactor for kernel/Kconfig.kexec")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> arch/powerpc/Kconfig | 4 ++--
> arch/riscv/Kconfig | 4 +---
This doesn't appear to work for rv32. The defconfig build in our
patchwork automation complains:
/tmp/tmp.Aq21JVRQTx/arch/riscv/purgatory/entry.S:29:2: error: instruction requires the following: RV64I Base Instruction Set
> arch/s390/Kconfig | 4 ++--
> arch/x86/Kconfig | 4 ++--
> kernel/Kconfig.kexec | 1 +
> 5 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d5d5388973ac7..4640cee33f123 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
> def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>
> config ARCH_SUPPORTS_KEXEC_FILE
> - def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
> + def_bool PPC64
>
> config ARCH_SUPPORTS_KEXEC_PURGATORY
> - def_bool KEXEC_FILE
> + def_bool y
>
> config ARCH_SELECTS_KEXEC_FILE
> def_bool y
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 25474f8c12b79..f571bad2d22d0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
> select KEXEC_ELF
>
> config ARCH_SUPPORTS_KEXEC_PURGATORY
> - def_bool KEXEC_FILE
> - depends on CRYPTO=y
> - depends on CRYPTO_SHA256=y
> + def_bool y
This being the problem, KEXEC_FILE is 64-bit only.
IIRC I commented on this same thing during the original conversion
patches.
Cheers,
Conor.
>
> config ARCH_SUPPORTS_CRASH_DUMP
> def_bool y
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0d67ac8695f9..ec77106af4137 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
> def_bool y
>
> config ARCH_SUPPORTS_KEXEC_FILE
> - def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> + def_bool y
>
> config ARCH_SUPPORTS_KEXEC_SIG
> def_bool MODULE_SIG_FORMAT
>
> config ARCH_SUPPORTS_KEXEC_PURGATORY
> - def_bool KEXEC_FILE
> + def_bool y
>
> config ARCH_SUPPORTS_CRASH_DUMP
> def_bool y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94efde80ebf35..f9975b15ccd57 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
> def_bool y
>
> config ARCH_SUPPORTS_KEXEC_FILE
> - def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> + def_bool X86_64
>
> config ARCH_SELECTS_KEXEC_FILE
> def_bool y
> @@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
> select HAVE_IMA_KEXEC if IMA
>
> config ARCH_SUPPORTS_KEXEC_PURGATORY
> - def_bool KEXEC_FILE
> + def_bool y
>
> config ARCH_SUPPORTS_KEXEC_SIG
> def_bool y
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f48..bfc636d64ff2b 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,6 +36,7 @@ config KEXEC
> config KEXEC_FILE
> bool "Enable kexec file based system call"
> depends on ARCH_SUPPORTS_KEXEC_FILE
> + depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> select KEXEC_CORE
> help
> This is new version of kexec system call. This system call is
> --
> 2.39.2
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Mon, Oct 23, 2023, at 17:37, Conor Dooley wrote:
> On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:
>> index 25474f8c12b79..f571bad2d22d0 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>> select KEXEC_ELF
>>
>> config ARCH_SUPPORTS_KEXEC_PURGATORY
>> - def_bool KEXEC_FILE
>> - depends on CRYPTO=y
>> - depends on CRYPTO_SHA256=y
>> + def_bool y
>
> This being the problem, KEXEC_FILE is 64-bit only.
>
> IIRC I commented on this same thing during the original conversion
> patches.
Does it work with this patch on top?
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -687,7 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
select KEXEC_ELF
config ARCH_SUPPORTS_KEXEC_PURGATORY
- def_bool y
+ def_bool ARCH_SUPPORTS_KEXEC_FILE
config ARCH_SUPPORTS_CRASH_DUMP
def_bool y
Arnd
On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> All other users of crypto code use 'select' instead of 'depends on',
> so do the same thing with KEXEC_FILE for consistency.
>
> In practice this makes very little difference as kernels with kexec
> support are very likely to also include some other feature that already
> selects both crypto and crypto_sha256, but being consistent here helps
> for usability as well as to avoid potential circular dependencies.
>
> This reverts the dependency back to what it was originally before commit
> 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
> new syscall"), which changed changed it with the comment "This should
~~~~~~~~~~~~~~ typo
> be safer as "select" is not recursive", but that appears to have been
> done in error, as "select" is indeed recursive, and there are no other
> dependencies that prevent CRYPTO_SHA256 from being selected here.
>
> Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
> Cc: Herbert Xu <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: [email protected]
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> kernel/Kconfig.kexec | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
LGTM,
Acked-by: Baoquan He <[email protected]>
>
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index bfc636d64ff2b..51f719af10e79 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,7 +36,8 @@ config KEXEC
> config KEXEC_FILE
> bool "Enable kexec file based system call"
> depends on ARCH_SUPPORTS_KEXEC_FILE
> - depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> + select CRYPTO
> + select CRYPTO_SHA256
> select KEXEC_CORE
> help
> This is new version of kexec system call. This system call is
> --
> 2.39.2
>
On 10/24/23 at 11:55am, Baoquan He wrote:
> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > All other users of crypto code use 'select' instead of 'depends on',
> > so do the same thing with KEXEC_FILE for consistency.
> >
> > In practice this makes very little difference as kernels with kexec
> > support are very likely to also include some other feature that already
> > selects both crypto and crypto_sha256, but being consistent here helps
> > for usability as well as to avoid potential circular dependencies.
> >
> > This reverts the dependency back to what it was originally before commit
> > 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
> > new syscall"), which changed changed it with the comment "This should
> ~~~~~~~~~~~~~~ typo
> > be safer as "select" is not recursive", but that appears to have been
> > done in error, as "select" is indeed recursive, and there are no other
> > dependencies that prevent CRYPTO_SHA256 from being selected here.
> >
> > Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
> > Cc: Herbert Xu <[email protected]>
> > Cc: "David S. Miller" <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Arnd Bergmann <[email protected]>
> > ---
> > kernel/Kconfig.kexec | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
>
> LGTM,
>
> Acked-by: Baoquan He <[email protected]>
Sorry, the patch 1/2 is not sent to me and kexec mailing list, so I
didn't get the intention of the entire patchset. I need hold the ack
until I read the patch 1. I have some concerns about patch 1 if I didn't
misunderstand it. Will come back later when patch 1 reviewing is
finished.
>
> >
> > diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> > index bfc636d64ff2b..51f719af10e79 100644
> > --- a/kernel/Kconfig.kexec
> > +++ b/kernel/Kconfig.kexec
> > @@ -36,7 +36,8 @@ config KEXEC
> > config KEXEC_FILE
> > bool "Enable kexec file based system call"
> > depends on ARCH_SUPPORTS_KEXEC_FILE
> > - depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> > + select CRYPTO
> > + select CRYPTO_SHA256
> > select KEXEC_CORE
> > help
> > This is new version of kexec system call. This system call is
> > --
> > 2.39.2
> >
>
On Tue, Oct 24, 2023, at 06:10, Baoquan He wrote:
> On 10/24/23 at 11:55am, Baoquan He wrote:
>> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
>> > From: Arnd Bergmann <[email protected]>
>> >
>> > All other users of crypto code use 'select' instead of 'depends on',
>> > so do the same thing with KEXEC_FILE for consistency.
>> >
>> > In practice this makes very little difference as kernels with kexec
>> > support are very likely to also include some other feature that already
>> > selects both crypto and crypto_sha256, but being consistent here helps
>> > for usability as well as to avoid potential circular dependencies.
>> >
>> > This reverts the dependency back to what it was originally before commit
>> > 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
>> > new syscall"), which changed changed it with the comment "This should
>> ~~~~~~~~~~~~~~ typo
>> > be safer as "select" is not recursive", but that appears to have been
>> > done in error, as "select" is indeed recursive, and there are no other
>> > dependencies that prevent CRYPTO_SHA256 from being selected here.
>> >
>> > Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
>> > Cc: Herbert Xu <[email protected]>
>> > Cc: "David S. Miller" <[email protected]>
>> > Cc: [email protected]
>> > Signed-off-by: Arnd Bergmann <[email protected]>
>> > ---
>> > kernel/Kconfig.kexec | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> LGTM,
>>
>> Acked-by: Baoquan He <[email protected]>
>
> Sorry, the patch 1/2 is not sent to me and kexec mailing list, so I
> didn't get the intention of the entire patchset. I need hold the ack
> until I read the patch 1. I have some concerns about patch 1 if I didn't
> misunderstand it. Will come back later when patch 1 reviewing is
> finished.
Sorry about missing you on Cc. If anyone else is looking for the
patch, it's archived at
https://lore.kernel.org/lkml/[email protected]/
The idea of patch 1 was only to address the build regression on
x86, so I was hoping that part would be uncontroversial. I split
out patch 2/2 since that is intended to actually change the behavior,
hopefully for the better.
I introduced a new regression on riscv that Conor Dooley found, and
that should already be fixed now. It looks like we may need a similar
change on s390
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS) += hypfs/
obj-$(CONFIG_APPLDATA_BASE) += appldata/
obj-y += net/
obj-$(CONFIG_PCI) += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/
# for cleaning
subdir- += boot tools
but I haven't tested that, and I'll wait for your reply then.
Arnd
Just add people and mailing list to CC since I didn't find this mail in
my box, just drag it via 'b4 am'.
On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
......
> ---
> arch/powerpc/Kconfig | 4 ++--
> arch/riscv/Kconfig | 4 +---
> arch/s390/Kconfig | 4 ++--
> arch/x86/Kconfig | 4 ++--
> kernel/Kconfig.kexec | 1 +
> 5 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d5d5388973ac7..4640cee33f123 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
> def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>
> config ARCH_SUPPORTS_KEXEC_FILE
> - def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
> + def_bool PPC64
>
> config ARCH_SUPPORTS_KEXEC_PURGATORY
> - def_bool KEXEC_FILE
> + def_bool y
>
> config ARCH_SELECTS_KEXEC_FILE
> def_bool y
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 25474f8c12b79..f571bad2d22d0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
> select KEXEC_ELF
>
> config ARCH_SUPPORTS_KEXEC_PURGATORY
> - def_bool KEXEC_FILE
> - depends on CRYPTO=y
> - depends on CRYPTO_SHA256=y
> + def_bool y
>
> config ARCH_SUPPORTS_CRASH_DUMP
> def_bool y
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0d67ac8695f9..ec77106af4137 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
> def_bool y
>
> config ARCH_SUPPORTS_KEXEC_FILE
> - def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> + def_bool y
>
> config ARCH_SUPPORTS_KEXEC_SIG
> def_bool MODULE_SIG_FORMAT
>
> config ARCH_SUPPORTS_KEXEC_PURGATORY
> - def_bool KEXEC_FILE
> + def_bool y
>
> config ARCH_SUPPORTS_CRASH_DUMP
> def_bool y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94efde80ebf35..f9975b15ccd57 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
> def_bool y
>
> config ARCH_SUPPORTS_KEXEC_FILE
> - def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> + def_bool X86_64
>
> config ARCH_SELECTS_KEXEC_FILE
> def_bool y
> @@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
> select HAVE_IMA_KEXEC if IMA
>
> config ARCH_SUPPORTS_KEXEC_PURGATORY
> - def_bool KEXEC_FILE
> + def_bool y
>
> config ARCH_SUPPORTS_KEXEC_SIG
> def_bool y
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f48..bfc636d64ff2b 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,6 +36,7 @@ config KEXEC
> config KEXEC_FILE
> bool "Enable kexec file based system call"
> depends on ARCH_SUPPORTS_KEXEC_FILE
> + depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
I am not sure if the logic is correct. In theory, kexec_file code
utilizes purgatory to verify the checksum digested during kernel loading
when try to jump to the kernel. That means kexec_file depends on
purgatory, but not contrary?
With these changes, we can achieve the goal to avoid building issue,
whereas the code logic becomes confusing. E.g people could disable
CONFIG_KEXEC_FILE, but still get purgatory code built in which is
totally useless.
Not sure if I think too much over this.
> select KEXEC_CORE
> help
> This is new version of kexec system call. This system call is
> --
> 2.39.2
On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> Just add people and mailing list to CC since I didn't find this mail in
> my box, just drag it via 'b4 am'.
>
> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> ......
>> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
>> index 7aff28ded2f48..bfc636d64ff2b 100644
>> --- a/kernel/Kconfig.kexec
>> +++ b/kernel/Kconfig.kexec
>> @@ -36,6 +36,7 @@ config KEXEC
>> config KEXEC_FILE
>> bool "Enable kexec file based system call"
>> depends on ARCH_SUPPORTS_KEXEC_FILE
>> + depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
>
> I am not sure if the logic is correct. In theory, kexec_file code
> utilizes purgatory to verify the checksum digested during kernel loading
> when try to jump to the kernel. That means kexec_file depends on
> purgatory, but not contrary?
The expression I wrote is a bit confusing, but I think this just
keeps the existing behavior:
- on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
(powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
to be built-in.
- on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
(arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
or =m.
Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
be written as
depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
|| !ARCH_SUPPORTS_KEXEC_PURGATORY
if you find that clearer. I see that the second patch
actually gets this wrong, it should actually do
select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY
> With these changes, we can achieve the goal to avoid building issue,
> whereas the code logic becomes confusing. E.g people could disable
> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> totally useless.
>
> Not sure if I think too much over this.
I see your point here, and I would suggest changing the
CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
the availability of the purgatory code for the arch, rather
than actually controlling the code itself. I already mentioned
this for s390, but riscv would need the same thing on top.
I think the change below should address your concern.
Arnd
diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
cmdline = modified_cmdline;
}
-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
/* Add purgatory to the image */
kbuf.top_down = true;
kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
sizeof(kernel_start), 0);
if (ret)
pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */
/* Add the initrd to the image */
if (initrd != NULL) {
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
obj-y += errata/
obj-$(CONFIG_KVM) += kvm/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/
# for cleaning
subdir- += boot
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..361aa01dbd49 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS) += hypfs/
obj-$(CONFIG_APPLDATA_BASE) += appldata/
obj-y += net/
obj-$(CONFIG_PCI) += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/
# for cleaning
subdir- += boot tools
Hi Arnd,
On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
>
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >> config KEXEC_FILE
> >> bool "Enable kexec file based system call"
> >> depends on ARCH_SUPPORTS_KEXEC_FILE
> >> + depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
>
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
>
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
> (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
> to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
> (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
> or =m.
>
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
>
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
> || !ARCH_SUPPORTS_KEXEC_PURGATORY
>
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
>
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY
>
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
>
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
>
> I think the change below should address your concern.
Since no new comment, do you mind spinning v2 to wrap all these up?
Thanks
Baoquan
>
> Arnd
>
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
> cmdline = modified_cmdline;
> }
>
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
> /* Add purgatory to the image */
> kbuf.top_down = true;
> kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
> sizeof(kernel_start), 0);
> if (ret)
> pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */
>
> /* Add the initrd to the image */
> if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
> obj-y += errata/
> obj-$(CONFIG_KVM) += kvm/
>
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>
> # for cleaning
> subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS) += hypfs/
> obj-$(CONFIG_APPLDATA_BASE) += appldata/
> obj-y += net/
> obj-$(CONFIG_PCI) += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>
> # for cleaning
> subdir- += boot tools
>
On 11/30/23 10:56, Andrew Morton wrote:
> On Thu, 2 Nov 2023 16:03:18 +0800 Baoquan He <[email protected]> wrote:
>
>>>> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
>>>> totally useless.
>>>>
>>>> Not sure if I think too much over this.
>>> I see your point here, and I would suggest changing the
>>> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
>>> the availability of the purgatory code for the arch, rather
>>> than actually controlling the code itself. I already mentioned
>>> this for s390, but riscv would need the same thing on top.
>>>
>>> I think the change below should address your concern.
>> Since no new comment, do you mind spinning v2 to wrap all these up?
> This patchset remains in mm-hotfixes-unstable from the previous -rc
> cycle. Eric, do you have any comments? Arnd, do you plan on a v2? If
> not, should I merge v1? If so, should I now add cc:stable?
My apologies, I lost this. I've looked at these changes, and I am in
favor of these changes.
Furthermore, I ran the following thru the Kconfig regression script, and
did not find anything!
I believe the following patch represents the current discussion threads
around Kconfig and KEXEC/CRASH.
Reviewed-by: Eric DeVolder <[email protected]>
Tested-by: Eric DeVolder <[email protected]>
Thanks!
eric
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6f105ee4f3cf..1f11a62809f2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -608,10 +608,10 @@ config ARCH_SUPPORTS_KEXEC
def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
config ARCH_SUPPORTS_KEXEC_FILE
- def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
+ def_bool PPC64
config ARCH_SUPPORTS_KEXEC_PURGATORY
- def_bool KEXEC_FILE
+ def_bool y
config ARCH_SELECTS_KEXEC_FILE
def_bool y
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
obj-y += errata/
obj-$(CONFIG_KVM) += kvm/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/
# for cleaning
subdir- += boot
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 95a2a06acc6a..98857d76e458 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -702,9 +702,7 @@ config ARCH_SELECTS_KEXEC_FILE
select KEXEC_ELF
config ARCH_SUPPORTS_KEXEC_PURGATORY
- def_bool KEXEC_FILE
- depends on CRYPTO=y
- depends on CRYPTO_SHA256=y
+ def_bool y
config ARCH_SUPPORTS_CRASH_DUMP
def_bool y
diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image,
char *kernel_buf,
cmdline = modified_cmdline;
}
-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
/* Add purgatory to the image */
kbuf.top_down = true;
kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image,
char *kernel_buf,
sizeof(kernel_start), 0);
if (ret)
pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */
/* Add the initrd to the image */
if (initrd != NULL) {
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..f2ce80b65551 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS) += hypfs/
obj-$(CONFIG_APPLDATA_BASE) += appldata/
obj-y += net/
obj-$(CONFIG_PCI) += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/
# for cleaning
subdir- += boot tools
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 3bec98d20283..d5d8f99d1f25 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -254,13 +254,13 @@ config ARCH_SUPPORTS_KEXEC
def_bool y
config ARCH_SUPPORTS_KEXEC_FILE
- def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+ def_bool y
config ARCH_SUPPORTS_KEXEC_SIG
def_bool MODULE_SIG_FORMAT
config ARCH_SUPPORTS_KEXEC_PURGATORY
- def_bool KEXEC_FILE
+ def_bool y
config ARCH_SUPPORTS_CRASH_DUMP
def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3762f41bb092..1566748f16c4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2072,7 +2072,7 @@ config ARCH_SUPPORTS_KEXEC
def_bool y
config ARCH_SUPPORTS_KEXEC_FILE
- def_bool X86_64 && CRYPTO && CRYPTO_SHA256
+ def_bool X86_64
config ARCH_SELECTS_KEXEC_FILE
def_bool y
@@ -2080,7 +2080,7 @@ config ARCH_SELECTS_KEXEC_FILE
select HAVE_IMA_KEXEC if IMA
config ARCH_SUPPORTS_KEXEC_PURGATORY
- def_bool KEXEC_FILE
+ def_bool y
config ARCH_SUPPORTS_KEXEC_SIG
def_bool y
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f4..92120e396008 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,6 +36,7 @@ config KEXEC
config KEXEC_FILE
bool "Enable kexec file based system call"
depends on ARCH_SUPPORTS_KEXEC_FILE
+ depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
select KEXEC_CORE
help
This is new version of kexec system call. This system call is
@@ -94,10 +95,8 @@ config KEXEC_JUMP
config CRASH_DUMP
bool "kernel crash dumps"
depends on ARCH_SUPPORTS_CRASH_DUMP
- depends on ARCH_SUPPORTS_KEXEC
select CRASH_CORE
select KEXEC_CORE
- select KEXEC
help
Generate crash dump after being started by kexec.
This should be normally only set in special crash dump kernels