From 27925bfba57562738f34a1a1394721c76663977e Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 8 Dec 2016 10:26:16 -0500 Subject: [PATCH] Work around thinkpad firmware memory layout issues and efi_mem_reserve() Signed-off-by: Peter Jones --- ...mmap-Call-out-invalid-entries-in-the.patch | 61 ++++++++++ ...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 | 9 ++ 5 files changed, 300 insertions(+) create mode 100644 0001-efi-efi_print_memmap-Call-out-invalid-entries-in-the.patch create mode 100644 0002-efi-efi_map_region-traceback-if-we-try-to-map-invali.patch create mode 100644 0003-efi-efi_memmap_insert-don-t-insert-a-region-more-tha.patch create 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 new file mode 100644 index 000000000..037a60c33 --- /dev/null +++ b/0001-efi-efi_print_memmap-Call-out-invalid-entries-in-the.patch @@ -0,0 +1,61 @@ +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/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 new file mode 100644 index 000000000..25b0ad19b --- /dev/null +++ b/0002-efi-efi_map_region-traceback-if-we-try-to-map-invali.patch @@ -0,0 +1,71 @@ +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 new file mode 100644 index 000000000..aceeb1d0c --- /dev/null +++ b/0003-efi-efi_memmap_insert-don-t-insert-a-region-more-tha.patch @@ -0,0 +1,114 @@ +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 new file mode 100644 index 000000000..a69c6108c --- /dev/null +++ b/0004-efi-efi_memmap_insert-don-t-split-regions-with-inval.patch @@ -0,0 +1,45 @@ +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 2dd4b01a6..421c5981b 100644 --- a/kernel.spec +++ b/kernel.spec @@ -616,6 +616,12 @@ Patch665: netfilter-x_tables-deal-with-bogus-nextoffset-values.patch #ongoing complaint, full discussion delayed until ksummit/plumbers 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 + # END OF PATCH DEFINITIONS %endif @@ -2168,6 +2174,9 @@ fi # # %changelog +* Thu Dec 08 2016 Peter Jones +- Work around thinkpad firmware memory layout issues and efi_mem_reserve() + * Wed Dec 07 2016 Laura Abbott - 4.9.0-0.rc8.git2.1 - Linux v4.9-rc8-55-gce779d6 - Disable CONFIG_AF_KCM (rhbz 1402489)