rpms/kernel
3
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Update UEFI memory map cleaning

Browse Source 
Signed-off-by: Peter Jones <pjones@redhat.com>
This commit is contained in:
Peter Jones 2016-12-13 11:12:18 -05:00
parent 1a59ad58a8
commit 71ec70ab2a
6 changed files with 142 additions and 295 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
0001-efi-efi_print_memmap-Call-out-invalid-entries-in-the.patch
View File

@ -1,61 +0,0 @@
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

141
0001-efi-prune-invalid-memory-map-entries.patch Normal file
View File

@ -0,0 +1,141 @@
From c7c7030a020405d5826c03839e38986e0f78f2ea Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
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 <matt@codeblueprint.co.uk>
Signed-off-by: Peter Jones <pjones@redhat.com>
[ardb: refactor for clarity with no functional changes, avoid PAGE_SHIFT]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
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

71
0002-efi-efi_map_region-traceback-if-we-try-to-map-invali.patch
View File

@ -1,71 +0,0 @@
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

114
0003-efi-efi_memmap_insert-don-t-insert-a-region-more-tha.patch
View File

@ -1,114 +0,0 @@
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

45
0004-efi-efi_memmap_insert-don-t-split-regions-with-inval.patch
View File

@ -1,45 +0,0 @@
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

5
kernel.spec
View File

@ -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