| 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
| ||
|
Date: Tue, 3 Mar 2020 23:26:01 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Arvind Sankar <nivedita@...m.mit.edu>
Cc: linux-efi <linux-efi@...r.kernel.org>,
"the arch/x86 maintainers" <x86@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 0/5] Minimize the need to move the kernel in the EFI stub
On Tue, 3 Mar 2020 at 23:12, Arvind Sankar <nivedita@...m.mit.edu> wrote:
>
> This series adds the ability to use the entire PE image space for
> decompression, provides the preferred address to the PE loader via the
> header, and finally restricts efi_relocate_kernel to cases where we
> really need it rather than whenever we were loaded at something other
> than preferred address.
>
> Based on tip:efi/core + the cleanup series [1]
> [1] https://lore.kernel.org/linux-efi/20200301230436.2246909-1-nivedita@alum.mit.edu/
>
> Changes from v1
> - clarify a few comments
> - cleanups to code formatting
>
> Arvind Sankar (5):
> x86/boot/compressed/32: Save the output address instead of
> recalculating it
> efi/x86: Decompress at start of PE image load address
> efi/x86: Add kernel preferred address to PE header
> efi/x86: Remove extra headroom for setup block
> efi/x86: Don't relocate the kernel unless necessary
>
Thanks. I have queued these up in efi/next, along with your mixed mode cleanups.
> arch/x86/boot/compressed/head_32.S | 42 +++++++++++++++-------
> arch/x86/boot/compressed/head_64.S | 42 ++++++++++++++++++++--
> arch/x86/boot/header.S | 6 ++--
> arch/x86/boot/tools/build.c | 44 ++++++++++++++++-------
> drivers/firmware/efi/libstub/x86-stub.c | 48 ++++++++++++++++++++++---
> 5 files changed, 147 insertions(+), 35 deletions(-)
>
> Range-diff against v1:
> 1: 0cdb6bf27a24 ! 1: 2ecbf60b9ecd x86/boot/compressed/32: Save the output address instead of recalculating it
> @@ Metadata
> ## Commit message ##
> x86/boot/compressed/32: Save the output address instead of recalculating it
>
> - In preparation for being able to decompress starting at a different
> - address than startup_32, save the calculated output address instead of
> - recalculating it later.
> + In preparation for being able to decompress into a buffer starting at a
> + different address than startup_32, save the calculated output address
> + instead of recalculating it later.
>
> We now keep track of three addresses:
> %edx: startup_32 as we were loaded by bootloader
> 2: d4df840752ac ! 2: e2bdbe6cb692 efi/x86: Decompress at start of PE image load address
> @@ arch/x86/boot/compressed/head_64.S: SYM_FUNC_START(efi32_pe_entry)
> movl -4(%ebp), %esi // loaded_image
> movl LI32_image_base(%esi), %esi // loaded_image->image_base
> movl %ebx, %ebp // startup_32 for efi32_pe_stub_entry
> ++ /*
> ++ * We need to set the image_offset variable here since startup_32 will
> ++ * use it before we get to the 64-bit efi_pe_entry in C code.
> ++ */
> + subl %esi, %ebx
> + movl %ebx, image_offset(%ebp) // save image_offset
> jmp efi32_pe_stub_entry
> @@ drivers/firmware/efi/libstub/x86-stub.c: unsigned long efi_main(efi_handle_t han
> efi_printk("efi_relocate_kernel() failed!\n");
> goto fail;
> }
> ++ /*
> ++ * Now that we've copied the kernel elsewhere, we no longer
> ++ * have a setup block before startup_32, so reset image_offset
> ++ * to zero in case it was set earlier.
> ++ */
> + image_offset = 0;
> }
>
> 3: 4bae68f25b90 ! 3: ea840f78f138 efi/x86: Add kernel preferred address to PE header
> @@ arch/x86/boot/header.S: optional_header:
>
> extra_header_fields:
> + # PE specification requires ImageBase to be 64k-aligned
> -+ .set ImageBase, (LOAD_PHYSICAL_ADDR+0xffff) & ~0xffff
> ++ .set image_base, (LOAD_PHYSICAL_ADDR + 0xffff) & ~0xffff
> #ifdef CONFIG_X86_32
> - .long 0 # ImageBase
> -+ .long ImageBase # ImageBase
> ++ .long image_base # ImageBase
> #else
> - .quad 0 # ImageBase
> -+ .quad ImageBase # ImageBase
> ++ .quad image_base # ImageBase
> #endif
> .long 0x20 # SectionAlignment
> .long 0x20 # FileAlignment
> 4: 2330a25c6b0f ! 4: c25a9b507d6d efi/x86: Remove extra headroom for setup block
> @@ Commit message
> account for setup block") added headroom to the PE image to account for
> the setup block, which wasn't used for the decompression buffer.
>
> - Now that we decompress from the start of the image, this is no longer
> - required.
> + Now that the decompression buffer is located at the start of the image,
> + and includes the setup block, this is no longer required.
>
> Add a check to make sure that the head section of the compressed kernel
> won't overwrite itself while relocating. This is only for
> 5: 2081f91cbe75 ! 5: d3dc3af1c7b8 efi/x86: Don't relocate the kernel unless necessary
> @@ arch/x86/boot/tools/build.c: static void update_pecoff_text(unsigned int text_st
> * Size of code: Subtract the size of the first sector (512 bytes)
>
> ## drivers/firmware/efi/libstub/x86-stub.c ##
> +@@
> +
> + #include "efistub.h"
> +
> ++/* Maximum physical address for 64-bit kernel with 4-level paging */
> ++#define MAXMEM_X86_64_4LEVEL (1ull << 46)
> ++
> + static efi_system_table_t *sys_table;
> + extern const bool efi_is64;
> + extern u32 image_offset;
> @@ drivers/firmware/efi/libstub/x86-stub.c: unsigned long efi_main(efi_handle_t handle,
> struct boot_params *boot_params)
> {
> @@ drivers/firmware/efi/libstub/x86-stub.c: unsigned long efi_main(efi_handle_t han
> - * address, relocate it.
> + * If the kernel isn't already loaded at a suitable address,
> + * relocate it.
> ++ *
> + * It must be loaded above LOAD_PHYSICAL_ADDR.
> -+ * The maximum address for 64-bit is 1 << 46 for 4-level paging.
> ++ *
> ++ * The maximum address for 64-bit is 1 << 46 for 4-level paging. This
> ++ * is defined as the macro MAXMEM, but unfortunately that is not a
> ++ * compile-time constant if 5-level paging is configured, so we instead
> ++ * define our own macro for use here.
> ++ *
> + * For 32-bit, the maximum address is complicated to figure out, for
> + * now use KERNEL_IMAGE_SIZE, which will be 512MiB, the same as what
> + * KASLR uses.
> ++ *
> + * Also relocate it if image_offset is zero, i.e. we weren't loaded by
> + * LoadImage, but we are not aligned correctly.
> */
> - if (bzimage_addr - image_offset != hdr->pref_address) {
> ++
> + buffer_start = ALIGN(bzimage_addr - image_offset,
> + hdr->kernel_alignment);
> + buffer_end = buffer_start + hdr->init_size;
> +
> -+ if (buffer_start < LOAD_PHYSICAL_ADDR
> -+ || IS_ENABLED(CONFIG_X86_32) && buffer_end > KERNEL_IMAGE_SIZE
> -+ || IS_ENABLED(CONFIG_X86_64) && buffer_end > 1ull << 46
> -+ || image_offset == 0 && !IS_ALIGNED(bzimage_addr,
> -+ hdr->kernel_alignment)) {
> ++ if ((buffer_start < LOAD_PHYSICAL_ADDR) ||
> ++ (IS_ENABLED(CONFIG_X86_32) && buffer_end > KERNEL_IMAGE_SIZE) ||
> ++ (IS_ENABLED(CONFIG_X86_64) && buffer_end > MAXMEM_X86_64_4LEVEL) ||
> ++ (image_offset == 0 && !IS_ALIGNED(bzimage_addr,
> ++ hdr->kernel_alignment))) {
> status = efi_relocate_kernel(&bzimage_addr,
> hdr->init_size, hdr->init_size,
> hdr->pref_address,
> --
> 2.24.1
>
Powered by blists - more mailing lists