2023-06-19 15:03:04

by Eric DeVolder

[permalink] [raw]
Subject: [PATCH v2 12/13] s390/kexec: refactor for kernel/Kconfig.kexec

The kexec and crash kernel options are provided in the common
kernel/Kconfig.kexec. Utilize the common options and provide
the ARCH_SUPPORTS_ and ARCH_SELECTS_ entries to recreate the
equivalent set of KEXEC and CRASH options.

NOTE: The original Kconfig has a KEXEC_SIG which depends on
MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
dependency (using the strategy outlined in this series, and other
techniques) results in 'error: recursive dependency detected'
on CRYPTO. This occurs due to any path through KEXEC_SIG
attempting to select CRYPTO is ultimately dependent upon CRYPTO:

CRYPTO
<- ARCH_SUPPORTS_KEXEC_FILE
<- KEXEC_FILE
<- KEXEC_SIG

Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
for KEXEC_SIG. In practice, however, MODULE_SIG_FORMAT is still
configured-in as the use of KEXEC_SIG is in step with the use of
SYSTEM_DATA_VERIFICATION, which does select MODULE_SIG_FORMAT.
Not ideal, but results in equivalent .config files for s390.

Signed-off-by: Eric DeVolder <[email protected]>
---
arch/s390/Kconfig | 65 ++++++++++++++---------------------------------
1 file changed, 19 insertions(+), 46 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 6dab9c1be508..58dc124433ca 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -243,6 +243,25 @@ config PGTABLE_LEVELS

source "kernel/livepatch/Kconfig"

+config ARCH_DEFAULT_KEXEC
+ def_bool y
+
+config ARCH_SUPPORTS_KEXEC
+ def_bool y
+
+config ARCH_SUPPORTS_KEXEC_FILE
+ def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+
+config ARCH_HAS_KEXEC_PURGATORY
+ def_bool KEXEC_FILE
+
+config ARCH_SUPPORTS_CRASH_DUMP
+ def_bool y
+ help
+ Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
+ This option also enables s390 zfcpdump.
+ See also <file:Documentation/s390/zfcpdump.rst>
+
menu "Processor type and features"

config HAVE_MARCH_Z10_FEATURES
@@ -481,36 +500,6 @@ config SCHED_TOPOLOGY

source "kernel/Kconfig.hz"

-config KEXEC
- def_bool y
- select KEXEC_CORE
-
-config KEXEC_FILE
- bool "kexec file based system call"
- select KEXEC_CORE
- depends on CRYPTO
- depends on CRYPTO_SHA256
- depends on CRYPTO_SHA256_S390
- help
- Enable the kexec file based system call. In contrast to the normal
- kexec system call this system call takes file descriptors for the
- kernel and initramfs as arguments.
-
-config ARCH_HAS_KEXEC_PURGATORY
- def_bool y
- depends on KEXEC_FILE
-
-config KEXEC_SIG
- bool "Verify kernel signature during kexec_file_load() syscall"
- depends on KEXEC_FILE && MODULE_SIG_FORMAT
- help
- This option makes kernel signature verification mandatory for
- the kexec_file_load() syscall.
-
- In addition to that option, you need to enable signature
- verification for the corresponding kernel image type being
- loaded in order for this to work.
-
config KERNEL_NOBP
def_bool n
prompt "Enable modified branch prediction for the kernel by default"
@@ -732,22 +721,6 @@ config VFIO_AP

endmenu

-menu "Dump support"
-
-config CRASH_DUMP
- bool "kernel crash dumps"
- select KEXEC
- help
- Generate crash dump after being started by kexec.
- Crash dump kernels are loaded in the main kernel with kexec-tools
- into a specially reserved region and then later executed after
- a crash by kdump/kexec.
- Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
- This option also enables s390 zfcpdump.
- See also <file:Documentation/s390/zfcpdump.rst>
-
-endmenu
-
config CCW
def_bool y

--
2.31.1



2023-06-21 05:44:43

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH v2 12/13] s390/kexec: refactor for kernel/Kconfig.kexec

On Mon, Jun 19, 2023 at 10:58:00AM -0400, Eric DeVolder wrote:

Hi Eric,

> The kexec and crash kernel options are provided in the common
> kernel/Kconfig.kexec. Utilize the common options and provide
> the ARCH_SUPPORTS_ and ARCH_SELECTS_ entries to recreate the
> equivalent set of KEXEC and CRASH options.
>
> NOTE: The original Kconfig has a KEXEC_SIG which depends on
> MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
> dependency (using the strategy outlined in this series, and other
> techniques) results in 'error: recursive dependency detected'
> on CRYPTO. This occurs due to any path through KEXEC_SIG
> attempting to select CRYPTO is ultimately dependent upon CRYPTO:
>
> CRYPTO
> <- ARCH_SUPPORTS_KEXEC_FILE
> <- KEXEC_FILE
> <- KEXEC_SIG
>
> Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
> for KEXEC_SIG. In practice, however, MODULE_SIG_FORMAT is still
> configured-in as the use of KEXEC_SIG is in step with the use of
> SYSTEM_DATA_VERIFICATION, which does select MODULE_SIG_FORMAT.

No, it is actually the other way around.
Could you please provide the correct explanation?

AFAICT the MODULE_SIG_FORMAT dependency was introduced with commit
c8424e776b09 ("MODSIGN: Export module signature definitions") and
in fact was not necessary, since s390 did/does not use mod_check_sig()
anyway. So the SYSTEM_DATA_VERIFICATION could have left intact.

However, the original SYSTEM_DATA_VERIFICATION seems sane and I do
not understand why other architectures do not have it also? May be
Mimi Zohar (putting on CC) could explain that?

It looks like such dependency actually exists in implicit form
(which you picked from x86):

In addition to this option, you need to enable signature
verification for the corresponding kernel image type being
loaded in order for this to work.

Does it mean that if an architecture did not enable the signature
verification type explicitly the linker could fail - both before
and after you series?

Thanks!

> Not ideal, but results in equivalent .config files for s390.
>
> Signed-off-by: Eric DeVolder <[email protected]>
> ---
> arch/s390/Kconfig | 65 ++++++++++++++---------------------------------
> 1 file changed, 19 insertions(+), 46 deletions(-)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 6dab9c1be508..58dc124433ca 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -243,6 +243,25 @@ config PGTABLE_LEVELS
>
> source "kernel/livepatch/Kconfig"
>
> +config ARCH_DEFAULT_KEXEC
> + def_bool y
> +
> +config ARCH_SUPPORTS_KEXEC
> + def_bool y
> +
> +config ARCH_SUPPORTS_KEXEC_FILE
> + def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +
> +config ARCH_HAS_KEXEC_PURGATORY
> + def_bool KEXEC_FILE
> +
> +config ARCH_SUPPORTS_CRASH_DUMP
> + def_bool y
> + help
> + Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
> + This option also enables s390 zfcpdump.
> + See also <file:Documentation/s390/zfcpdump.rst>
> +
> menu "Processor type and features"
>
> config HAVE_MARCH_Z10_FEATURES
> @@ -481,36 +500,6 @@ config SCHED_TOPOLOGY
>
> source "kernel/Kconfig.hz"
>
> -config KEXEC
> - def_bool y
> - select KEXEC_CORE
> -
> -config KEXEC_FILE
> - bool "kexec file based system call"
> - select KEXEC_CORE
> - depends on CRYPTO
> - depends on CRYPTO_SHA256
> - depends on CRYPTO_SHA256_S390
> - help
> - Enable the kexec file based system call. In contrast to the normal
> - kexec system call this system call takes file descriptors for the
> - kernel and initramfs as arguments.
> -
> -config ARCH_HAS_KEXEC_PURGATORY
> - def_bool y
> - depends on KEXEC_FILE
> -
> -config KEXEC_SIG
> - bool "Verify kernel signature during kexec_file_load() syscall"
> - depends on KEXEC_FILE && MODULE_SIG_FORMAT
> - help
> - This option makes kernel signature verification mandatory for
> - the kexec_file_load() syscall.
> -
> - In addition to that option, you need to enable signature
> - verification for the corresponding kernel image type being
> - loaded in order for this to work.
> -
> config KERNEL_NOBP
> def_bool n
> prompt "Enable modified branch prediction for the kernel by default"
> @@ -732,22 +721,6 @@ config VFIO_AP
>
> endmenu
>
> -menu "Dump support"
> -
> -config CRASH_DUMP
> - bool "kernel crash dumps"
> - select KEXEC
> - help
> - Generate crash dump after being started by kexec.
> - Crash dump kernels are loaded in the main kernel with kexec-tools
> - into a specially reserved region and then later executed after
> - a crash by kdump/kexec.
> - Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
> - This option also enables s390 zfcpdump.
> - See also <file:Documentation/s390/zfcpdump.rst>
> -
> -endmenu
> -
> config CCW
> def_bool y
>
> --
> 2.31.1
>

2023-06-21 17:48:33

by Eric DeVolder

[permalink] [raw]
Subject: Re: [PATCH v2 12/13] s390/kexec: refactor for kernel/Kconfig.kexec



On 6/21/23 00:00, Alexander Gordeev wrote:
> On Mon, Jun 19, 2023 at 10:58:00AM -0400, Eric DeVolder wrote:
>
> Hi Eric,
>
>> The kexec and crash kernel options are provided in the common
>> kernel/Kconfig.kexec. Utilize the common options and provide
>> the ARCH_SUPPORTS_ and ARCH_SELECTS_ entries to recreate the
>> equivalent set of KEXEC and CRASH options.
>>
>> NOTE: The original Kconfig has a KEXEC_SIG which depends on
>> MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
>> dependency (using the strategy outlined in this series, and other
>> techniques) results in 'error: recursive dependency detected'
>> on CRYPTO. This occurs due to any path through KEXEC_SIG
>> attempting to select CRYPTO is ultimately dependent upon CRYPTO:
>>
>> CRYPTO
>> <- ARCH_SUPPORTS_KEXEC_FILE
>> <- KEXEC_FILE
>> <- KEXEC_SIG
>>
>> Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
>> for KEXEC_SIG. In practice, however, MODULE_SIG_FORMAT is still
>> configured-in as the use of KEXEC_SIG is in step with the use of
>> SYSTEM_DATA_VERIFICATION, which does select MODULE_SIG_FORMAT.
>
> No, it is actually the other way around.
> Could you please provide the correct explanation?
>
> AFAICT the MODULE_SIG_FORMAT dependency was introduced with commit
> c8424e776b09 ("MODSIGN: Export module signature definitions") and
> in fact was not necessary, since s390 did/does not use mod_check_sig()
> anyway. So the SYSTEM_DATA_VERIFICATION could have left intact.

Thomas, would the correct explanation be simply indicating that MODULE_SIG_FORMAT isn't needed as it
is not used by s390 (crediting your summary above)?

>
> However, the original SYSTEM_DATA_VERIFICATION seems sane and I do
> not understand why other architectures do not have it also? May be
> Mimi Zohar (putting on CC) could explain that?
>
> It looks like such dependency actually exists in implicit form
> (which you picked from x86):
>
> In addition to this option, you need to enable signature
> verification for the corresponding kernel image type being
> loaded in order for this to work.
>
> Does it mean that if an architecture did not enable the signature
> verification type explicitly the linker could fail - both before
> and after you series?

As a quick test I checked x86 and it compiles/links ok if KEXEC_SIG and KEXEC_SIG_FORCE are
configured-in, but KEXEC_BZIMAGE_VERIFY_SIG (used for x86 sig verify) is not. The reason being that
the kexec_image_verify_sig() function checks if the fops.verify_sig is non-NULL before invoking the
verification. If it is NULL, the sig check fails. This would appear to be valid outcome for other
archs as well.

At any rate, I think attempting to determine if other archs need SYSTEM_DATA_VERIFICATION is out of
the scope of this series; I'm targeting just the refactor to be equivalent to what is what prior.

Thanks for looking at this!
eric
>
> Thanks!
>
>> Not ideal, but results in equivalent .config files for s390.
>>
>> Signed-off-by: Eric DeVolder <[email protected]>
>> ---
>> arch/s390/Kconfig | 65 ++++++++++++++---------------------------------
>> 1 file changed, 19 insertions(+), 46 deletions(-)
>>
>> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
>> index 6dab9c1be508..58dc124433ca 100644
>> --- a/arch/s390/Kconfig
>> +++ b/arch/s390/Kconfig
>> @@ -243,6 +243,25 @@ config PGTABLE_LEVELS
>>
>> source "kernel/livepatch/Kconfig"
>>
>> +config ARCH_DEFAULT_KEXEC
>> + def_bool y
>> +
>> +config ARCH_SUPPORTS_KEXEC
>> + def_bool y
>> +
>> +config ARCH_SUPPORTS_KEXEC_FILE
>> + def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
>> +
>> +config ARCH_HAS_KEXEC_PURGATORY
>> + def_bool KEXEC_FILE
>> +
>> +config ARCH_SUPPORTS_CRASH_DUMP
>> + def_bool y
>> + help
>> + Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
>> + This option also enables s390 zfcpdump.
>> + See also <file:Documentation/s390/zfcpdump.rst>
>> +
>> menu "Processor type and features"
>>
>> config HAVE_MARCH_Z10_FEATURES
>> @@ -481,36 +500,6 @@ config SCHED_TOPOLOGY
>>
>> source "kernel/Kconfig.hz"
>>
>> -config KEXEC
>> - def_bool y
>> - select KEXEC_CORE
>> -
>> -config KEXEC_FILE
>> - bool "kexec file based system call"
>> - select KEXEC_CORE
>> - depends on CRYPTO
>> - depends on CRYPTO_SHA256
>> - depends on CRYPTO_SHA256_S390
>> - help
>> - Enable the kexec file based system call. In contrast to the normal
>> - kexec system call this system call takes file descriptors for the
>> - kernel and initramfs as arguments.
>> -
>> -config ARCH_HAS_KEXEC_PURGATORY
>> - def_bool y
>> - depends on KEXEC_FILE
>> -
>> -config KEXEC_SIG
>> - bool "Verify kernel signature during kexec_file_load() syscall"
>> - depends on KEXEC_FILE && MODULE_SIG_FORMAT
>> - help
>> - This option makes kernel signature verification mandatory for
>> - the kexec_file_load() syscall.
>> -
>> - In addition to that option, you need to enable signature
>> - verification for the corresponding kernel image type being
>> - loaded in order for this to work.
>> -
>> config KERNEL_NOBP
>> def_bool n
>> prompt "Enable modified branch prediction for the kernel by default"
>> @@ -732,22 +721,6 @@ config VFIO_AP
>>
>> endmenu
>>
>> -menu "Dump support"
>> -
>> -config CRASH_DUMP
>> - bool "kernel crash dumps"
>> - select KEXEC
>> - help
>> - Generate crash dump after being started by kexec.
>> - Crash dump kernels are loaded in the main kernel with kexec-tools
>> - into a specially reserved region and then later executed after
>> - a crash by kdump/kexec.
>> - Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
>> - This option also enables s390 zfcpdump.
>> - See also <file:Documentation/s390/zfcpdump.rst>
>> -
>> -endmenu
>> -
>> config CCW
>> def_bool y
>>
>> --
>> 2.31.1
>>

2023-06-21 22:51:08

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v2 12/13] s390/kexec: refactor for kernel/Kconfig.kexec

On Wed, 2023-06-21 at 07:00 +0200, Alexander Gordeev wrote:
> AFAICT the MODULE_SIG_FORMAT dependency was introduced with commit
> c8424e776b09 ("MODSIGN: Export module signature definitions") and
> in fact was not necessary, since s390 did/does not use mod_check_sig()
> anyway. So the SYSTEM_DATA_VERIFICATION could have left intact.

FYI, this patch was included in the patch set to allow IMA to verify
the kexec kernel image appended signature on OpenPOWER.

--
thanks,

Mimi


2023-06-22 16:54:42

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH v2 12/13] s390/kexec: refactor for kernel/Kconfig.kexec

On Wed, Jun 21, 2023 at 12:10:49PM -0500, Eric DeVolder wrote:
Hi Eric,
...
> > > NOTE: The original Kconfig has a KEXEC_SIG which depends on
> > > MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
> > > dependency (using the strategy outlined in this series, and other
> > > techniques) results in 'error: recursive dependency detected'
> > > on CRYPTO. This occurs due to any path through KEXEC_SIG
> > > attempting to select CRYPTO is ultimately dependent upon CRYPTO:
> > >
> > > CRYPTO
> > > <- ARCH_SUPPORTS_KEXEC_FILE
> > > <- KEXEC_FILE
> > > <- KEXEC_SIG
> > >
> > > Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
> > > for KEXEC_SIG. In practice, however, MODULE_SIG_FORMAT is still
> > > configured-in as the use of KEXEC_SIG is in step with the use of
> > > SYSTEM_DATA_VERIFICATION, which does select MODULE_SIG_FORMAT.
> >
> > No, it is actually the other way around.
> > Could you please provide the correct explanation?
> >
> > AFAICT the MODULE_SIG_FORMAT dependency was introduced with commit
> > c8424e776b09 ("MODSIGN: Export module signature definitions") and
> > in fact was not necessary, since s390 did/does not use mod_check_sig()
> > anyway. So the SYSTEM_DATA_VERIFICATION could have left intact.
>
> Thomas, would the correct explanation be simply indicating that
> MODULE_SIG_FORMAT isn't needed as it is not used by s390 (crediting your
> summary above)?

I guess, you asked me? Anyway, I will try to answer as if I were Thomas :)

MODULE_SIG_FORMAT is needed to select SYSTEM_DATA_VERIFICATION.
But SYSTEM_DATA_VERIFICATION is also selected by FS_VERITY*, so
dropping MODULE_SIG_FORMAT does not hurt.

Thanks!

2023-06-22 18:44:47

by Eric DeVolder

[permalink] [raw]
Subject: Re: [PATCH v2 12/13] s390/kexec: refactor for kernel/Kconfig.kexec



On 6/22/23 11:24, Alexander Gordeev wrote:
> On Wed, Jun 21, 2023 at 12:10:49PM -0500, Eric DeVolder wrote:
> Hi Eric,
> ...
>>>> NOTE: The original Kconfig has a KEXEC_SIG which depends on
>>>> MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
>>>> dependency (using the strategy outlined in this series, and other
>>>> techniques) results in 'error: recursive dependency detected'
>>>> on CRYPTO. This occurs due to any path through KEXEC_SIG
>>>> attempting to select CRYPTO is ultimately dependent upon CRYPTO:
>>>>
>>>> CRYPTO
>>>> <- ARCH_SUPPORTS_KEXEC_FILE
>>>> <- KEXEC_FILE
>>>> <- KEXEC_SIG
>>>>
>>>> Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
>>>> for KEXEC_SIG. In practice, however, MODULE_SIG_FORMAT is still
>>>> configured-in as the use of KEXEC_SIG is in step with the use of
>>>> SYSTEM_DATA_VERIFICATION, which does select MODULE_SIG_FORMAT.
>>>
>>> No, it is actually the other way around.
>>> Could you please provide the correct explanation?
>>>
>>> AFAICT the MODULE_SIG_FORMAT dependency was introduced with commit
>>> c8424e776b09 ("MODSIGN: Export module signature definitions") and
>>> in fact was not necessary, since s390 did/does not use mod_check_sig()
>>> anyway. So the SYSTEM_DATA_VERIFICATION could have left intact.
>>
>> Thomas, would the correct explanation be simply indicating that
>> MODULE_SIG_FORMAT isn't needed as it is not used by s390 (crediting your
>> summary above)?
>
> I guess, you asked me? Anyway, I will try to answer as if I were Thomas :)
>
> MODULE_SIG_FORMAT is needed to select SYSTEM_DATA_VERIFICATION.
> But SYSTEM_DATA_VERIFICATION is also selected by FS_VERITY*, so
> dropping MODULE_SIG_FORMAT does not hurt.
>
> Thanks!

For the commit message for this s390/Kconfig change, are you ok
with the following?
eric

=====
The kexec and crash kernel options are provided in the common
kernel/Kconfig.kexec. Utilize the common options and provide
the ARCH_SUPPORTS_ and ARCH_SELECTS_ entries to recreate the
equivalent set of KEXEC and CRASH options.

NOTE: The original Kconfig has a KEXEC_SIG which depends on
MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
dependency (using the strategy outlined in this series, and other
techniques) results in 'error: recursive dependency detected'
on CRYPTO.

Per Alexander Gordeev <[email protected]>: "the MODULE_SIG_FORMAT
dependency was introduced with c8424e776b09 ("MODSIGN: Export module
signature definitions") and in fact was not necessary, since s390
did/does not use mod_check_sig() anyway. MODULE_SIG_FORMAT is needed
to select SYSTEM_DATA_VERIFICATION. But SYSTEM_DATA_VERIFICATION is
also selected by FS_VERITY*, so dropping MODULE_SIG_FORMAT does not
hurt."

Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
from KEXEC_SIG.