Work around thinkpad firmware memory layout issues and efi_mem_reserve()

Signed-off-by: Peter Jones <pjones@redhat.com>
This commit is contained in:
Peter Jones 2016-12-08 10:26:16 -05:00
parent 43988ee4c0
commit 27925bfba5
5 changed files with 300 additions and 0 deletions

View File

@ -0,0 +1,61 @@
From 64ec700cb0b75047ce2ac68cca020e75369cb409 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
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 <pjones@redhat.com>
---
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

View File

@ -0,0 +1,71 @@
From 510cd0c36a3beb0907bdbd31a48b71abdddb44a7 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
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 <pjones@redhat.com>
---
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

View File

@ -0,0 +1,114 @@
From f6e5aedc786ca2360a9b2aaa2fc31769a9c182d0 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
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 <pjones@redhat.com>
---
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

View File

@ -0,0 +1,45 @@
From 1c38760731eefdbd5e9ce288009d6d19afcff004 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
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 <pjones@redhat.com>
---
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

View File

@ -616,6 +616,12 @@ Patch665: netfilter-x_tables-deal-with-bogus-nextoffset-values.patch
#ongoing complaint, full discussion delayed until ksummit/plumbers #ongoing complaint, full discussion delayed until ksummit/plumbers
Patch849: 0001-iio-Use-event-header-from-kernel-tree.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
# END OF PATCH DEFINITIONS # END OF PATCH DEFINITIONS
%endif %endif
@ -2168,6 +2174,9 @@ fi
# #
# #
%changelog %changelog
* Thu Dec 08 2016 Peter Jones <pjones@redhat.com>
- Work around thinkpad firmware memory layout issues and efi_mem_reserve()
* Wed Dec 07 2016 Laura Abbott <labbott@fedoraproject.org> - 4.9.0-0.rc8.git2.1 * Wed Dec 07 2016 Laura Abbott <labbott@fedoraproject.org> - 4.9.0-0.rc8.git2.1
- Linux v4.9-rc8-55-gce779d6 - Linux v4.9-rc8-55-gce779d6
- Disable CONFIG_AF_KCM (rhbz 1402489) - Disable CONFIG_AF_KCM (rhbz 1402489)