lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6cf04378-782a-1b1b-f215-92ad7cd9be6e@oracle.com>
Date:   Thu, 6 Jul 2023 11:07:13 -0500
From:   Eric DeVolder <eric.devolder@...cle.com>
To:     Alexander Gordeev <agordeev@...ux.ibm.com>,
        Nathan Chancellor <nathan@...nel.org>
Cc:     linux@...linux.org.uk, catalin.marinas@....com, will@...nel.org,
        chenhuacai@...nel.org, geert@...ux-m68k.org,
        tsbogend@...ha.franken.de, James.Bottomley@...senpartnership.com,
        deller@....de, ysato@...rs.sourceforge.jp, dalias@...c.org,
        glaubitz@...sik.fu-berlin.de, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-ia64@...r.kernel.org, loongarch@...ts.linux.dev,
        linux-m68k@...ts.linux-m68k.org, linux-mips@...r.kernel.org,
        linux-parisc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-riscv@...ts.infradead.org, linux-s390@...r.kernel.org,
        linux-sh@...r.kernel.org, kernel@...0n.name, mpe@...erman.id.au,
        npiggin@...il.com, christophe.leroy@...roup.eu,
        paul.walmsley@...ive.com, palmer@...belt.com,
        aou@...s.berkeley.edu, hca@...ux.ibm.com, gor@...ux.ibm.com,
        borntraeger@...ux.ibm.com, svens@...ux.ibm.com, hpa@...or.com,
        keescook@...omium.org, paulmck@...nel.org, peterz@...radead.org,
        frederic@...nel.org, akpm@...ux-foundation.org, ardb@...nel.org,
        samitolvanen@...gle.com, juerg.haefliger@...onical.com,
        arnd@...db.de, rmk+kernel@...linux.org.uk,
        linus.walleij@...aro.org, sebastian.reichel@...labora.com,
        rppt@...nel.org, kirill.shutemov@...ux.intel.com,
        anshuman.khandual@....com, ziy@...dia.com, masahiroy@...nel.org,
        ndesaulniers@...gle.com, mhiramat@...nel.org, ojeda@...nel.org,
        thunder.leizhen@...wei.com, xin3.li@...el.com, tj@...nel.org,
        gregkh@...uxfoundation.org, tsi@...oix.net, bhe@...hat.com,
        hbathini@...ux.ibm.com, sourabhjain@...ux.ibm.com,
        boris.ostrovsky@...cle.com, konrad.wilk@...cle.com,
        Naresh Kamboju <naresh.kamboju@...aro.org>
Subject: Re: [PATCH v4 12/13] s390/kexec: refactor for kernel/Kconfig.kexec



On 7/6/23 10:58, Alexander Gordeev wrote:
> On Wed, Jul 05, 2023 at 08:49:58AM -0700, Nathan Chancellor wrote:
> ...
>> I just bisected the following build failure visible with 'ARCH=s390
>> allnoconfig' to this change as commit 842ce0e1dafa ("s390/kexec:
>> refactor for kernel/Kconfig.kexec") in -next.
>>
>>    arch/s390/kernel/machine_kexec.c:120:37: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
>>      120 | static bool kdump_csum_valid(struct kimage *image)
>>          |                                     ^~~~~~
>>    arch/s390/kernel/machine_kexec.c:188:34: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
>>      188 | int machine_kexec_prepare(struct kimage *image)
>>          |                                  ^~~~~~
>>    arch/s390/kernel/machine_kexec.c: In function 'machine_kexec_prepare':
>>    arch/s390/kernel/machine_kexec.c:192:18: error: invalid use of undefined type 'struct kimage'
>>      192 |         if (image->type == KEXEC_TYPE_CRASH)
>>          |                  ^~
>>    arch/s390/kernel/machine_kexec.c:192:28: error: 'KEXEC_TYPE_CRASH' undeclared (first use in this function); did you mean 'KEXEC_ON_CRASH'?
>>      192 |         if (image->type == KEXEC_TYPE_CRASH)
>>          |                            ^~~~~~~~~~~~~~~~
>>          |                            KEXEC_ON_CRASH
>>    arch/s390/kernel/machine_kexec.c:192:28: note: each undeclared identifier is reported only once for each function it appears in
>>    arch/s390/kernel/machine_kexec.c:196:18: error: invalid use of undefined type 'struct kimage'
>>      196 |         if (image->type != KEXEC_TYPE_DEFAULT)
>>          |                  ^~
>>    arch/s390/kernel/machine_kexec.c:196:28: error: 'KEXEC_TYPE_DEFAULT' undeclared (first use in this function); did you mean 'KEXEC_ARCH_DEFAULT'?
>>      196 |         if (image->type != KEXEC_TYPE_DEFAULT)
>>          |                            ^~~~~~~~~~~~~~~~~~
>>          |                            KEXEC_ARCH_DEFAULT
>>    In file included from arch/s390/include/asm/thread_info.h:31,
>>                     from include/linux/thread_info.h:60,
>>                     from arch/s390/include/asm/preempt.h:6,
>>                     from include/linux/preempt.h:79,
>>                     from arch/s390/include/asm/percpu.h:5,
>>                     from include/linux/irqflags.h:18,
>>                     from include/linux/rcupdate.h:26,
>>                     from include/linux/rculist.h:11,
>>                     from include/linux/pid.h:5,
>>                     from include/linux/sched.h:14,
>>                     from include/linux/ratelimit.h:6,
>>                     from include/linux/dev_printk.h:16,
>>                     from include/linux/device.h:15,
>>                     from arch/s390/kernel/machine_kexec.c:9:
>>    arch/s390/kernel/machine_kexec.c:200:48: error: invalid use of undefined type 'struct kimage'
>>      200 |         reboot_code_buffer = page_to_virt(image->control_code_page);
>>          |                                                ^~
>>    arch/s390/include/asm/page.h:186:58: note: in definition of macro '__va'
>>      186 | #define __va(x)                 ((void *)(unsigned long)(x))
>>          |                                                          ^
>>    arch/s390/include/asm/page.h:194:38: note: in expansion of macro 'pfn_to_phys'
>>      194 | #define pfn_to_virt(pfn)        __va(pfn_to_phys(pfn))
>>          |                                      ^~~~~~~~~~~
>>    arch/s390/include/asm/page.h:199:33: note: in expansion of macro 'pfn_to_virt'
>>      199 | #define page_to_virt(page)      pfn_to_virt(page_to_pfn(page))
>>          |                                 ^~~~~~~~~~~
>>    include/asm-generic/memory_model.h:64:21: note: in expansion of macro '__page_to_pfn'
>>       64 | #define page_to_pfn __page_to_pfn
>>          |                     ^~~~~~~~~~~~~
>>    arch/s390/kernel/machine_kexec.c:200:30: note: in expansion of macro 'page_to_virt'
>>      200 |         reboot_code_buffer = page_to_virt(image->control_code_page);
>>          |                              ^~~~~~~~~~~~
>>    arch/s390/kernel/machine_kexec.c: At top level:
>>    arch/s390/kernel/machine_kexec.c:207:35: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
>>      207 | void machine_kexec_cleanup(struct kimage *image)
>>          |                                   ^~~~~~
>>    arch/s390/kernel/machine_kexec.c: In function '__do_machine_kexec':
>>    arch/s390/kernel/machine_kexec.c:243:40: error: invalid use of undefined type 'struct kimage'
>>      243 |         data_mover = page_to_phys(image->control_code_page);
>>          |                                        ^~
>>    arch/s390/include/asm/page.h:189:35: note: in definition of macro 'pfn_to_phys'
>>      189 | #define pfn_to_phys(pfn)        ((pfn) << PAGE_SHIFT)
>>          |                                   ^~~
>>    include/asm-generic/memory_model.h:64:21: note: in expansion of macro '__page_to_pfn'
>>       64 | #define page_to_pfn __page_to_pfn
>>          |                     ^~~~~~~~~~~~~
>>    arch/s390/kernel/machine_kexec.c:243:22: note: in expansion of macro 'page_to_phys'
>>      243 |         data_mover = page_to_phys(image->control_code_page);
>>          |                      ^~~~~~~~~~~~
>>    arch/s390/kernel/machine_kexec.c:244:36: error: invalid use of undefined type 'struct kimage'
>>      244 |         entry = virt_to_phys(&image->head);
>>          |                                    ^~
>>    In file included from arch/s390/kernel/machine_kexec.c:27:
>>    arch/s390/kernel/machine_kexec.c:252:40: error: invalid use of undefined type 'struct kimage'
>>      252 |                    unsigned long, image->start,
>>          |                                        ^~
>>    arch/s390/include/asm/stacktrace.h:101:32: note: in definition of macro 'CALL_LARGS_2'
>>      101 |         long arg2 = (long)(t2)(a2)
>>          |                                ^~
>>    arch/s390/include/asm/stacktrace.h:216:9: note: in expansion of macro 'CALL_LARGS_3'
>>      216 |         CALL_LARGS_##nr(__VA_ARGS__);                                   \
>>          |         ^~~~~~~~~~~
>>    arch/s390/kernel/machine_kexec.c:250:9: note: in expansion of macro 'call_nodat'
>>      250 |         call_nodat(3, void, (relocate_kernel_t)data_mover,
>>          |         ^~~~~~~~~~
>>    In file included from include/linux/irqflags.h:15:
>>    arch/s390/kernel/machine_kexec.c:252:40: error: invalid use of undefined type 'struct kimage'
>>      252 |                    unsigned long, image->start,
>>          |                                        ^~
>>    include/linux/typecheck.h:11:16: note: in definition of macro 'typecheck'
>>       11 |         typeof(x) __dummy2; \
>>          |                ^
>>    arch/s390/include/asm/stacktrace.h:136:9: note: in expansion of macro 'CALL_TYPECHECK_2'
>>      136 |         CALL_TYPECHECK_2(__VA_ARGS__);                                  \
>>          |         ^~~~~~~~~~~~~~~~
>>    arch/s390/include/asm/stacktrace.h:219:9: note: in expansion of macro 'CALL_TYPECHECK_3'
>>      219 |         CALL_TYPECHECK_##nr(__VA_ARGS__);                               \
>>          |         ^~~~~~~~~~~~~~~
>>    arch/s390/kernel/machine_kexec.c:250:9: note: in expansion of macro 'call_nodat'
>>      250 |         call_nodat(3, void, (relocate_kernel_t)data_mover,
>>          |         ^~~~~~~~~~
>>    include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
>>       12 |         (void)(&__dummy == &__dummy2); \
>>          |                         ^~
>>    arch/s390/include/asm/stacktrace.h:134:9: note: in expansion of macro 'typecheck'
>>      134 |         typecheck(t, a)
>>          |         ^~~~~~~~~
>>    arch/s390/include/asm/stacktrace.h:136:9: note: in expansion of macro 'CALL_TYPECHECK_2'
>>      136 |         CALL_TYPECHECK_2(__VA_ARGS__);                                  \
>>          |         ^~~~~~~~~~~~~~~~
>>    arch/s390/include/asm/stacktrace.h:219:9: note: in expansion of macro 'CALL_TYPECHECK_3'
>>      219 |         CALL_TYPECHECK_##nr(__VA_ARGS__);                               \
>>          |         ^~~~~~~~~~~~~~~
>>    arch/s390/kernel/machine_kexec.c:250:9: note: in expansion of macro 'call_nodat'
>>      250 |         call_nodat(3, void, (relocate_kernel_t)data_mover,
>>          |         ^~~~~~~~~~
>>    arch/s390/kernel/machine_kexec.c: At top level:
>>    arch/s390/kernel/machine_kexec.c:278:27: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
>>      278 | void machine_kexec(struct kimage *image)
>>          |                           ^~~~~~
>>    arch/s390/kernel/machine_kexec.c: In function 'machine_kexec':
>>    arch/s390/kernel/machine_kexec.c:280:18: error: invalid use of undefined type 'struct kimage'
>>      280 |         if (image->type == KEXEC_TYPE_CRASH && !kdump_csum_valid(image))
>>          |                  ^~
>>    arch/s390/kernel/machine_kexec.c:280:28: error: 'KEXEC_TYPE_CRASH' undeclared (first use in this function); did you mean 'KEXEC_ON_CRASH'?
>>      280 |         if (image->type == KEXEC_TYPE_CRASH && !kdump_csum_valid(image))
>>          |                            ^~~~~~~~~~~~~~~~
>>          |                            KEXEC_ON_CRASH
>>    arch/s390/kernel/machine_kexec.c:280:66: error: passing argument 1 of 'kdump_csum_valid' from incompatible pointer type [-Werror=incompatible-pointer-types]
>>      280 |         if (image->type == KEXEC_TYPE_CRASH && !kdump_csum_valid(image))
>>          |                                                                  ^~~~~
>>          |                                                                  |
>>          |                                                                  struct kimage *
>>    arch/s390/kernel/machine_kexec.c:120:45: note: expected 'struct kimage *' but argument is of type 'struct kimage *'
>>      120 | static bool kdump_csum_valid(struct kimage *image)
>>          |                              ~~~~~~~~~~~~~~~^~~~~
>>    cc1: some warnings being treated as errors
>>
>> I don't think this change is equivalent for s390, which had
>>
>>    config KEXEC
>>        def_bool y
>>        select KEXEC_CORE
>>
>> but it is now the equivalent of
>>
>>    config KEXEC
>>        bool "Enable kexec system call"
>>        default y
>>
>> which enables KEXEC by default but it also allows KEXEC to be disabled
>> for s390 now, because it is a user-visible symbol, not one that is
>> unconditionally enabled no matter what. If s390 can tolerate KEXEC being
>> user selectable, then I assume the fix is just adjusting
>> arch/s390/kernel/Makefile to only build the machine_kexec files when
>> CONFIG_KEXEC_CORE is set:
>>
>> diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile
>> index 6b2a051e1f8a..a06b39da95f0 100644
>> --- a/arch/s390/kernel/Makefile
>> +++ b/arch/s390/kernel/Makefile
>> @@ -37,10 +37,10 @@ CFLAGS_unwind_bc.o	+= -fno-optimize-sibling-calls
>>   obj-y	:= head64.o traps.o time.o process.o earlypgm.o early.o setup.o idle.o vtime.o
>>   obj-y	+= processor.o syscall.o ptrace.o signal.o cpcmd.o ebcdic.o nmi.o
>>   obj-y	+= debug.o irq.o ipl.o dis.o diag.o vdso.o cpufeature.o
>> -obj-y	+= sysinfo.o lgr.o os_info.o machine_kexec.o
>> +obj-y	+= sysinfo.o lgr.o os_info.o
>>   obj-y	+= runtime_instr.o cache.o fpu.o dumpstack.o guarded_storage.o sthyi.o
>>   obj-y	+= entry.o reipl.o relocate_kernel.o kdebugfs.o alternative.o
>> -obj-y	+= nospec-branch.o ipl_vmparm.o machine_kexec_reloc.o unwind_bc.o
>> +obj-y	+= nospec-branch.o ipl_vmparm.o unwind_bc.o
>>   obj-y	+= smp.o text_amode31.o stacktrace.o abs_lowcore.o
>>   
>>   extra-y				+= vmlinux.lds
>> @@ -66,6 +66,7 @@ obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
>>   obj-$(CONFIG_UPROBES)		+= uprobes.o
>>   obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o
>>   
>> +obj-$(CONFIG_KEXEC_CORE)	+= machine_kexec.o machine_kexec_reloc.o
>>   obj-$(CONFIG_KEXEC_FILE)	+= machine_kexec_file.o kexec_image.o
>>   obj-$(CONFIG_KEXEC_FILE)	+= kexec_elf.o
>>   
>>
>> Otherwise, the prompt for KEXEC could be made conditional on some ARCH
>> symbol so that architectures can opt out of it.
> 
> Hi Nathan,
> 
> Thanks a lot for looking into it!
> With few modification the fix would looke like below.
> It probably needs to be a pre-requisite for this series:
> 
> 
> [PATCH] s390/kexec: make machine_kexec depend on CONFIG_KEXEC_CORE
> 
> Make machine_kexec.o and relocate_kernel.o depend on
> CONFIG_KEXEC_CORE option as other architectures do.
> 
> Still generate machine_kexec_reloc.o unconditionally,
> since arch_kexec_do_relocs() function is neded by the
> decompressor.
> 
> Probably, #include <asm/kexec.h> could be be removed from
> machine_kexec_reloc.c source as well, but that would revert
> commit 155e6c706125 ("s390/kexec: add missing include to
> machine_kexec_reloc.c").
> 
> Suggested-by: Nathan Chancellor <nathan@...nel.org>
> Reported-by: Nathan Chancellor <nathan@...nel.org>
> Reported-by: Linux Kernel Functional Testing <lkft@...aro.org>
> Signed-off-by: Alexander Gordeev <agordeev@...ux.ibm.com>
> ---
>   arch/s390/kernel/Makefile | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile
> index 8d7514c72bb8..0df2b88cc0da 100644
> --- a/arch/s390/kernel/Makefile
> +++ b/arch/s390/kernel/Makefile
> @@ -37,9 +37,9 @@ CFLAGS_unwind_bc.o	+= -fno-optimize-sibling-calls
>   obj-y	:= head64.o traps.o time.o process.o earlypgm.o early.o setup.o idle.o vtime.o
>   obj-y	+= processor.o syscall.o ptrace.o signal.o cpcmd.o ebcdic.o nmi.o
>   obj-y	+= debug.o irq.o ipl.o dis.o diag.o vdso.o cpufeature.o
> -obj-y	+= sysinfo.o lgr.o os_info.o machine_kexec.o
> +obj-y	+= sysinfo.o lgr.o os_info.o
>   obj-y	+= runtime_instr.o cache.o fpu.o dumpstack.o guarded_storage.o sthyi.o
> -obj-y	+= entry.o reipl.o relocate_kernel.o kdebugfs.o alternative.o
> +obj-y	+= entry.o reipl.o kdebugfs.o alternative.o
>   obj-y	+= nospec-branch.o ipl_vmparm.o machine_kexec_reloc.o unwind_bc.o
>   obj-y	+= smp.o text_amode31.o stacktrace.o abs_lowcore.o
>   
> @@ -63,6 +63,7 @@ obj-$(CONFIG_RETHOOK)		+= rethook.o
>   obj-$(CONFIG_FUNCTION_TRACER)	+= ftrace.o
>   obj-$(CONFIG_FUNCTION_TRACER)	+= mcount.o
>   obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
> +obj-$(CONFIG_KEXEC_CORE)	+= machine_kexec.o relocate_kernel.o
>   obj-$(CONFIG_UPROBES)		+= uprobes.o
>   obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o
>   
>> Cheers,
>> Nathan
> 
> Thanks!

A bit of additional information. I've corrected the problem with s390 and now all config files pass 
with the olddefconfig, allyesconfig and allnoconfig targets (using approach outlined in the cover 
letter). What I did to resolve the last s390 problem is that I realized that KEXEC was 
unconditionally set, so I did the same by adding 'select KEXEC' to the config S390 section.

I'm not saying you shouldn't do the above changes, but wanted to make you aware that we're attacking 
the same problem...

I'm running my final regression run here (takes about 8 hours to get through all 385 now) and then I 
hope to post v5 within a day.

Thanks,
eric

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ