From f6e5aedc786ca2360a9b2aaa2fc31769a9c182d0 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 7 Dec 2016 14:41:25 -0500 Subject: [PATCH 3/4] efi: efi_memmap_insert(): don't insert a region more than once Some machines, such as the Lenovo ThinkPad W541 with firmware GNET80WW (2.28), include memory map entries with phys_addr=0x0 and num_pages=0. When this is true on a machine with an ESRT config table, efi_esrt_init() will try to reserve that table with efi_mem_reserve(). When it does, efi_arch_mem_reserve() will use efi_mem_desc_lookup() to find the memory map in question, call efi_memmap_split_count() to determine how many regions it will be split into, allocates new ram for the map, and calls efi_memmap_insert() to add the entry. efi_memmap_insert() iterates across all regions in the old map, and for each region, it tests whether the address for the new map is within that region. Unfortunately we'll try to insert the new map in *each* of these entries, causing even more duplicate entries. But efi_memmap_split_count() only returned the split count for the first entry, because it was called with the result of efi_mem_desc_lookup(), which only returns the first entry it came across. As a result, this will overflow the memory map's allocation and eventually panic. Unfortunately the method of computing the size does not handle the 0xffffffffffffffff or 0x0 edge cases well, and all valid addresses wind up being covered by any such entry. Even if the math weren't off by 1 byte in the num_pages=0 case, a duplicate entry in the table, or an entry with num_pages of ((u64)-1LL >> EFI_PAGE_SHIFT) or (u64)-1LL (i.e. either the maximum number of virtually addressable pages or just the largest value you can stuff in the field) would have the same effect. In any case, if we see more than one memory map in efi_memmap_insert() that covers the same region, it is an error to try to split up more than one of them, overflowing the allocation. So this makes it never try to split more than one region per invocation of efi_arch_mem_reserve(). I have not attempted to address the math error. Signed-off-by: Peter Jones --- drivers/firmware/efi/memmap.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c index f03ddec..5b71c717 100644 --- a/drivers/firmware/efi/memmap.c +++ b/drivers/firmware/efi/memmap.c @@ -219,6 +219,7 @@ void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf, efi_memory_desc_t *md; u64 start, end; void *old, *new; + bool did_split=false; /* modifying range */ m_start = mem->range.start; @@ -251,6 +252,15 @@ void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf, if (m_start <= start && (start < m_end && m_end < end)) { + if (did_split) { + pr_warn_once("%s: memory map for 0x%llx pages at 0x%016llx also covers address 0x%016llx. Not splitting.\n", + __func__, + md->num_pages, md->phys_addr, + m_start); + continue; + } + + did_split = true; /* first part */ md->attribute |= m_attr; md->num_pages = (m_end - md->phys_addr + 1) >> @@ -265,9 +275,18 @@ void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf, } if ((start < m_start && m_start < end) && m_end < end) { + if (did_split) { + pr_warn_once("%s: memory map for 0x%llx pages at 0x%016llx also covers address 0x%016llx. Not splitting.\n", + __func__, + md->num_pages, md->phys_addr, + m_start); + continue; + } + did_split = true; /* first part */ md->num_pages = (m_start - md->phys_addr) >> EFI_PAGE_SHIFT; + /* middle part */ new += old_memmap->desc_size; memcpy(new, old, old_memmap->desc_size); @@ -284,9 +303,17 @@ void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf, md->num_pages = (end - m_end) >> EFI_PAGE_SHIFT; } - +// else if ((start < m_start && m_start < end) && (end <= m_end)) { + if (did_split) { + pr_warn_once("%s: memory map for 0x%llx pages at 0x%016llx also covers address 0x%016llx. Not splitting.\n", + __func__, + md->num_pages, md->phys_addr, + m_start); + continue; + } + did_split = true; /* first part */ md->num_pages = (m_start - md->phys_addr) >> EFI_PAGE_SHIFT; -- 2.9.3