From 71ec70ab2af18413e0631c497d5168a5fa4c6d1d Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 13 Dec 2016 11:12:18 -0500 Subject: [PATCH] Update UEFI memory map cleaning Signed-off-by: Peter Jones --- ...mmap-Call-out-invalid-entries-in-the.patch | 61 -------- ...efi-prune-invalid-memory-map-entries.patch | 141 ++++++++++++++++++ ...on-traceback-if-we-try-to-map-invali.patch | 71 --------- ...nsert-don-t-insert-a-region-more-tha.patch | 114 -------------- ...nsert-don-t-split-regions-with-inval.patch | 45 ------ kernel.spec | 5 +- 6 files changed, 142 insertions(+), 295 deletions(-) delete mode 100644 0001-efi-efi_print_memmap-Call-out-invalid-entries-in-the.patch create mode 100644 0001-efi-prune-invalid-memory-map-entries.patch delete mode 100644 0002-efi-efi_map_region-traceback-if-we-try-to-map-invali.patch delete mode 100644 0003-efi-efi_memmap_insert-don-t-insert-a-region-more-tha.patch delete mode 100644 0004-efi-efi_memmap_insert-don-t-split-regions-with-inval.patch diff --git a/0001-efi-efi_print_memmap-Call-out-invalid-entries-in-the.patch b/0001-efi-efi_print_memmap-Call-out-invalid-entries-in-the.patch deleted file mode 100644 index 037a60c33..000000000 --- a/0001-efi-efi_print_memmap-Call-out-invalid-entries-in-the.patch +++ /dev/null @@ -1,61 +0,0 @@ -From 64ec700cb0b75047ce2ac68cca020e75369cb409 Mon Sep 17 00:00:00 2001 -From: Peter Jones -Date: Wed, 7 Dec 2016 16:19:20 -0500 -Subject: [PATCH 1/4] efi: efi_print_memmap(): Call out invalid entries in the - memory map early. - -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. -Currently the log output for this case (with efi=debug) looks like: - -[ 0.000000] efi: mem45: [Reserved | | | | | | | | | | | | ] range=[0x0000000000000000-0xffffffffffffffff] (0MB) - -This is clearly wrong, and also not as informative as it could be. This -patch changes it so that if we find obviously invalid memory map -entries, say so when we're printing the memory map. It also detects the -display of the address range calculation overflow, so the new output is: - -[ 0.000000] efi: [Firmware Bug]: Invalid EFI memory map entry for 0x0 pages at 0x0000000000000000 -[ 0.000000] efi: mem45: [Reserved | | | | | | | | | | | | ] range=[0x0000000000000000-0x0000000000000000] (0MB) - -Signed-off-by: Peter Jones ---- - arch/x86/platform/efi/efi.c | 18 +++++++++++++++++- - 1 file changed, 17 insertions(+), 1 deletion(-) - -diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c -index bf99aa7..181c915 100644 ---- a/arch/x86/platform/efi/efi.c -+++ b/arch/x86/platform/efi/efi.c -@@ -217,11 +217,27 @@ void __init efi_print_memmap(void) - - for_each_efi_memory_desc(md) { - char buf[64]; -+ bool valid = true; -+ u64 size = (md->num_pages << EFI_PAGE_SHIFT) - 1; -+ -+ if (md->num_pages == 0) { -+ size = 0; -+ valid = false; -+ } -+ -+ if (md->num_pages >= (((u64)-1LL) >> EFI_PAGE_SHIFT)) { -+ size = (u64)-1LL; -+ valid = false; -+ } -+ -+ if (!valid) -+ pr_info(FW_BUG "Invalid EFI memory map entry for 0x%llx pages at 0x%016llx\n", -+ md->num_pages, md->phys_addr); - - pr_info("mem%02u: %s range=[0x%016llx-0x%016llx] (%lluMB)\n", - i++, efi_md_typeattr_format(buf, sizeof(buf), md), - md->phys_addr, -- md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1, -+ md->phys_addr + size, - (md->num_pages >> (20 - EFI_PAGE_SHIFT))); - } - } --- -2.9.3 - diff --git a/0001-efi-prune-invalid-memory-map-entries.patch b/0001-efi-prune-invalid-memory-map-entries.patch new file mode 100644 index 000000000..4b65ef504 --- /dev/null +++ b/0001-efi-prune-invalid-memory-map-entries.patch @@ -0,0 +1,141 @@ +From c7c7030a020405d5826c03839e38986e0f78f2ea Mon Sep 17 00:00:00 2001 +From: Peter Jones +Date: Tue, 13 Dec 2016 10:25:10 +0000 +Subject: [PATCH] efi: prune invalid memory map entries + +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. + +Currently the log output for this case (with efi=debug) looks like: + +[ 0.000000] efi: mem45: [Reserved | | | | | | | | | | | | ] range=[0x0000000000000000-0xffffffffffffffff] (0MB) + +This is clearly wrong, and also not as informative as it could be. This +patch changes it so that if we find obviously invalid memory map +entries, we print an error and those entries. It also detects the +display of the address range calculation overflow, so the new output is: + +[ 0.000000] efi: [Firmware Bug]: Invalid EFI memory map entries: +[ 0.000000] efi: mem45: [Reserved | | | | | | | | | | | | ] range=[0x0000000000000000-0x0000000000000000] (invalid) + +It also detects memory map sizes that would overflow the physical +address, for example phys_addr=0xfffffffffffff000 and +num_pages=0x0200000000000001, and prints: + +[ 0.000000] efi: [Firmware Bug]: Invalid EFI memory map entries: +[ 0.000000] efi: mem45: [Reserved | | | | | | | | | | | | ] range=[phys_addr=0xfffffffffffff000-0x20ffffffffffffffff] (invalid) + +It then removes these entries from the memory map. + +Cc: Matt Fleming +Signed-off-by: Peter Jones +[ardb: refactor for clarity with no functional changes, avoid PAGE_SHIFT] +Signed-off-by: Ard Biesheuvel +--- + arch/x86/platform/efi/efi.c | 70 +++++++++++++++++++++++++++++++++++++++++++++ + include/linux/efi.h | 1 + + 2 files changed, 71 insertions(+) + +diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c +index bf99aa7..0a1550b 100644 +--- a/arch/x86/platform/efi/efi.c ++++ b/arch/x86/platform/efi/efi.c +@@ -210,6 +210,74 @@ int __init efi_memblock_x86_reserve_range(void) + return 0; + } + ++#define OVERFLOW_ADDR_SHIFT (64 - EFI_PAGE_SHIFT) ++#define OVERFLOW_ADDR_MASK (U64_MAX << OVERFLOW_ADDR_SHIFT) ++#define U64_HIGH_BIT (~(U64_MAX >> 1)) ++ ++static bool __init efi_memmap_entry_valid(const efi_memory_desc_t *md, int i) ++{ ++ static __initdata bool once = true; ++ u64 end = (md->num_pages << EFI_PAGE_SHIFT) + md->phys_addr - 1; ++ u64 end_hi = 0; ++ char buf[64]; ++ ++ if (md->num_pages == 0) { ++ end = 0; ++ } else if (md->num_pages > EFI_PAGES_MAX || ++ EFI_PAGES_MAX - md->num_pages < ++ (md->phys_addr >> EFI_PAGE_SHIFT)) { ++ end_hi = (md->num_pages & OVERFLOW_ADDR_MASK) ++ >> OVERFLOW_ADDR_SHIFT; ++ ++ if ((md->phys_addr & U64_HIGH_BIT) && !(end & U64_HIGH_BIT)) ++ end_hi += 1; ++ } else { ++ return true; ++ } ++ ++ if (once) { ++ pr_warn(FW_BUG "Invalid EFI memory map entries:\n"); ++ once = false; ++ } ++ ++ if (end_hi) { ++ pr_warn("mem%02u: %s range=[0x%016llx-0x%llx%016llx] (invalid)\n", ++ i, efi_md_typeattr_format(buf, sizeof(buf), md), ++ md->phys_addr, end_hi, end); ++ } else { ++ pr_warn("mem%02u: %s range=[0x%016llx-0x%016llx] (invalid)\n", ++ i, efi_md_typeattr_format(buf, sizeof(buf), md), ++ md->phys_addr, end); ++ } ++ return false; ++} ++ ++static void __init efi_clean_memmap(void) ++{ ++ efi_memory_desc_t *out = efi.memmap.map; ++ const efi_memory_desc_t *in = out; ++ const efi_memory_desc_t *end = efi.memmap.map_end; ++ int i, n_removal; ++ ++ for (i = n_removal = 0; in < end; i++) { ++ if (efi_memmap_entry_valid(in, i)) { ++ if (out != in) ++ memcpy(out, in, efi.memmap.desc_size); ++ out = (void *)out + efi.memmap.desc_size; ++ } else { ++ n_removal++; ++ } ++ in = (void *)in + efi.memmap.desc_size; ++ } ++ ++ if (n_removal > 0) { ++ u64 size = efi.memmap.nr_map - n_removal; ++ ++ pr_warn("Removing %d invalid memory map entries.\n", n_removal); ++ efi_memmap_install(efi.memmap.phys_map, size); ++ } ++} ++ + void __init efi_print_memmap(void) + { + efi_memory_desc_t *md; +@@ -472,6 +540,8 @@ void __init efi_init(void) + } + } + ++ efi_clean_memmap(); ++ + if (efi_enabled(EFI_DBG)) + efi_print_memmap(); + } +diff --git a/include/linux/efi.h b/include/linux/efi.h +index 4c1b3ea..712a3aa 100644 +--- a/include/linux/efi.h ++++ b/include/linux/efi.h +@@ -103,6 +103,7 @@ typedef struct { + + #define EFI_PAGE_SHIFT 12 + #define EFI_PAGE_SIZE (1UL << EFI_PAGE_SHIFT) ++#define EFI_PAGES_MAX (U64_MAX >> EFI_PAGE_SHIFT) + + typedef struct { + u32 type; +-- +2.9.3 + diff --git a/0002-efi-efi_map_region-traceback-if-we-try-to-map-invali.patch b/0002-efi-efi_map_region-traceback-if-we-try-to-map-invali.patch deleted file mode 100644 index 25b0ad19b..000000000 --- a/0002-efi-efi_map_region-traceback-if-we-try-to-map-invali.patch +++ /dev/null @@ -1,71 +0,0 @@ -From 510cd0c36a3beb0907bdbd31a48b71abdddb44a7 Mon Sep 17 00:00:00 2001 -From: Peter Jones -Date: Wed, 7 Dec 2016 16:20:10 -0500 -Subject: [PATCH 2/4] efi: efi_map_region(): traceback if we try to map invalid - sized regions - -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. -We shouldn't ever try to map these errors, so if we get as far as -efi_map_region(), show a traceback. - -This additionally makes should_map_region() say not to map them, but I -fixed both places in case another caller of efi_map_region() ever arises -in the future. - -Signed-off-by: Peter Jones ---- - arch/x86/platform/efi/efi.c | 4 ++++ - arch/x86/platform/efi/efi_64.c | 19 ++++++++++++++++--- - 2 files changed, 20 insertions(+), 3 deletions(-) - -diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c -index 181c915..bf32454 100644 ---- a/arch/x86/platform/efi/efi.c -+++ b/arch/x86/platform/efi/efi.c -@@ -707,6 +707,10 @@ static bool should_map_region(efi_memory_desc_t *md) - if (IS_ENABLED(CONFIG_X86_32)) - return false; - -+ if (md->num_pages == 0 || -+ md->num_pages >= (((u64)-1LL) >> EFI_PAGE_SHIFT)) -+ return false; -+ - /* - * Map all of RAM so that we can access arguments in the 1:1 - * mapping when making EFI runtime calls. -diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c -index de12d9f..f80de01 100644 ---- a/arch/x86/platform/efi/efi_64.c -+++ b/arch/x86/platform/efi/efi_64.c -@@ -283,11 +283,24 @@ static void __init __map_region(efi_memory_desc_t *md, u64 va) - - void __init efi_map_region(efi_memory_desc_t *md) - { -- unsigned long size = md->num_pages << PAGE_SHIFT; -+ u64 size = md->num_pages << PAGE_SHIFT; - u64 pa = md->phys_addr; - -- if (efi_enabled(EFI_OLD_MEMMAP)) -- return old_map_region(md); -+ /* -+ * hah hah the system firmware is having a good one on us -+ */ -+ if (md->num_pages == 0 || -+ md->num_pages >= (((u64)-1LL) >> EFI_PAGE_SHIFT)) { -+ pr_err("memmap from %p to %p is unreasonable. Not mapping it.\n", -+ (void *)pa, (void *)(pa+size)); -+ WARN_ON(1); -+ return; -+ } -+ -+ if (efi_enabled(EFI_OLD_MEMMAP)) { -+ old_map_region(md); -+ return; -+ } - - /* - * Make sure the 1:1 mappings are present as a catch-all for b0rked --- -2.9.3 - diff --git a/0003-efi-efi_memmap_insert-don-t-insert-a-region-more-tha.patch b/0003-efi-efi_memmap_insert-don-t-insert-a-region-more-tha.patch deleted file mode 100644 index aceeb1d0c..000000000 --- a/0003-efi-efi_memmap_insert-don-t-insert-a-region-more-tha.patch +++ /dev/null @@ -1,114 +0,0 @@ -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 - diff --git a/0004-efi-efi_memmap_insert-don-t-split-regions-with-inval.patch b/0004-efi-efi_memmap_insert-don-t-split-regions-with-inval.patch deleted file mode 100644 index a69c6108c..000000000 --- a/0004-efi-efi_memmap_insert-don-t-split-regions-with-inval.patch +++ /dev/null @@ -1,45 +0,0 @@ -From 1c38760731eefdbd5e9ce288009d6d19afcff004 Mon Sep 17 00:00:00 2001 -From: Peter Jones -Date: Wed, 7 Dec 2016 16:34:20 -0500 -Subject: [PATCH 4/4] efi: efi_memmap_insert(): don't split regions with - invalid sizes. - -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. - -If we're inserting a new memmap and we find a map that is either 0 -pages or all of possible memory (or more!), skip it. When a map exists -at 0 that's 0 pages, the "end" math here winds up making *every* address -within the range, and so it'll try to split that entry, and things go -poorly after that. The same would be true if num_pages were (u64)-1LL -(all bits set) or (u64)-1LL >> EFI_PAGE_SHIFT (i.e. all bits set as a -size in bytes, but then shifted to page size to fill the table in). - -Don't even try to split those entries, they're nonsense. - -Signed-off-by: Peter Jones ---- - drivers/firmware/efi/memmap.c | 7 +++++++ - 1 file changed, 7 insertions(+) - -diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c -index 5b71c717..f8c6870 100644 ---- a/drivers/firmware/efi/memmap.c -+++ b/drivers/firmware/efi/memmap.c -@@ -244,6 +244,13 @@ void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf, - /* copy original EFI memory descriptor */ - memcpy(new, old, old_memmap->desc_size); - md = new; -+ if (md->num_pages == 0 || -+ md->num_pages >= (((u64)-1LL) >> EFI_PAGE_SHIFT)) { -+ pr_warn("%s: Skipping absurd memory map entry for 0x%llx pages at 0x%016llx.\n", -+ __func__, md->num_pages, md->phys_addr); -+ continue; -+ } -+ - start = md->phys_addr; - end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1; - --- -2.9.3 - diff --git a/kernel.spec b/kernel.spec index f4af092f2..72d07643d 100644 --- a/kernel.spec +++ b/kernel.spec @@ -620,10 +620,7 @@ Patch665: netfilter-x_tables-deal-with-bogus-nextoffset-values.patch Patch849: 0001-iio-Use-event-header-from-kernel-tree.patch # Work around thinkpad firmware memory layout issues and efi_mem_reserve() -Patch850: 0001-efi-efi_print_memmap-Call-out-invalid-entries-in-the.patch -Patch851: 0002-efi-efi_map_region-traceback-if-we-try-to-map-invali.patch -Patch852: 0003-efi-efi_memmap_insert-don-t-insert-a-region-more-tha.patch -Patch853: 0004-efi-efi_memmap_insert-don-t-split-regions-with-inval.patch +Patch850: 0001-efi-prune-invalid-memory-map-entries.patch # END OF PATCH DEFINITIONS