Changing devmem_is_allowed so that on an EFI machine, access to physical
address below 1 MB is allowed only to physical pages that are valid in
the EFI memory map. This prevents the possibility of an MCE due to
accessing an invalid physical address.
Signed-off-by: T Makphaibulchoke <[email protected]>
---
arch/x86/mm/init.c | 12 ++++++++++--
include/linux/mm.h | 1 +
kernel/resource.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index ab1f6a9..3ed95c5 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -4,6 +4,7 @@
#include <linux/swap.h>
#include <linux/memblock.h>
#include <linux/bootmem.h> /* for max_low_pfn */
+#include <linux/efi.h> /* for efi_enabled */
#include <asm/cacheflush.h>
#include <asm/e820.h>
@@ -319,8 +320,15 @@ unsigned long __init_refok init_memory_mapping(unsigned long start,
*/
int devmem_is_allowed(unsigned long pagenr)
{
- if (pagenr < 256)
- return 1;
+ if (pagenr < 256) {
+ if (!efi_enabled)
+ return 1;
+ /* For EFI, allow access only to valid physical addresses. */
+ if (page_is_valid(pagenr))
+ return 1;
+ return 0;
+ }
+
if (iomem_is_exclusive(pagenr << PAGE_SHIFT))
return 0;
if (!page_is_ram(pagenr))
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 311be90..fd1bcd4 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -288,6 +288,7 @@ static inline int get_page_unless_zero(struct page *page)
}
extern int page_is_ram(unsigned long pfn);
+extern int page_is_valid(unsigned long pfn);
/* Support for virtually mapped pages */
struct page *vmalloc_to_page(const void *addr);
diff --git a/kernel/resource.c b/kernel/resource.c
index 34d4588..aeb091b 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -367,6 +367,53 @@ int __weak page_is_ram(unsigned long pfn)
return walk_system_ram_range(pfn, 1, NULL, __is_ram) == 1;
}
+static int find_next_system_resource(struct resource *res)
+{
+ resource_size_t start, end;
+ struct resource *p;
+
+ BUG_ON(!res);
+
+ start = res->start;
+ end = res->end;
+ BUG_ON(start >= end);
+
+ read_lock(&resource_lock);
+ for (p = iomem_resource.child; p ; p = p->sibling) {
+ /* system ram is just marked as IORESOURCE_MEM */
+ if (!(p->flags & res->flags))
+ continue;
+ if (p->start > end) {
+ p = NULL;
+ break;
+ }
+ if ((p->end >= start) && (p->start < end))
+ break;
+ }
+ read_unlock(&resource_lock);
+ if (!p)
+ return -1;
+ /* copy data */
+ if (res->start < p->start)
+ res->start = p->start;
+ if (res->end > p->end)
+ res->end = p->end;
+ return 0;
+}
+
+int __weak page_is_valid(unsigned long start_pfn)
+{
+ struct resource res;
+ int ret = 0;
+
+ res.start = (u64) start_pfn << PAGE_SHIFT;
+ res.end = ((u64)(start_pfn + 1) << PAGE_SHIFT) - 1;
+ res.flags = IORESOURCE_MEM;
+ if (find_next_system_resource(&res) >= 0)
+ ret = 1;
+ return ret;
+}
+
void __weak arch_remove_reservations(struct resource *avail)
{
}
--
1.7.1
On 10/02/2012 02:32 PM, T Makphaibulchoke wrote:
> Changing devmem_is_allowed so that on an EFI machine, access to physical
> address below 1 MB is allowed only to physical pages that are valid in
> the EFI memory map. This prevents the possibility of an MCE due to
> accessing an invalid physical address.
What?
That sounds like exactly the opposite of normal /dev/mem behavior... we
allow access to non-memory resources (which really could do anything if
misused), but not memory.
You seem like you're flipping it on its head.
-hpa
On Tue, Oct 02, 2012 at 02:50:09PM -0700, H. Peter Anvin wrote:
> That sounds like exactly the opposite of normal /dev/mem behavior... we
> allow access to non-memory resources (which really could do anything if
> misused), but not memory.
>From arch/x86/mm/init.c:
* On x86, access has to be given to the first megabyte of ram because that area
* contains bios code and data regions used by X and dosemu and similar apps.
Limiting this to just RAM would be safer than it currently is. I'm not
convinced that there's any good reason to allow *any* access down there
for EFI systems, though.
--
Matthew Garrett | [email protected]
On 10/02/2012 09:31 PM, Matthew Garrett wrote:
> On Tue, Oct 02, 2012 at 02:50:09PM -0700, H. Peter Anvin wrote:
>
>> That sounds like exactly the opposite of normal /dev/mem behavior... we
>> allow access to non-memory resources (which really could do anything if
>> misused), but not memory.
>
> From arch/x86/mm/init.c:
>
> * On x86, access has to be given to the first megabyte of ram because that area
> * contains bios code and data regions used by X and dosemu and similar apps.
>
> Limiting this to just RAM would be safer than it currently is. I'm not
> convinced that there's any good reason to allow *any* access down there
> for EFI systems, though.
>
Sorry, fail.
We *always* expose the I/O regions to /dev/mem. That is what /dev/mem
*does*. The above is an exception (which is really obsolete, too: we
should simply disallow access to anything which is treated as system
RAM, which doesn't include the BIOS regions in question; the only reason
we don't is that some versions of X take a checksum of the RAM in the
first megabyte as some kind of idiotic random seed.)
-hpa
On Tue, Oct 02, 2012 at 09:44:16PM -0700, H. Peter Anvin wrote:
> We *always* expose the I/O regions to /dev/mem. That is what /dev/mem
> *does*. The above is an exception (which is really obsolete, too: we
> should simply disallow access to anything which is treated as system
> RAM, which doesn't include the BIOS regions in question; the only reason
> we don't is that some versions of X take a checksum of the RAM in the
> first megabyte as some kind of idiotic random seed.)
Oh, right, got you. In that case I think we potentially need a
finer-grained check on EFI platforms - the EFI memory map is kind enough
to tell us the difference between unusable regions and io regions, and
we could avoid access to the unusable ones.
--
Matthew Garrett | [email protected]
Thank you both for the comments.
Sounds like a better solution is to allow accesses to only I/O regions
presented in the EFI memory map for physical addresses below 1 MB.
Do we need to worry about the X checksum in the first MB on an EFI system?
Thanks,
Mak.
On 10/02/2012 11:15 PM, Matthew Garrett wrote:
> On Tue, Oct 02, 2012 at 09:44:16PM -0700, H. Peter Anvin wrote:
>
>> We *always* expose the I/O regions to /dev/mem. That is what /dev/mem
>> *does*. The above is an exception (which is really obsolete, too: we
>> should simply disallow access to anything which is treated as system
>> RAM, which doesn't include the BIOS regions in question; the only reason
>> we don't is that some versions of X take a checksum of the RAM in the
>> first megabyte as some kind of idiotic random seed.)
>
> Oh, right, got you. In that case I think we potentially need a
> finer-grained check on EFI platforms - the EFI memory map is kind enough
> to tell us the difference between unusable regions and io regions, and
> we could avoid access to the unusable ones.
>
On 10/02/2012 10:15 PM, Matthew Garrett wrote:
> On Tue, Oct 02, 2012 at 09:44:16PM -0700, H. Peter Anvin wrote:
>
>> We *always* expose the I/O regions to /dev/mem. That is what /dev/mem
>> *does*. The above is an exception (which is really obsolete, too: we
>> should simply disallow access to anything which is treated as system
>> RAM, which doesn't include the BIOS regions in question; the only reason
>> we don't is that some versions of X take a checksum of the RAM in the
>> first megabyte as some kind of idiotic random seed.)
>
> Oh, right, got you. In that case I think we potentially need a
> finer-grained check on EFI platforms - the EFI memory map is kind enough
> to tell us the difference between unusable regions and io regions, and
> we could avoid access to the unusable ones.
>
Well, we have the same in BIOS space with "reserved" regions. The
problem is that they are actually I/O regions as far as programs like X,
dmidecode and so on.
-hpa
On Tue, Oct 02, 2012 at 11:13:17PM -0600, Thavatchai Makphaibulchoke wrote:
> Sounds like a better solution is to allow accesses to only I/O regions
> presented in the EFI memory map for physical addresses below 1 MB.
That won't work - unfortunately we do still need the low region to be
available for X because some platforms expect us to use int10 even on
EFI (yes, yes, I know). Do you have a copy of the EFI memory map for a
system that's broken with the current code?
--
Matthew Garrett | [email protected]
On 10/02/2012 10:28 PM, Matthew Garrett wrote:
> On Tue, Oct 02, 2012 at 11:13:17PM -0600, Thavatchai Makphaibulchoke wrote:
>
>> Sounds like a better solution is to allow accesses to only I/O regions
>> presented in the EFI memory map for physical addresses below 1 MB.
>
> That won't work - unfortunately we do still need the low region to be
> available for X because some platforms expect us to use int10 even on
> EFI (yes, yes, I know). Do you have a copy of the EFI memory map for a
> system that's broken with the current code?
>
I honestly think this calls for a quirk, or more likely, no action at
all ("don't do that, then.")
-hpa