Re: Potential config regression after 89cde455 ("kexec: consolidate kexec and crash options into kernel/Kconfig.kexec")

2023-11-22 Thread Ignat Korchagin
On Wed, Nov 22, 2023 at 9:34 AM Baoquan He  wrote:
>
> On 11/21/23 at 09:43am, Ignat Korchagin wrote:
> > On Tue, Nov 21, 2023 at 7:53 AM Ignat Korchagin  
> > wrote:
> > >
> > > On Tue, Nov 21, 2023 at 1:50 AM Baoquan He  wrote:
> > > >
> > > > Eric DeVolder's Oracle mail address is not available anymore, add his
> > > > current mail address he told me.
> > >
> > > Thank you!
> > >
> > > > On 11/20/23 at 10:52pm, Ignat Korchagin wrote:
> > > > > Good day!
> > > > >
> > > > > We have recently started to evaluate Linux 6.6 and noticed that we
> > > > > cannot disable CONFIG_KEXEC anymore, but keep CONFIG_CRASH_DUMP
> > > > > enabled. It seems to be related to commit 89cde455 ("kexec:
> > > > > consolidate kexec and crash options into kernel/Kconfig.kexec"), where
> > > > > a CONFIG_KEXEC dependency was added to CONFIG_CRASH_DUMP.
> > > > >
> > > > > In our current kernel (Linux 6.1) we only enable CONFIG_KEXEC_FILE
> > > > > with enforced signature check to support the kernel crash dumping
> > > > > functionality and would like to keep CONFIG_KEXEC disabled for
> > > > > security reasons [1].
> > > > >
> > > > > I was reading the long commit message, but the reason for adding
> > > > > CONFIG_KEXEC as a dependency for CONFIG_CRASH_DUMP evaded me. And I
> > > > > believe from the implementation perspective CONFIG_KEXEC_FILE should
> > > > > suffice here (as we successfully used it for crashdumps on Linux 6.1).
> > > > >
> > > > > Is there a reason for adding this dependency or is it just an
> > > > > oversight? Would some solution of requiring either CONFIG_KEXEC or
> > > > > CONFIG_KEXEC_FILE work here?
> > > >
> > > > I searched the patch history, found Eric didn't add the dependency on
> > > > CONFIG_KEXEC at the beginning. Later a linux-next building failure with
> > > > randconfig was reported, in there CONFIG_CRASH_DUMP enabled, while
> > > > CONFIG_KEXEC is disabled. Finally Eric added the KEXEC dependency for
> > > > CRASH_DUMP. Please see below link for more details:
> > > >
> > > > https://lore.kernel.org/all/3e8eecd1-a277-2cfb-690e-5de2eb7b9...@oracle.com/T/#u
> > >
> > > Thank you for digging this up. However I'm still confused, because
> > > this is exactly how we configure Linux 6.1 (although we do have
> > > CONFIG_KEXEC_FILE enabled) and we don't have any problems. I believe
> > > we did not investigate this issue properly.
> >
> > I did some preliminary investigation for this. If I patch out the
> > dependency on CONFIG_KEXEC the kernel builds just fine for x86
> > (without CONFIG_CRASH_HOTPLUG - which is probably another issue) - so
> > this was the previous behaviour. I can see that the reported error is
> > for arm architecture and was able to reproduce it with a simple cross
> > compiler in Debian. However, I think it is still somehow related to
> > this patchset as the previous kernels (up to 6.5) build fine with just
> > CONFIG_CRASH_DUMP and without CONFIG_KEXEC for arm as well. So even
> > for arm it was introduced in 6.6.
>
> Thanks for the information.
>
> I haven't run the reproducer of issue reported on Eric's old patchset,
> while checkout to kernel 6.1, only s390 selected KEXEC for CRASH_DUMP
> already. And with the ARM building breakage, the simplest idea is
> to select KEXEC only for ARM or S390 CRASH_DUMP. I plan to try the
> reproducer later. If you have any idea or draft patch, please feel free
> to post.

The thing is - before 6.6 even ARM did not require KEXEC for
CRASH_DUMP (at least to successfully compile), so I think we should
understand what changed first before adding a dependency for ARM. I'll
try to investigate more, if I have time.

> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f4..382dcd8d7a9d 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -97,7 +97,7 @@ config CRASH_DUMP
> depends on ARCH_SUPPORTS_KEXEC
> select CRASH_CORE
> select KEXEC_CORE
> -   select KEXEC
> +   select KEXEC if (ARM || S390)
>
>
> arch/s390/Kconfig in kernel 6.1:
> 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 

Re: Potential config regression after 89cde455 ("kexec: consolidate kexec and crash options into kernel/Kconfig.kexec")

2023-11-21 Thread Ignat Korchagin
On Tue, Nov 21, 2023 at 7:53 AM Ignat Korchagin  wrote:
>
> On Tue, Nov 21, 2023 at 1:50 AM Baoquan He  wrote:
> >
> > Eric DeVolder's Oracle mail address is not available anymore, add his
> > current mail address he told me.
>
> Thank you!
>
> > On 11/20/23 at 10:52pm, Ignat Korchagin wrote:
> > > Good day!
> > >
> > > We have recently started to evaluate Linux 6.6 and noticed that we
> > > cannot disable CONFIG_KEXEC anymore, but keep CONFIG_CRASH_DUMP
> > > enabled. It seems to be related to commit 89cde455 ("kexec:
> > > consolidate kexec and crash options into kernel/Kconfig.kexec"), where
> > > a CONFIG_KEXEC dependency was added to CONFIG_CRASH_DUMP.
> > >
> > > In our current kernel (Linux 6.1) we only enable CONFIG_KEXEC_FILE
> > > with enforced signature check to support the kernel crash dumping
> > > functionality and would like to keep CONFIG_KEXEC disabled for
> > > security reasons [1].
> > >
> > > I was reading the long commit message, but the reason for adding
> > > CONFIG_KEXEC as a dependency for CONFIG_CRASH_DUMP evaded me. And I
> > > believe from the implementation perspective CONFIG_KEXEC_FILE should
> > > suffice here (as we successfully used it for crashdumps on Linux 6.1).
> > >
> > > Is there a reason for adding this dependency or is it just an
> > > oversight? Would some solution of requiring either CONFIG_KEXEC or
> > > CONFIG_KEXEC_FILE work here?
> >
> > I searched the patch history, found Eric didn't add the dependency on
> > CONFIG_KEXEC at the beginning. Later a linux-next building failure with
> > randconfig was reported, in there CONFIG_CRASH_DUMP enabled, while
> > CONFIG_KEXEC is disabled. Finally Eric added the KEXEC dependency for
> > CRASH_DUMP. Please see below link for more details:
> >
> > https://lore.kernel.org/all/3e8eecd1-a277-2cfb-690e-5de2eb7b9...@oracle.com/T/#u
>
> Thank you for digging this up. However I'm still confused, because
> this is exactly how we configure Linux 6.1 (although we do have
> CONFIG_KEXEC_FILE enabled) and we don't have any problems. I believe
> we did not investigate this issue properly.

I did some preliminary investigation for this. If I patch out the
dependency on CONFIG_KEXEC the kernel builds just fine for x86
(without CONFIG_CRASH_HOTPLUG - which is probably another issue) - so
this was the previous behaviour. I can see that the reported error is
for arm architecture and was able to reproduce it with a simple cross
compiler in Debian. However, I think it is still somehow related to
this patchset as the previous kernels (up to 6.5) build fine with just
CONFIG_CRASH_DUMP and without CONFIG_KEXEC for arm as well. So even
for arm it was introduced in 6.6.

> > And besides, the newly added CONFIG_CRASH_HOTPLUG also needs
> > CONFIG_KEXEC if the elfcorehdr is allowed to be manipulated when
> > cpu/memory hotplug hapened.
>
> This still feels like a regression to me: any crash dump support
> should be independent of KEXEC syscalls being present. While probably
> the common case (including us) that the crashing kernel and recovery
> kernel are the same, they don't have to be. We need kexec syscall in
> the crashing kernel, but crashdump support in the recovery kernel (but
> the recovery kernel not having the kexec syscalls should be totally
> fine). If we do require some code definitions from kexec - at most we
> should put them under CONFIG_KEXEC_CORE.
>
> > Thanks
> > Baoquan
> >

Ignat


Re: Potential config regression after 89cde455 ("kexec: consolidate kexec and crash options into kernel/Kconfig.kexec")

2023-11-21 Thread Ignat Korchagin
On Tue, Nov 21, 2023 at 1:50 AM Baoquan He  wrote:
>
> Eric DeVolder's Oracle mail address is not available anymore, add his
> current mail address he told me.

Thank you!

> On 11/20/23 at 10:52pm, Ignat Korchagin wrote:
> > Good day!
> >
> > We have recently started to evaluate Linux 6.6 and noticed that we
> > cannot disable CONFIG_KEXEC anymore, but keep CONFIG_CRASH_DUMP
> > enabled. It seems to be related to commit 89cde455 ("kexec:
> > consolidate kexec and crash options into kernel/Kconfig.kexec"), where
> > a CONFIG_KEXEC dependency was added to CONFIG_CRASH_DUMP.
> >
> > In our current kernel (Linux 6.1) we only enable CONFIG_KEXEC_FILE
> > with enforced signature check to support the kernel crash dumping
> > functionality and would like to keep CONFIG_KEXEC disabled for
> > security reasons [1].
> >
> > I was reading the long commit message, but the reason for adding
> > CONFIG_KEXEC as a dependency for CONFIG_CRASH_DUMP evaded me. And I
> > believe from the implementation perspective CONFIG_KEXEC_FILE should
> > suffice here (as we successfully used it for crashdumps on Linux 6.1).
> >
> > Is there a reason for adding this dependency or is it just an
> > oversight? Would some solution of requiring either CONFIG_KEXEC or
> > CONFIG_KEXEC_FILE work here?
>
> I searched the patch history, found Eric didn't add the dependency on
> CONFIG_KEXEC at the beginning. Later a linux-next building failure with
> randconfig was reported, in there CONFIG_CRASH_DUMP enabled, while
> CONFIG_KEXEC is disabled. Finally Eric added the KEXEC dependency for
> CRASH_DUMP. Please see below link for more details:
>
> https://lore.kernel.org/all/3e8eecd1-a277-2cfb-690e-5de2eb7b9...@oracle.com/T/#u

Thank you for digging this up. However I'm still confused, because
this is exactly how we configure Linux 6.1 (although we do have
CONFIG_KEXEC_FILE enabled) and we don't have any problems. I believe
we did not investigate this issue properly.

> And besides, the newly added CONFIG_CRASH_HOTPLUG also needs
> CONFIG_KEXEC if the elfcorehdr is allowed to be manipulated when
> cpu/memory hotplug hapened.

This still feels like a regression to me: any crash dump support
should be independent of KEXEC syscalls being present. While probably
the common case (including us) that the crashing kernel and recovery
kernel are the same, they don't have to be. We need kexec syscall in
the crashing kernel, but crashdump support in the recovery kernel (but
the recovery kernel not having the kexec syscalls should be totally
fine). If we do require some code definitions from kexec - at most we
should put them under CONFIG_KEXEC_CORE.

> Thanks
> Baoquan
>


Potential config regression after 89cde455 ("kexec: consolidate kexec and crash options into kernel/Kconfig.kexec")

2023-11-20 Thread Ignat Korchagin
Good day!

We have recently started to evaluate Linux 6.6 and noticed that we
cannot disable CONFIG_KEXEC anymore, but keep CONFIG_CRASH_DUMP
enabled. It seems to be related to commit 89cde455 ("kexec:
consolidate kexec and crash options into kernel/Kconfig.kexec"), where
a CONFIG_KEXEC dependency was added to CONFIG_CRASH_DUMP.

In our current kernel (Linux 6.1) we only enable CONFIG_KEXEC_FILE
with enforced signature check to support the kernel crash dumping
functionality and would like to keep CONFIG_KEXEC disabled for
security reasons [1].

I was reading the long commit message, but the reason for adding
CONFIG_KEXEC as a dependency for CONFIG_CRASH_DUMP evaded me. And I
believe from the implementation perspective CONFIG_KEXEC_FILE should
suffice here (as we successfully used it for crashdumps on Linux 6.1).

Is there a reason for adding this dependency or is it just an
oversight? Would some solution of requiring either CONFIG_KEXEC or
CONFIG_KEXEC_FILE work here?

Ignat

[1]: https://mjg59.dreamwidth.org/28746.html