Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Feedback-ID: i01d149f8:Fastmail
Date:   Tue, 3 Oct 2023 04:02:54 +0200
From:   Jan Hendrik Farr <kernel@jfarr.cc>
To:     Ard Biesheuvel <ardb@google.com>
Cc:     linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
        Ard Biesheuvel <ardb@kernel.org>,
        Evgeniy Baskov <baskov@ispras.ru>,
        Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Jones <pjones@redhat.com>,
        Matthew Garrett <mjg59@srcf.ucam.org>,
        Gerd Hoffmann <kraxel@redhat.com>,
        Kees Cook <keescook@chromium.org>,
        "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH v2 00/15] x86/boot: Rework PE header generation
Message-ID: <ZRt2ToOHm2XT8MlU@desktop>
References: <20230912090051.4014114-17-ardb@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230912090051.4014114-17-ardb@google.com>
Precedence: bulk

On 12 09:00:51, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Now that the EFI stub boot flow no longer relies on memory that is
> executable and writable at the same time, we can reorganize the PE/COFF
> view of the kernel image and expose the decompressor binary's code and
> r/o data as a .text section and data/bss as a .data section, using 4k
> alignment and limited permissions.
> 
> Doing so is necessary for compatibility with hardening measures that are
> being rolled out on x86 PCs built to run Windows (i.e., the majority of
> them). The EFI boot environment that the Linux EFI stub executes in is
> especially sensitive to safety issues, given that a vulnerability in the
> loader of one OS can be abused to attack another.

This split is also useful for the work of kexecing the next kernel as an
EFI application. With the current EFI stub I have to set the memory both
writable and executable which results in W^X warnings with a default
config.

What made this more confusing was that the flags of the .text section in
current EFI stub bzImages are set to
IMAGE_SCN_MEM_EXECUTE | IMAGE_SCN_MEM_READ. So if you load that section
according to those flags the EFI stub will quickly run into issues.

I assume current firmware on x86 machines does not set any restricted
permissions on the memory. Can someone enlighten me on their behavior?


> In true x86 fashion, this is a lot more complicated than on other
> architectures, which have implemented this code/data split with 4k
> alignment from the beginning. The complicating factor here is that the
> boot image consists of two different parts, which are stitched together
> and fixed up using a special build tool.
> 
> After this series is applied, the only remaining task performed by the
> build tool is generating the CRC-32. Even though this checksum is
> usually wrong (given that distro kernels are signed for secure boot in a
> way that corrupts the CRC), this feature is retained as we cannot be
> sure that nobody is relying on this.
> 
> This supersedes the work proposed by Evgeniy last year, which did a
> major rewrite of the build tool in order to clean it up, before updating
> it to generate the new 4k aligned image layout. As this series proves,
> the build tool is mostly unnecessary, and we have too many of those
> already.
> 
> Changes since v1:
> - drop patch that removed the CRC and the build tool
> - do not use fixed setup_size but derive it in the setup.ld linker
>   script
> - reorganize the PE header so the .compat section only covers its
>   payload and the padding that follows it
> - add hpa's ack to patch #4
> 
> Cc: Evgeniy Baskov <baskov@ispras.ru>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> 
> Ard Biesheuvel (15):
>   x86/efi: Drop EFI stub .bss from .data section
>   x86/efi: Disregard setup header of loaded image
>   x86/efi: Drop alignment flags from PE section headers
>   x86/boot: Remove the 'bugger off' message
>   x86/boot: Omit compression buffer from PE/COFF image memory footprint
>   x86/boot: Drop redundant code setting the root device
>   x86/boot: Grab kernel_info offset from zoffset header directly
>   x86/boot: Drop references to startup_64
>   x86/boot: Set EFI handover offset directly in header asm
>   x86/boot: Define setup size in linker script
>   x86/boot: Derive file size from _edata symbol
>   x86/boot: Construct PE/COFF .text section from assembler
>   x86/boot: Drop PE/COFF .reloc section
>   x86/boot: Split off PE/COFF .data section
>   x86/boot: Increase section and file alignment to 4k/512
> 
>  arch/x86/boot/Makefile                  |   2 +-
>  arch/x86/boot/compressed/vmlinux.lds.S  |   6 +-
>  arch/x86/boot/header.S                  | 213 ++++++---------
>  arch/x86/boot/setup.ld                  |  14 +-
>  arch/x86/boot/tools/build.c             | 273 +-------------------
>  drivers/firmware/efi/libstub/Makefile   |   7 -
>  drivers/firmware/efi/libstub/x86-stub.c |  46 +---
>  7 files changed, 114 insertions(+), 447 deletions(-)
> 
> -- 
> 2.42.0.283.g2d96d420d3-goog
>