Thanks you for the patch, it passed review and validation, we got it merged.
-Ning -----Original Message----- From: Sahil Rihan [mailto:sri...@fb.com] Sent: Friday, February 10, 2017 5:01 PM To: tboot-devel@lists.sourceforge.net Subject: [tboot-devel] Fix EFI memory map handling Pass through the EFI memory map that's provided by grub2. Also remove the get_efi_memmap code which was buggy to start with (it was using the struct size instead of the descriptor size while creating the efi memmap). If no memmap is provided the kernel will panic early since it tries to compute the number of entries in the memmap by dividing memmap size by mempap descr size. Fixing the div by 0 lets the kernel make further progress, so it panics at a point where the panic is visible without needing earlyprintk. Note that booting *without* noefi is a security risk since the EFI runtime is not part of the trust base after a dynamic launch. This can be addressed by extending the chain of trust from the bootloader to tboot. Testing: Remove noefi kernel param, verify efibootmgr works. Boot with noefi kernel param, verify tboot works, but efibootmgr fails. Halt from inside kernel, verify tboot shutdown is called. Signed-off-by: Sahil Rihan <sri...@fb.com> tboot/common/e820.c | 70 ---------------------------------------------------------------------- tboot/common/linux.c | 39 ++++++++++++++++++++++----------------- tboot/common/loader.c | 20 ++++++++++++++++++++ tboot/include/e820.h | 3 --- tboot/include/loader.h | 2 ++ tboot/include/multiboot.h | 11 +++++++++++ 6 files changed, 55 insertions(+), 90 deletions(-) diff --git a/tboot/common/e820.c b/tboot/common/e820.c --- a/tboot/common/e820.c +++ b/tboot/common/e820.c @@ -60,9 +60,6 @@ static unsigned int g_nr_map; static memory_map_t *g_copy_e820_map = (memory_map_t *)TBOOT_E820_COPY_ADDR; -static efi_memory_desc_t *efi_memmap_addr = NULL; -static uint32_t efi_memmap_size = 0; - static inline void split64b(uint64_t val, uint32_t *val_lo, uint32_t *val_hi) { *val_lo = (uint32_t)(val & 0xffffffff); @@ -691,73 +688,6 @@ *ram_size = last_fit_size; } -#define PAGE_4K (1 << 12) - -efi_memory_desc_t -*get_efi_memmap(uint32_t *memmap_size) -{ - unsigned int i; - memory_map_t *mp; - efi_memory_desc_t *ep; - uint32_t space_required; - - if (efi_memmap_addr == NULL){ - /* we haven't done the conversion yet--is there room? */ - space_required = sizeof(memory_map_t) * g_nr_map + - sizeof(efi_memory_desc_t) * g_nr_map + 0xf; - if (space_required >= TBOOT_E820_COPY_SIZE){ - printk(TBOOT_ERR - "Insufficient space to make EFI copy of E820 [%d => %d]\n", - space_required, TBOOT_E820_COPY_SIZE); - return NULL; - } - /* for fun, we'll align the entries to 0x10 */ - ep = efi_memmap_addr = (efi_memory_desc_t *) - ((TBOOT_E820_COPY_ADDR + sizeof(memory_map_t) * g_nr_map + 0xf) - & ~0xf); - /* printk(TBOOT_INFO"efi memmap base now at %p\n", ep); */ - mp = g_copy_e820_map; - for (i = 0; i < g_nr_map; i++){ - uint64_t length; - ep[i].phys_addr = ep[i].virt_addr = e820_base_64(mp + i); - ep[i].pad = 0; - length = e820_length_64(mp + i); - length += PAGE_4K - 1; - length &= ~(PAGE_4K - 1); - ep[i].num_pages = length / PAGE_4K; - switch (mp[i].type){ - case E820_RAM: - ep[i].type = EFI_CONVENTIONAL_MEMORY; - ep[i].attribute |= EFI_MEMORY_WB; - break; - case E820_ACPI: - ep[i].type = EFI_ACPI_RECLAIM_MEMORY; - break; - case E820_NVS: - ep[i].type = EFI_ACPI_MEMORY_NVS; - break; - case E820_UNUSABLE: - ep[i].type = EFI_UNUSABLE_MEMORY; - break; - case E820_GAP: - case E820_MIXED: - case E820_RESERVED: - default: - ep[i].type = EFI_RESERVED_TYPE; - break; - } - /* - printk(TBOOT_INFO - "EFI entry %d at %016Lx type %d with %Lx pages\n", - i, ep[i].phys_addr, ep[i].type, ep[i].num_pages); - */ - efi_memmap_size += sizeof(efi_memory_desc_t); - } - } - *memmap_size = efi_memmap_size; - return efi_memmap_addr; -} - /* * Local variables: * mode: C diff --git a/tboot/common/linux.c b/tboot/common/linux.c --- a/tboot/common/linux.c +++ b/tboot/common/linux.c @@ -327,6 +327,7 @@ (struct screen_info_t *)(boot_params->screen_info); uint32_t address = 0; uint64_t long_address = 0UL; + uint32_t descr_size = 0, descr_vers = 0, mmap_size = 0, + efi_mmap_addr = 0; /* loader signature */ memcpy(&efi->efi_ldr_sig, "EL64", sizeof(uint32_t)); @@ -346,26 +347,30 @@ } } - /* EFI memmap descriptor size */ - efi->efi_memdescr_size = 0x30; - - /* EFI memmap descriptor version */ - efi->efi_memdescr_ver = 1; + efi_mmap_addr = find_efi_memmap(g_ldr_ctx, &descr_size, + &descr_vers, &mmap_size); + if (!efi_mmap_addr) { + printk(TBOOT_INFO"failed to get EFI memory map\n"); + efi->efi_memdescr_size = 0x1; // Avoid div by 0 in kernel. + efi->efi_memmap_size = 0; + efi->efi_memmap = 0; + } else { + efi->efi_memdescr_size = descr_size; + efi->efi_memdescr_ver = descr_vers; + efi->efi_memmap_size = mmap_size; + efi->efi_memmap = efi_mmap_addr; + /* From Multiboot2 spec: + * The bootloader must not load any part of the kernel, the modules, + * the Multiboot2 information structure, etc. higher than 4 GiB - 1. + */ + efi->efi_memmap_hi = 0; -#if 1 /* EFI memmap addr */ - { - uint32_t length; - efi->efi_memmap = (uint32_t) get_efi_memmap(&length); - /* EFI memmap size */ - efi->efi_memmap_size = length; + printk(TBOOT_INFO "EFI memmap: memmap base: 0x%x, memmap size: 0x%x\n", + efi->efi_memmap, efi->efi_memmap_size); + printk(TBOOT_INFO "EFI memmap: descr size: 0x%x, descr version: 0x%x\n", + efi->efi_memdescr_size, efi->efi_memdescr_ver); } -#else - efi->efi_memmap = 0; - efi->efi_memmap_size = 0x70; -#endif - /* EFI memmap high--since we're consing our own, we know this == 0 */ - efi->efi_memmap_hi = 0; /* if we're here, GRUB2 probably threw a framebuffer tag at us */ load_framebuffer_info(g_ldr_ctx, (void *)scr); } diff --git a/tboot/common/loader.c b/tboot/common/loader.c --- a/tboot/common/loader.c +++ b/tboot/common/loader.c @@ -1899,6 +1899,26 @@ return false; } +uint32_t +find_efi_memmap(loader_ctx *lctx, uint32_t *descr_size, + uint32_t *descr_vers, uint32_t *mmap_size) { + struct mb2_tag *start = NULL, *hit = NULL; + struct mb2_tag_efi_mmap *efi_mmap = NULL; + + start = (struct mb2_tag *)(lctx->addr + 8); + hit = find_mb2_tag_type(start, MB2_TAG_TYPE_EFI_MMAP); + if (hit == NULL) { + return 0; + } + + efi_mmap = (struct mb2_tag_efi_mmap *)hit; + *descr_size = efi_mmap->descr_size; + *descr_vers = efi_mmap->descr_vers; + *mmap_size = efi_mmap->size; + return (uint32_t)(&efi_mmap->efi_mmap); } + bool is_loader_launch_efi(loader_ctx *lctx) { diff --git a/tboot/include/e820.h b/tboot/include/e820.h --- a/tboot/include/e820.h +++ b/tboot/include/e820.h @@ -89,9 +89,6 @@ extern void get_highest_sized_ram(uint64_t size, uint64_t limit, uint64_t *ram_base, uint64_t *ram_size); -/* EFI memory map support */ -extern efi_memory_desc_t *get_efi_memmap(uint32_t *memmap_size); - /* * Memory map descriptor: */ diff --git a/tboot/include/loader.h b/tboot/include/loader.h --- a/tboot/include/loader.h +++ b/tboot/include/loader.h @@ -76,6 +76,8 @@ extern bool is_kernel_linux(void); +extern uint32_t find_efi_memmap(loader_ctx *lctx, uint32_t *descr_size, + uint32_t *descr_vers, uint32_t +*mmap_size); extern bool launch_kernel(bool is_measured_launch); extern bool verify_loader_context(loader_ctx *lctx); extern bool verify_modules(loader_ctx *lctx); diff --git a/tboot/include/multiboot.h b/tboot/include/multiboot.h --- a/tboot/include/multiboot.h +++ b/tboot/include/multiboot.h @@ -98,6 +98,8 @@ #define MB2_TAG_TYPE_ACPI_OLD 14 #define MB2_TAG_TYPE_ACPI_NEW 15 #define MB2_TAG_TYPE_NETWORK 16 +#define MB2_TAG_TYPE_EFI_MMAP 17 +#define MB2_TAG_TYPE_EFI_BS 18 #ifndef __ASSEMBLY__ @@ -314,6 +316,15 @@ struct mb2_vbe_mode_info_block vbe_mode_info; }; +struct mb2_tag_efi_mmap +{ + uint32_t type; + uint32_t size; + uint32_t descr_size; + uint32_t descr_vers; + uint8_t efi_mmap[0]; +}; + struct mb2_fb_common { uint32_t type; ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ tboot-devel mailing list tboot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tboot-devel ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ tboot-devel mailing list tboot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tboot-devel
