linux-kernel - Re: [PATCH v2] x86/purgatory: Remove LTO flags

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKwvOdmGSNj0FF0Mx97xAq6Zn5nc816k1QMzXQtR2C+VDq_3+Q@mail.gmail.com>
Date:   Mon, 11 Sep 2023 08:59:48 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Song Liu <song@...nel.org>
Cc:     linux-kernel@...r.kernel.org,
        Ricardo Ribalda <ribalda@...omium.org>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        kexec@...ts.infradead.org, x86@...nel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v2] x86/purgatory: Remove LTO flags

On Fri, Sep 8, 2023 at 4:13 PM Song Liu <song@...nel.org> wrote:
>
> With LTO enabled, ld.lld generates multiple .text sections for
> purgatory.ro:
>
> $ readelf -S purgatory.ro  | grep " .text"
>   [ 1] .text             PROGBITS         0000000000000000  00000040
>   [ 7] .text.purgatory   PROGBITS         0000000000000000  000020e0
>   [ 9] .text.warn        PROGBITS         0000000000000000  000021c0
>   [13] .text.sha256_upda PROGBITS         0000000000000000  000022f0
>   [15] .text.sha224_upda PROGBITS         0000000000000000  00002be0
>   [17] .text.sha256_fina PROGBITS         0000000000000000  00002bf0
>   [19] .text.sha224_fina PROGBITS         0000000000000000  00002cc0
>
> This cause WARNING from kexec_purgatory_setup_sechdrs():
>
> WARNING: CPU: 26 PID: 110894 at kernel/kexec_file.c:919
> kexec_load_purgatory+0x37f/0x390
>
> Fix this by disabling LTO for purgatory.

Thanks for the v2!

>
> Fixes: 8652d44f466a ("kexec: support purgatories with .text.hot sections")

Dunno that this fixes tag is precise.  I think perhaps

Fixes: b33fff07e3e3 ("x86, build: allow LTO to be selected")

would be more precise.

> Cc: Ricardo Ribalda <ribalda@...omium.org>
> Cc: Sami Tolvanen <samitolvanen@...gle.com>
> Cc: kexec@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> Cc: x86@...nel.org
> Cc: llvm@...ts.linux.dev
> Signed-off-by: Song Liu <song@...nel.org>
>
> ---
> AFAICT, x86 is the only arch that supports LTO and purgatory.
>
> Changes in v2:
> 1. Use CC_FLAGS_LTO instead of hardcode -flto. (Nick Desaulniers)
> ---
>  arch/x86/purgatory/Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> index c2a29be35c01..08aa0f25f12a 100644
> --- a/arch/x86/purgatory/Makefile
> +++ b/arch/x86/purgatory/Makefile
> @@ -19,6 +19,10 @@ CFLAGS_sha256.o := -D__DISABLE_EXPORTS -D__NO_FORTIFY
>  # optimization flags.
>  KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=% -fprofile-use=%,$(KBUILD_CFLAGS))
>
> +# When LTO is enabled, llvm emits many text sections, which is not supported
> +# by kexec. Remove -flto=* flags.

-flto* in LLVM implies -ffunction-sections, which creates a .text.<fn
name> section per function definition to facilitate -Wl,--gc-sections.

Overall the question here is "do we really need to optimize kexec?"

If the answer is yes, then this patch AND 8652d44f466a are both
pessimizing kexec (though having it work at all is strictly better
than not at all).  The best fix IMO would be to provide a linker
script for this purgatory image that rejoins the text sections back
into one .text.  For example:

commit eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections")

I assume people do care about the time to kexec, hence the raison
d'etre for projects like kpatch.

I'm fine to sign off on this approach if we don't really care, or want
to care more later, but we can do better here.

> +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO),$(KBUILD_CFLAGS))
> +
>  # When linking purgatory.ro with -r unresolved symbols are not checked,
>  # also link a purgatory.chk binary without -r to check for unresolved symbols.
>  PURGATORY_LDFLAGS := -e purgatory_start -z nodefaultlib
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers