[PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[T Makphaibulchoke]

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 <tmac@hp.com>
---
 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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[H. Peter Anvin]

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[H. Peter Anvin]

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[Thavatchai Makphaibulchoke]

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[H. Peter Anvin]

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


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix devmem_is_allowed for below 1MB accesses for an efi machine

[H. Peter Anvin]

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


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>