From ec1d8aff437117ca6c4b0d93107499ae038f94e5 Mon Sep 17 00:00:00 2001 From: David Sebek Date: Tue, 1 Jun 2021 16:32:26 -0400 Subject: [PATCH] efi: loader: Fix a boot freeze on some amd64 systems Fix an issue when the boot process could freeze shortly after displaying EFI framebuffer information on some amd64 systems. Instead of calling the efi_copy_finish function from amd64_tramp, include the copy instructions in the trampoline code itself. This avoids boot hangs and restarts in the cases when the EFI loader code and/or data segments happen to be located at a memory location that is overwritten during the copy process. Also, add missing return value checks. Add a new function that prints a warning message if there is a danger that the bootloader may attempt to overwrite memory pages of a type that should not be overwritten. Signed-off-by: David Sebek --- stand/efi/loader/arch/amd64/amd64_tramp.S | 52 ++++++++++++ stand/efi/loader/arch/amd64/elf64_freebsd.c | 42 +++++++--- stand/efi/loader/bootinfo.c | 6 +- stand/efi/loader/copy.c | 89 ++++++++++++++++++++- stand/efi/loader/loader_efi.h | 2 + 5 files changed, 180 insertions(+), 11 deletions(-) diff --git a/stand/efi/loader/arch/amd64/amd64_tramp.S b/stand/efi/loader/arch/amd64/amd64_tramp.S index 877705407f9..07354308e73 100644 --- a/stand/efi/loader/arch/amd64/amd64_tramp.S +++ b/stand/efi/loader/arch/amd64/amd64_tramp.S @@ -61,6 +61,54 @@ amd64_tramp: ALIGN_TEXT amd64_tramp_end: +/* + * void amd64_tramp_inline(uint64_t stack, uint64_t kernend, + * uint64_t modulep, uint64_t pagetable, uint64_t entry, + * uint64_t copy_dst, uint64_t copy_src, uint64_t copy_src_end) + */ + .globl amd64_tramp_inline +amd64_tramp_inline: + cli /* Make sure we don't get interrupted. */ + + movq %rsi,%r12 /* Stash the kernel values for later. */ + movq %rdx,%r13 + movq %rcx,%r14 + movq %r8,%r15 + + /* Copy the kernel from the staging area to the expected location + * in memory. The following code is equivalent to the efi_copy_finish + * function that amd64_tramp used to call. Inlining this code avoids + * a scenario when the system froze because efi_copy_finish + * overwrote its own code that just happened to be located somewhere + * in the destination range. + * + * while (copy_src < copy_src_end) *copy_dst++ = *copy_src++; + */ + movq 8(%rsp), %rax /* rax = copy_src */ + movq 16(%rsp), %rcx /* rcx = copy_src_end */ + cmpq %rcx, %rax + jnb copy_done + subq %rax, %r9 /* r9 = copy_dst - copy_src */ +loop: + movq (%rax), %rdx + movq %rdx, (%rax,%r9) + addq $8, %rax + cmpq %rax, %rcx + ja loop +copy_done: + + movq %rdi,%rsp /* Switch to our temporary stack. */ + + pushq %r12 /* Push kernend. */ + salq $32,%r13 /* Shift modulep and push it. */ + pushq %r13 + pushq %r15 /* Push the entry address. */ + movq %r14,%cr3 /* Switch page tables. */ + ret /* "Return" to kernel entry. */ + + ALIGN_TEXT +amd64_tramp_inline_end: + /* void multiboot2_exec(uint64_t entry, uint64_t multiboot_info, uint64_t stack) */ .globl multiboot2_exec multiboot2_exec: @@ -74,3 +122,7 @@ multiboot2_exec: .globl amd64_tramp_size amd64_tramp_size: .long amd64_tramp_end-amd64_tramp + + .globl amd64_tramp_inline_size +amd64_tramp_inline_size: + .long amd64_tramp_inline_end-amd64_tramp_inline diff --git a/stand/efi/loader/arch/amd64/elf64_freebsd.c b/stand/efi/loader/arch/amd64/elf64_freebsd.c index a950ca55e84..62e99ac63e6 100644 --- a/stand/efi/loader/arch/amd64/elf64_freebsd.c +++ b/stand/efi/loader/arch/amd64/elf64_freebsd.c @@ -84,11 +84,12 @@ static pml4_entry_t *PT4; static pdp_entry_t *PT3; static pd_entry_t *PT2; -static void (*trampoline)(uint64_t stack, void *copy_finish, uint64_t kernend, - uint64_t modulep, pml4_entry_t *pagetable, uint64_t entry); +static void (*trampoline)(uint64_t stack, uint64_t kernend, + uint64_t modulep, pml4_entry_t *pagetable, uint64_t entry, + uint64_t copy_dst, uint64_t copy_src, uint64_t copy_src_end); -extern uintptr_t amd64_tramp; -extern uint32_t amd64_tramp_size; +extern uintptr_t amd64_tramp_inline; +extern uint32_t amd64_tramp_inline_size; /* * There is an ELF kernel and one or more ELF modules loaded. @@ -101,6 +102,8 @@ elf64_exec(struct preloaded_file *fp) struct file_metadata *md; Elf_Ehdr *ehdr; vm_offset_t modulep, kernend, trampcode, trampstack; + uint64_t copy_dst, copy_src, copy_src_end; + EFI_STATUS status; int err, i; ACPI_TABLE_RSDP *rsdp; char buf[24]; @@ -155,16 +158,26 @@ elf64_exec(struct preloaded_file *fp) ehdr = (Elf_Ehdr *)&(md->md_data); trampcode = (vm_offset_t)0x0000000040000000; - err = BS->AllocatePages(AllocateMaxAddress, EfiLoaderData, 1, + status = BS->AllocatePages(AllocateMaxAddress, EfiLoaderData, 1, (EFI_PHYSICAL_ADDRESS *)&trampcode); + if (EFI_ERROR(status)) { + printf("Failed to allocate pages for trampoline code: %lu\n", + EFI_ERROR_CODE(status)); + return(ENOMEM); + } bzero((void *)trampcode, EFI_PAGE_SIZE); trampstack = trampcode + EFI_PAGE_SIZE - 8; - bcopy((void *)&amd64_tramp, (void *)trampcode, amd64_tramp_size); + bcopy((void *)&amd64_tramp_inline, (void *)trampcode, amd64_tramp_inline_size); trampoline = (void *)trampcode; PT4 = (pml4_entry_t *)0x0000000040000000; - err = BS->AllocatePages(AllocateMaxAddress, EfiLoaderData, 3, + status = BS->AllocatePages(AllocateMaxAddress, EfiLoaderData, 3, (EFI_PHYSICAL_ADDRESS *)&PT4); + if (EFI_ERROR(status)) { + printf("Failed to allocate pages for PT4: %lu\n", + EFI_ERROR_CODE(status)); + return(ENOMEM); + } bzero(PT4, 3 * EFI_PAGE_SIZE); PT3 = &PT4[512]; @@ -191,6 +204,15 @@ elf64_exec(struct preloaded_file *fp) printf("Start @ 0x%lx ...\n", ehdr->e_entry); + /* Check the type of memory pages that will be overwritten + * by the trampoline and print a warning message for easier + * debugging. The memory map will most likely change until + * then, but I don't expect new reserved memory blocks to + * suddenly appear. */ + if (!efi_verify_destination_type()) { + printf("Important memory pages may get overwritten!\n"); + } + efi_time_fini(); err = bi_load(fp->f_args, &modulep, &kernend, true); if (err != 0) { @@ -200,8 +222,10 @@ elf64_exec(struct preloaded_file *fp) dev_cleanup(); - trampoline(trampstack, efi_copy_finish, kernend, modulep, PT4, - ehdr->e_entry); + efi_copy_get_locations(©_dst, ©_src, ©_src_end); + + trampoline(trampstack, kernend, modulep, PT4, + ehdr->e_entry, copy_dst, copy_src, copy_src_end); panic("exec returned"); } diff --git a/stand/efi/loader/bootinfo.c b/stand/efi/loader/bootinfo.c index 9924901d29e..51fa619a1ea 100644 --- a/stand/efi/loader/bootinfo.c +++ b/stand/efi/loader/bootinfo.c @@ -444,6 +444,7 @@ bi_load(char *args, vm_offset_t *modulep, vm_offset_t *kernendp, bool exit_bs) vm_offset_t size; char *rootdevname; int howto; + int err; #if defined(LOADER_FDT_SUPPORT) vm_offset_t dtbp; int dtb_size; @@ -539,7 +540,10 @@ bi_load(char *args, vm_offset_t *modulep, vm_offset_t *kernendp, bool exit_bs) #ifdef LOADER_GELI_SUPPORT geli_export_key_metadata(kfp); #endif - bi_load_efi_data(kfp, exit_bs); + err = bi_load_efi_data(kfp, exit_bs); + if (err != 0) { + return(err); + } size = bi_copymodules(0); kernend = roundup(addr + size, PAGE_SIZE); diff --git a/stand/efi/loader/copy.c b/stand/efi/loader/copy.c index e723b61e3bc..5a3834a1983 100644 --- a/stand/efi/loader/copy.c +++ b/stand/efi/loader/copy.c @@ -208,7 +208,7 @@ efi_copy_init(void) * memory: see elf64_exec() in * boot/efi/loader/arch/amd64/elf64_freebsd.c. */ - staging = 1024*1024*1024; + staging = 1024*1024*1024 - 1; status = BS->AllocatePages(AllocateMaxAddress, EfiLoaderData, nr_pages, &staging); #else @@ -316,6 +316,9 @@ efi_copyin(const void *src, vm_offset_t dest, const size_t len) /* XXX: Callers do not check for failure. */ if (!efi_check_space(dest + stage_offset + len)) { errno = ENOMEM; + /* XXX: For now, it is better to stop + * booting instead of ignoring it. */ + panic("efi_copyin: Staging area is too small"); return (-1); } bcopy(src, (void *)(dest + stage_offset), len); @@ -329,6 +332,9 @@ efi_copyout(const vm_offset_t src, void *dest, const size_t len) /* XXX: Callers do not check for failure. */ if (src + stage_offset + len > staging_end) { errno = ENOMEM; + /* XXX: For now, it is better to stop + * booting instead of ignoring it. */ + panic("efi_copyout: Staging area is too small"); return (-1); } bcopy((void *)(src + stage_offset), dest, len); @@ -345,8 +351,13 @@ efi_readin(readin_handle_t fd, vm_offset_t dest, const size_t len) stage_offset_set = 1; } + /* XXX: Callers don't seem to differentiate between a read failure + * from fd and an insufficient space in the staging area. */ if (!efi_check_space(dest + stage_offset + len)) { errno = ENOMEM; + /* XXX: For now, at least print that the size of + * the staging area is at fault and should be larger. */ + printf("efi_readin: Staging area is too small\n"); return (-1); } return (VECTX_READ(fd, (void *)(dest + stage_offset), len)); @@ -364,3 +375,79 @@ efi_copy_finish(void) while (src < last) *dst++ = *src++; } + +void +efi_copy_get_locations(uint64_t *dst, uint64_t *src, uint64_t *src_end) +{ + *src = (uint64_t)staging; + *dst = (uint64_t)(staging - stage_offset); + *src_end = (uint64_t)staging_end; +} + +bool +efi_verify_destination_type(void) +{ + EFI_MEMORY_DESCRIPTOR *map = NULL, *p; + EFI_PHYSICAL_ADDRESS dest, dest_end; + EFI_PHYSICAL_ADDRESS page_start, page_end, overlap_start, overlap_end; + UINTN sz, key, dsz; + UINT32 dver; + EFI_STATUS status; + int i, ndesc; + bool safe; + + dest = staging - stage_offset; + dest_end = dest + (staging_end - staging); + safe = false; + + sz = 0; + + for (;;) { + status = BS->GetMemoryMap(&sz, map, &key, &dsz, &dver); + if (!EFI_ERROR(status)) + break; + + if (status != EFI_BUFFER_TOO_SMALL) { + printf("Can't read memory map: %lu\n", + EFI_ERROR_CODE(status)); + goto out; + } + + free(map); + + /* Allocate 10 descriptors more than the size reported, + * to allow for any fragmentation caused by calling + * malloc */ + map = malloc(sz + (10 * dsz)); + if (map == NULL) { + printf("Unable to allocate memory\n"); + goto out; + } + } + + safe = true; + + ndesc = sz / dsz; + for (i = 0, p = map; i < ndesc; + i++, p = NextMemoryDescriptor(p, dsz)) { + page_start = p->PhysicalStart; + page_end = page_start + p->NumberOfPages * EFI_PAGE_SIZE; + + overlap_start = page_start > dest ? page_start : dest; + overlap_end = page_end < dest_end ? page_end : dest_end; + + if (overlap_start < overlap_end && + p->Type != EfiLoaderCode && + p->Type != EfiLoaderData && + p->Type != EfiBootServicesCode && + p->Type != EfiBootServicesData && + p->Type != EfiConventionalMemory) { + safe = false; + break; + } + } + +out: + free(map); + return safe; +} \ No newline at end of file diff --git a/stand/efi/loader/loader_efi.h b/stand/efi/loader/loader_efi.h index 4d077514e42..f2e65dc95b4 100644 --- a/stand/efi/loader/loader_efi.h +++ b/stand/efi/loader/loader_efi.h @@ -44,5 +44,7 @@ ssize_t efi_readin(readin_handle_t fd, vm_offset_t dest, const size_t len); void * efi_translate(vm_offset_t ptr); void efi_copy_finish(void); +void efi_copy_get_locations(uint64_t *dst, uint64_t *src, uint64_t *src_end); +bool efi_verify_destination_type(void); #endif /* _LOADER_EFI_COPY_H_ */ -- 2.31.1