lkml.org 
[lkml]   [2022]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 13/16] efi/x86: Support extracting kernel from libstub
    On Tue, 6 Sept 2022 at 12:42, Evgeniy Baskov <baskov@ispras.ru> wrote:
    >
    > Doing it that way allows setting up stricter memory attributes,
    > simplifies boot code path and removes potential relocation
    > of kernel image.
    >
    > Wire up required interfaces and minimally initialize zero page
    > fields needed for it to function correctly.
    >
    > Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
    >
    > create mode 100644 arch/x86/include/asm/shared/extract.h
    > create mode 100644 drivers/firmware/efi/libstub/x86-extract-direct.c
    > ---
    > arch/x86/boot/compressed/head_32.S | 6 +-
    > arch/x86/boot/compressed/head_64.S | 45 ++++
    > arch/x86/include/asm/shared/extract.h | 25 ++
    > drivers/firmware/efi/Kconfig | 14 ++
    > drivers/firmware/efi/libstub/Makefile | 1 +
    > drivers/firmware/efi/libstub/efistub.h | 5 +
    > .../firmware/efi/libstub/x86-extract-direct.c | 220 ++++++++++++++++++
    > drivers/firmware/efi/libstub/x86-stub.c | 45 ++--
    > 8 files changed, 343 insertions(+), 18 deletions(-)
    > create mode 100644 arch/x86/include/asm/shared/extract.h
    > create mode 100644 drivers/firmware/efi/libstub/x86-extract-direct.c
    >
    > diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
    > index b46a1c4109cf..d2866f06bc9f 100644
    > --- a/arch/x86/boot/compressed/head_32.S
    > +++ b/arch/x86/boot/compressed/head_32.S
    > @@ -155,7 +155,11 @@ SYM_FUNC_START(efi32_stub_entry)
    > add $0x4, %esp
    > movl 8(%esp), %esi /* save boot_params pointer */
    > call efi_main
    > - /* efi_main returns the possibly relocated address of startup_32 */
    > +
    > + /*
    > + * efi_main returns the possibly
    > + * relocated address of exteracted kernel entry point.

    extracted

    > + */
    > jmp *%eax
    > SYM_FUNC_END(efi32_stub_entry)
    > SYM_FUNC_ALIAS(efi_stub_entry, efi32_stub_entry)
    > diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
    > index 37ce094571b5..b6bae8e7ee71 100644
    > --- a/arch/x86/boot/compressed/head_64.S
    > +++ b/arch/x86/boot/compressed/head_64.S
    > @@ -555,9 +555,54 @@ SYM_FUNC_START(efi64_stub_entry)
    > and $~0xf, %rsp /* realign the stack */
    > movq %rdx, %rbx /* save boot_params pointer */
    > call efi_main
    > +
    > +#ifdef CONFIG_EFI_STUB_EXTRACT_DIRECT
    > + cld
    > + cli
    > +
    > + movq %rbx, %rdi /* boot_params */
    > + movq %rax, %rsi /* decompressed kernel address */
    > +
    > + /* Make sure we have GDT with 32-bit code segment */
    > + leaq gdt64(%rip), %rax
    > + addq %rax, 2(%rax)
    > + lgdt (%rax)
    > +
    > + /* Setup data segments. */
    > + xorl %eax, %eax
    > + movl %eax, %ds
    > + movl %eax, %es
    > + movl %eax, %ss
    > + movl %eax, %fs
    > + movl %eax, %gs
    > +
    > + pushq %rsi
    > + pushq %rdi
    > +
    > + call startup32_enable_nx_if_supported
    > +
    > + call trampoline_pgtable_init
    > + movq %rax, %rdx
    > +
    > +
    > + /* Swap %rsi and %rsi */
    > + popq %rsi
    > + popq %rdi
    > +
    > + /* Save the trampoline address in RCX */
    > + movq trampoline_32bit(%rip), %rcx
    > +
    > + /* Switch to compatibility mode (CS.L = 0 CS.D = 1) via far return */
    > + pushq $__KERNEL32_CS
    > + leaq TRAMPOLINE_32BIT_CODE_OFFSET(%rcx), %rax
    > + pushq %rax
    > + lretq
    > +#else
    > movq %rbx,%rsi
    > leaq rva(startup_64)(%rax), %rax
    > jmp *%rax
    > +#endif
    > +
    > SYM_FUNC_END(efi64_stub_entry)
    > SYM_FUNC_ALIAS(efi_stub_entry, efi64_stub_entry)
    > #endif
    > diff --git a/arch/x86/include/asm/shared/extract.h b/arch/x86/include/asm/shared/extract.h
    > new file mode 100644
    > index 000000000000..163678145884
    > --- /dev/null
    > +++ b/arch/x86/include/asm/shared/extract.h
    > @@ -0,0 +1,25 @@
    > +/* SPDX-License-Identifier: GPL-2.0 */
    > +#ifndef ASM_SHARED_EXTRACT_H
    > +#define ASM_SHARED_EXTRACT_H
    > +
    > +#define MAP_WRITE 0x02 /* Writable memory */
    > +#define MAP_EXEC 0x04 /* Executable memory */
    > +#define MAP_ALLOC 0x10 /* Range needs to be allocated */
    > +#define MAP_PROTECT 0x20 /* Set exact memory attributes for memory range */
    > +
    > +struct efi_iofunc {
    > + void (*putstr)(const char *msg);
    > + void (*puthex)(unsigned long x);
    > + unsigned long (*map_range)(unsigned long start,
    > + unsigned long end,
    > + unsigned int flags);

    This looks a bit random - having a map_range() routine as a member of
    the console I/O struct. Can we make this abstraction a bit more
    natural?

    > +};
    > +
    > +void *efi_extract_kernel(struct boot_params *rmode,
    > + struct efi_iofunc *iofunc,
    > + unsigned char *input_data,
    > + unsigned long input_len,
    > + unsigned char *output,
    > + unsigned long output_len);
    > +
    > +#endif /* ASM_SHARED_EXTRACT_H */
    > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
    > index 6cb7384ad2ac..2418402a0bda 100644
    > --- a/drivers/firmware/efi/Kconfig
    > +++ b/drivers/firmware/efi/Kconfig
    > @@ -91,6 +91,20 @@ config EFI_DXE_MEM_ATTRIBUTES
    > Use DXE services to check and alter memory protection
    > attributes during boot via EFISTUB to ensure that memory
    > ranges used by the kernel are writable and executable.
    > + This option also enables stricter memory attributes
    > + on compressed kernel PE image.
    > +
    > +config EFI_STUB_EXTRACT_DIRECT
    > + bool "Extract kernel directly from UEFI environment"
    > + depends on EFI && EFI_STUB && X86_64
    > + default y

    What is the reason for making this configurable? Couldn't we just
    enable it unconditionally?

    > + help
    > + Extract kernel before exiting EFI boot services
    > + This allows maintaining W^X for kernel image for
    > + the whole execution of compressed kernel code.
    > + This also slightly improves efficiency of extraction
    > + code by removing the need to copy the kernel around
    > + and rebuild page tables.
    >
    > config EFI_PARAMS_FROM_FDT
    > bool
    > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
    > index d0537573501e..1cea7d913356 100644
    > --- a/drivers/firmware/efi/libstub/Makefile
    > +++ b/drivers/firmware/efi/libstub/Makefile
    > @@ -69,6 +69,7 @@ lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o \
    > lib-$(CONFIG_ARM) += arm32-stub.o
    > lib-$(CONFIG_ARM64) += arm64-stub.o
    > lib-$(CONFIG_X86) += x86-stub.o
    > +lib-$(CONFIG_EFI_STUB_EXTRACT_DIRECT) += x86-extract-direct.o
    > lib-$(CONFIG_RISCV) += riscv-stub.o
    > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
    >
    > diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
    > index 22fe28385db7..cdd1bb50c786 100644
    > --- a/drivers/firmware/efi/libstub/efistub.h
    > +++ b/drivers/firmware/efi/libstub/efistub.h
    > @@ -968,6 +968,11 @@ static inline void
    > efi_enable_reset_attack_mitigation(void) { }
    > #endif
    >
    > +#ifdef CONFIG_X86
    > +unsigned long extract_kernel_direct(struct boot_params *boot_params);
    > +void startup_32(struct boot_params *boot_params);
    > +#endif
    > +

    Please put this somewhere else

    > void efi_retrieve_tpm2_eventlog(void);
    >
    > #endif
    > diff --git a/drivers/firmware/efi/libstub/x86-extract-direct.c b/drivers/firmware/efi/libstub/x86-extract-direct.c
    > new file mode 100644
    > index 000000000000..6076bd75cfd6
    > --- /dev/null
    > +++ b/drivers/firmware/efi/libstub/x86-extract-direct.c
    > @@ -0,0 +1,220 @@
    > +// SPDX-License-Identifier: GPL-2.0-only
    > +
    > +#include <linux/acpi.h>
    > +#include <linux/efi.h>
    > +#include <linux/elf.h>
    > +#include <linux/stddef.h>
    > +
    > +#include <asm/efi.h>
    > +#include <asm/e820/types.h>
    > +#include <asm/desc.h>
    > +#include <asm/boot.h>
    > +#include <asm/bootparam_utils.h>
    > +#include <asm/shared/extract.h>
    > +#include <asm/shared/pgtable.h>
    > +
    > +#include "efistub.h"
    > +
    > +static void do_puthex(unsigned long value);
    > +static void do_putstr(const char *msg);
    > +

    Can we get rid of these forward declarations?

    > +static unsigned long do_map_range(unsigned long start,
    > + unsigned long end,
    > + unsigned int flags)
    > +{
    > + efi_status_t status;
    > +
    > + unsigned long size = end - start;
    > +
    > + if (flags & MAP_ALLOC) {
    > + if (start == (unsigned long)startup_32)
    > + start = LOAD_PHYSICAL_ADDR;
    > +
    > + unsigned long addr;
    > +
    > + status = efi_low_alloc_above(size, CONFIG_PHYSICAL_ALIGN,
    > + &addr, start);
    > + if (status != EFI_SUCCESS)
    > + efi_err("Unable to allocate memory for uncompressed kernel");
    > +
    > + if (start != addr) {
    > + efi_debug("Unable to allocate at given address"
    > + " (desired=0x%lx, actual=0x%lx)",
    > + (unsigned long)start, addr);
    > + start = addr;
    > + }
    > + }
    > +
    > + if (flags & (MAP_PROTECT | MAP_ALLOC)) {
    > + unsigned long attr = 0;
    > +
    > + if (!(flags & MAP_EXEC))
    > + attr |= EFI_MEMORY_XP;
    > +
    > + if (!(flags & MAP_WRITE))
    > + attr |= EFI_MEMORY_RO;
    > +
    > + status = efi_adjust_memory_range_protection(start,
    > + end - start,
    > + attr);
    > + if (status != EFI_SUCCESS)
    > + efi_err("Unable to protect memory range");
    > + }
    > +
    > + return start;
    > +}
    > +
    > +/*
    > + * Trampoline takes 3 pages and can be loaded in first megabyte of memory
    > + * with its end placed between 0 and 640k where BIOS might start.
    > + * (see arch/x86/boot/compressed/pgtable_64.c)
    > + */
    > +
    > +#ifdef CONFIG_64BIT
    > +static efi_status_t prepare_trampoline(void)
    > +{
    > + efi_status_t status;
    > +
    > + status = efi_allocate_pages(TRAMPOLINE_32BIT_SIZE,
    > + (unsigned long *)&trampoline_32bit,
    > + TRAMPOLINE_32BIT_PLACEMENT_MAX);
    > +
    > + if (status != EFI_SUCCESS)
    > + return status;
    > +
    > + unsigned long trampoline_start = (unsigned long)trampoline_32bit;
    > +
    > + memset(trampoline_32bit, 0, TRAMPOLINE_32BIT_SIZE);
    > +
    > + /* First page of trampoline is a top level page table */
    > + efi_adjust_memory_range_protection(trampoline_start,
    > + PAGE_SIZE,
    > + EFI_MEMORY_XP);
    > +
    > + /* Second page of trampoline is the code (with a padding) */
    > +
    > + void *caddr = (void *)trampoline_32bit + TRAMPOLINE_32BIT_CODE_OFFSET;
    > +
    > + memcpy(caddr, trampoline_32bit_src, TRAMPOLINE_32BIT_CODE_SIZE);
    > +
    > + efi_adjust_memory_range_protection((unsigned long)caddr,
    > + PAGE_SIZE,
    > + EFI_MEMORY_RO);
    > +
    > + /* And the last page of trampoline is the stack */
    > +
    > + efi_adjust_memory_range_protection(trampoline_start + 2 * PAGE_SIZE,
    > + PAGE_SIZE,
    > + EFI_MEMORY_XP);
    > +
    > + return EFI_SUCCESS;
    > +}
    > +#else
    > +static inline efi_status_t prepare_trampoline(void)
    > +{
    > + return EFI_SUCCESS;
    > +}
    > +#endif
    > +
    > +static efi_status_t init_loader_data(struct boot_params *params)
    > +{
    > + struct efi_info *efi = (void *)&params->efi_info;
    > + efi_status_t status;
    > +
    > + unsigned long map_size, desc_size, buff_size;
    > + u32 desc_ver;
    > + efi_memory_desc_t *mmap;
    > +
    > + struct efi_boot_memmap map = {
    > + .map = &mmap,
    > + .map_size = &map_size,
    > + .desc_size = &desc_size,
    > + .desc_ver = &desc_ver,
    > + .key_ptr = NULL,
    > + .buff_size = &buff_size,
    > + };
    > +
    > + status = efi_get_memory_map(&map);

    efi_get_memory_map() has been updated in the mean time, so this needs a rewrite.

    > +
    > + if (status != EFI_SUCCESS) {
    > + efi_err("Unable to get EFI memory map...\n");
    > + return status;
    > + }
    > +
    > + const char *signature = efi_is_64bit() ? EFI64_LOADER_SIGNATURE
    > + : EFI32_LOADER_SIGNATURE;
    > +
    > + memcpy(&efi->efi_loader_signature, signature, sizeof(__u32));
    > +
    > + efi->efi_memdesc_size = desc_size;
    > + efi->efi_memdesc_version = desc_ver;
    > + efi->efi_memmap_size = map_size;
    > +
    > + efi_set_u64_split((unsigned long)mmap,
    > + &efi->efi_memmap, &efi->efi_memmap_hi);
    > +
    > + efi_set_u64_split((unsigned long)efi_system_table,
    > + &efi->efi_systab, &efi->efi_systab_hi);
    > +
    > + return EFI_SUCCESS;
    > +}
    > +
    > +static void free_loader_data(struct boot_params *params)
    > +{
    > + struct efi_info *efi = (void *)&params->efi_info;
    > + unsigned long mmap = efi->efi_memmap;
    > +
    > +#ifdef CONFIG_64BIT
    > + mmap |= ((unsigned long)efi->efi_memmap_hi << 32);
    > +#endif
    > +
    > + efi_bs_call(free_pool, (void *)mmap);
    > +
    > + efi->efi_memdesc_size = 0;
    > + efi->efi_memdesc_version = 0;
    > + efi->efi_memmap_size = 0;
    > + efi_set_u64_split(0, &efi->efi_memmap, &efi->efi_memmap_hi);
    > +}
    > +
    > +unsigned long extract_kernel_direct(struct boot_params *params)
    > +{
    > +
    > + extern unsigned char input_data[];
    > + extern unsigned int output_len, input_len;
    > +
    > + void *res;
    > + efi_status_t status;
    > + struct efi_iofunc iof = { 0 };
    > +
    > + status = prepare_trampoline();
    > +
    > + if (status != EFI_SUCCESS)
    > + return 0;
    > +
    > + /* Prepare environment for do_extract_kernel() call */
    > + status = init_loader_data(params);
    > +
    > + if (status != EFI_SUCCESS)
    > + return 0;
    > +
    > + iof.puthex = do_puthex;
    > + iof.putstr = do_putstr;
    > + iof.map_range = do_map_range;
    > +
    > + res = efi_extract_kernel(params, &iof, input_data, input_len,
    > + (unsigned char *)startup_32, output_len);
    > +
    > + free_loader_data(params);
    > +
    > + return (unsigned long)res;
    > +}
    > +
    > +static void do_puthex(unsigned long value)
    > +{
    > + efi_printk("%08lx", value);
    > +}
    > +
    > +static void do_putstr(const char *msg)
    > +{
    > + efi_printk("%s", msg);
    > +}
    > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
    > index 678f9c2ccafc..680184034cb7 100644
    > --- a/drivers/firmware/efi/libstub/x86-stub.c
    > +++ b/drivers/firmware/efi/libstub/x86-stub.c
    > @@ -230,26 +230,25 @@ static void
    > setup_memory_protection(unsigned long image_base, unsigned long image_size)
    > {
    > /*
    > - * Allow execution of possible trampoline used
    > - * for switching between 4- and 5-level page tables
    > - * and relocated kernel image.
    > - */
    > + * Allow execution of possible trampoline used
    > + * for switching between 4- and 5-level page tables
    > + * and relocated kernel image.
    > + */
    >

    Drop this hunk please

    > efi_adjust_memory_range_protection(TRAMPOLINE_PLACEMENT_BASE,
    > TRAMPOLINE_PLACEMENT_SIZE, 0);
    >
    > #ifdef CONFIG_64BIT
    > - if (image_base != (unsigned long)startup_32)
    > - efi_adjust_memory_range_protection(image_base, image_size, 0);
    > + efi_adjust_memory_range_protection(image_base, image_size, 0);
    > #else
    > /*
    > - * Clear protection flags on a whole range of possible
    > - * addresses used for KASLR. We don't need to do that
    > - * on x86_64, since KASLR/extraction is performed after
    > - * dedicated identity page tables are built and we only
    > - * need to remove possible protection on relocated image
    > - * itself disregarding further relocations.
    > - */
    > + * Clear protection flags on a whole range of possible
    > + * addresses used for KASLR. We don't need to do that
    > + * on x86_64, since KASLR/extraction is performed after
    > + * dedicated identity page tables are built and we only
    > + * need to remove possible protection on relocated image
    > + * itself disregarding further relocations.
    > + */

    Drop this hunk please

    > efi_adjust_memory_range_protection(LOAD_PHYSICAL_ADDR,
    > KERNEL_IMAGE_SIZE - LOAD_PHYSICAL_ADDR,
    > 0);
    > @@ -270,8 +269,10 @@ static void setup_quirks(struct boot_params *boot_params,
    > retrieve_apple_device_properties(boot_params);
    > }
    >
    > - if (IS_ENABLED(CONFIG_EFI_DXE_MEM_ATTRIBUTES))
    > + if (IS_ENABLED(CONFIG_EFI_DXE_MEM_ATTRIBUTES) &&
    > + !IS_ENABLED(CONFIG_EFI_STUB_EXTRACT_DIRECT)) {
    > setup_memory_protection(image_base, image_size);
    > + }
    > }
    >
    > /*
    > @@ -710,8 +711,10 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
    > }
    >
    > /*
    > - * On success, we return the address of startup_32, which has potentially been
    > - * relocated by efi_relocate_kernel.
    > + * On success, we return:
    > + * - the address of startup_32, which has potentially been
    > + * relocated by efi_relocate_kernel, if libstub direct extraction is disabled.
    > + * - extracted kernel entry point if libstub direct extraction is enabled.
    > * On failure, we exit to the firmware via efi_exit instead of returning.
    > */
    > unsigned long efi_main(efi_handle_t handle,
    > @@ -736,6 +739,7 @@ unsigned long efi_main(efi_handle_t handle,
    > efi_dxe_table = NULL;
    > }
    >
    > +#ifndef CONFIG_EFI_STUB_EXTRACT_DIRECT
    > /*
    > * If the kernel isn't already loaded at a suitable address,
    > * relocate it.
    > @@ -789,6 +793,7 @@ unsigned long efi_main(efi_handle_t handle,
    > */
    > image_offset = 0;
    > }
    > +#endif
    >
    > #ifdef CONFIG_CMDLINE_BOOL
    > status = efi_parse_options(CONFIG_CMDLINE);
    > @@ -845,7 +850,13 @@ unsigned long efi_main(efi_handle_t handle,
    >
    > setup_efi_pci(boot_params);
    >
    > - setup_quirks(boot_params, bzimage_addr, buffer_end - buffer_start);
    > + setup_quirks(boot_params, buffer_start, buffer_end - buffer_start);
    > +
    > +#ifdef CONFIG_EFI_STUB_EXTRACT_DIRECT
    > + bzimage_addr = extract_kernel_direct(boot_params);
    > + if (!bzimage_addr)
    > + goto fail;
    > +#endif
    >
    > status = exit_boot(boot_params, handle);
    > if (status != EFI_SUCCESS) {
    > --
    > 2.35.1
    >

    \
     
     \ /
      Last update: 2022-10-19 09:36    [W:5.232 / U:0.564 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site