linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: [patch 2/2] /dev/mem validate mmap requests
@ 2005-12-14  0:25 Luck, Tony
  2005-12-14 15:48 ` Bjorn Helgaas
  0 siblings, 1 reply; 6+ messages in thread
From: Luck, Tony @ 2005-12-14  0:25 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-kernel, Andrew Morton; +Cc: linux-ia64

> Tony, can you ack/nak this please?  It touches both ia64 and generic
> code.

So if someone tries to mmap a range that spans across more than
one EFI memory descriptor, the size will get trimmed back to an
EFI memory boundary.  Isn't that a problem since 1<<EFI_PAGE_SHIFT
is less than the default ia64 Linux page size?

I think you may need a more complex checker that does aggregation
of adjacent efi memory descriptors with the same attributes.

-Tony

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] /dev/mem validate mmap requests
  2005-12-14  0:25 [patch 2/2] /dev/mem validate mmap requests Luck, Tony
@ 2005-12-14 15:48 ` Bjorn Helgaas
  2005-12-15 22:06   ` Luck, Tony
  0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2005-12-14 15:48 UTC (permalink / raw)
  To: Luck, Tony; +Cc: linux-kernel, Andrew Morton, linux-ia64

On Tuesday 13 December 2005 5:25 pm, Luck, Tony wrote:
> > Tony, can you ack/nak this please?  It touches both ia64 and generic
> > code.
> 
> So if someone tries to mmap a range that spans across more than
> one EFI memory descriptor, the size will get trimmed back to an
> EFI memory boundary.  Isn't that a problem since 1<<EFI_PAGE_SHIFT
> is less than the default ia64 Linux page size?

The EFI page size is smaller than the Linux page size, but firmware
typically coalesces adjacent ranges with the same attributes.

> I think you may need a more complex checker that does aggregation
> of adjacent efi memory descriptors with the same attributes.

We could, but I don't think it's worth it at this point.  We've
been using the same simple-minded scheme for validating /dev/mem
read & write requests for quite a while with no problems, and I
don't want to over-engineer this.

If hot-plug memory is ever finished, the checker may have to
be extended to comprehend regions described by ACPI as well as
those described in the boot-time EFI memory map.  I think that
would be the right time to make it smarter about spanning
descriptors.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] /dev/mem validate mmap requests
  2005-12-14 15:48 ` Bjorn Helgaas
@ 2005-12-15 22:06   ` Luck, Tony
  2005-12-20 20:53     ` Bjorn Helgaas
  0 siblings, 1 reply; 6+ messages in thread
From: Luck, Tony @ 2005-12-15 22:06 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-kernel, Andrew Morton, linux-ia64

On Wed, Dec 14, 2005 at 08:48:56AM -0700, Bjorn Helgaas wrote:
> > I think you may need a more complex checker that does aggregation
> > of adjacent efi memory descriptors with the same attributes.
> 
> We could, but I don't think it's worth it at this point.  We've
> been using the same simple-minded scheme for validating /dev/mem
> read & write requests for quite a while with no problems, and I
> don't want to over-engineer this.

Here's the spot in the efi memory map that worried me:

4000000 - 4a54000 type=2 attr=b
4a54000 - ff80000 type=7 attr=b

The first of these two blocks is the space that Elilo allocated
to hold the kernel, the second is the remainder of memory in
the block of same-attribute memory.  In this case the boundary
is on a 16K boundary, so all is well.  But is this guaranteed
to work?

Booting a kernel with a 64K pagesize, I see:
4000000 - 4aa0000 type=2 attr=b
4aa0000 - ff80000 type=7 attr=b

So perhaps this is ok ... maybe the size of the kernel
will always come out as a whole number of pages?

But looking further down the map (on the 64K boot) I see:

180000000 - 1fedfa000 type=7 attr=b
1fedfa000 - 1fee00000 type=2 attr=b
1fee00000 - 1fee01000 type=7 attr=b
1fee01000 - 1fef56000 type=2 attr=b
1fef56000 - 1fefae000 type=1 attr=b
1fefae000 - 1fefb8000 type=2 attr=b
1fefb8000 - 1feffe000 type=1 attr=b
1feffe000 - 1ff454000 type=7 attr=b
1ff454000 - 1ff801000 type=4 attr=b
1ff801000 - 1ff8cc000 type=7 attr=b
1ff8cc000 - 1ff904000 type=4 attr=b
1ff904000 - 1ff908000 type=7 attr=b
1ff908000 - 1ff90a000 type=4 attr=b

Now these EFI areas all have the same attributes, but the boundaries
are not neatly aligned to kernel page size.

-Tony

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] /dev/mem validate mmap requests
  2005-12-15 22:06   ` Luck, Tony
@ 2005-12-20 20:53     ` Bjorn Helgaas
  2005-12-22 21:49       ` Bjorn Helgaas
  0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2005-12-20 20:53 UTC (permalink / raw)
  To: Luck, Tony; +Cc: linux-kernel, Andrew Morton, linux-ia64

On Thursday 15 December 2005 3:06 pm, Luck, Tony wrote:
> On Wed, Dec 14, 2005 at 08:48:56AM -0700, Bjorn Helgaas wrote:
> > > I think you may need a more complex checker that does aggregation
> > > of adjacent efi memory descriptors with the same attributes.

OK, I'm convinced :-)  How about the below?  It looks a bit
more complicated because I moved a few things around and fixed
up the /dev/mem read/write path check as well as the mmap check.

One of these days I'll get around to consolidating more of the
ia64/x86 EFI code.

If this looks OK to you, I'll ask Andrew to replace his
dev-mem-validate-mmap-requests.patch with this.

Index: work6/arch/ia64/kernel/efi.c
===================================================================
--- work6.orig/arch/ia64/kernel/efi.c	2005-12-19 12:50:52.000000000 -0700
+++ work6/arch/ia64/kernel/efi.c	2005-12-20 10:32:49.000000000 -0700
@@ -247,6 +247,32 @@
 
 static kern_memdesc_t *kern_memmap;
 
+#define efi_md_size(md)	(md->num_pages << EFI_PAGE_SHIFT)
+
+static inline u64
+kmd_end(kern_memdesc_t *kmd)
+{
+	return (kmd->start + (kmd->num_pages << EFI_PAGE_SHIFT));
+}
+
+static inline u64
+efi_md_end(efi_memory_desc_t *md)
+{
+	return (md->phys_addr + efi_md_size(md));
+}
+
+static inline int
+efi_wb(efi_memory_desc_t *md)
+{
+	return (md->attribute & EFI_MEMORY_WB);
+}
+
+static inline int
+efi_uc(efi_memory_desc_t *md)
+{
+	return (md->attribute & EFI_MEMORY_UC);
+}
+
 static void
 walk (efi_freemem_callback_t callback, void *arg, u64 attr)
 {
@@ -595,8 +621,8 @@
 	return 0;
 }
 
-u32
-efi_mem_type (unsigned long phys_addr)
+static efi_memory_desc_t *
+efi_memory_descriptor (unsigned long phys_addr)
 {
 	void *efi_map_start, *efi_map_end, *p;
 	efi_memory_desc_t *md;
@@ -610,13 +636,13 @@
 		md = p;
 
 		if (phys_addr - md->phys_addr < (md->num_pages << EFI_PAGE_SHIFT))
-			 return md->type;
+			 return md;
 	}
 	return 0;
 }
 
-u64
-efi_mem_attributes (unsigned long phys_addr)
+static int
+efi_memmap_has_mmio (void)
 {
 	void *efi_map_start, *efi_map_end, *p;
 	efi_memory_desc_t *md;
@@ -629,36 +655,98 @@
 	for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
 		md = p;
 
-		if (phys_addr - md->phys_addr < (md->num_pages << EFI_PAGE_SHIFT))
-			return md->attribute;
+		if (md->type == EFI_MEMORY_MAPPED_IO)
+			return 1;
 	}
 	return 0;
 }
+
+u32
+efi_mem_type (unsigned long phys_addr)
+{
+	efi_memory_desc_t *md = efi_memory_descriptor(phys_addr);
+
+	if (md)
+		return md->type;
+	return 0;
+}
+
+u64
+efi_mem_attributes (unsigned long phys_addr)
+{
+	efi_memory_desc_t *md = efi_memory_descriptor(phys_addr);
+
+	if (md)
+		return md->attribute;
+	return 0;
+}
 EXPORT_SYMBOL(efi_mem_attributes);
 
+/*
+ * Determines whether the memory at phys_addr supports the desired
+ * attribute (WB, UC, etc).  If this returns 1, the caller can safely
+ * access *size bytes at phys_addr with the specified attribute.
+ */
+static int
+efi_mem_attribute_range (unsigned long phys_addr, unsigned long *size, u64 attr)
+{
+	efi_memory_desc_t *md = efi_memory_descriptor(phys_addr);
+	unsigned long md_end;
+
+	if (!md || (md->attribute & attr) != attr)
+		return 0;
+
+	do {
+		md_end = efi_md_end(md);
+		if (phys_addr + *size <= md_end)
+			return 1;
+
+		md = efi_memory_descriptor(md_end);
+		if (!md || (md->attribute & attr) != attr) {
+			*size = md_end - phys_addr;
+			return 1;
+		}
+	} while (md);
+	return 0;
+}
+
+/*
+ * For /dev/mem, we only allow read & write system calls to access
+ * write-back memory, because read & write don't allow the user to
+ * control access size.
+ */
 int
 valid_phys_addr_range (unsigned long phys_addr, unsigned long *size)
 {
-	void *efi_map_start, *efi_map_end, *p;
-	efi_memory_desc_t *md;
-	u64 efi_desc_size;
+	return efi_mem_attribute_range(phys_addr, size, EFI_MEMORY_WB);
+}
 
-	efi_map_start = __va(ia64_boot_param->efi_memmap);
-	efi_map_end   = efi_map_start + ia64_boot_param->efi_memmap_size;
-	efi_desc_size = ia64_boot_param->efi_memdesc_size;
+/*
+ * We allow mmap of anything in the EFI memory map that supports
+ * either write-back or uncacheable access.  For uncacheable regions,
+ * the supported access sizes are system-dependent, and the user is
+ * responsible for using the correct size.
+ *
+ * Note that this doesn't currently allow access to hot-added memory,
+ * because that doesn't appear in the boot-time EFI memory map.
+ */
+int
+valid_mmap_phys_addr_range (unsigned long phys_addr, unsigned long *size)
+{
+	if (efi_mem_attribute_range(phys_addr, size, EFI_MEMORY_WB))
+		return 1;
 
-	for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
-		md = p;
+	if (efi_mem_attribute_range(phys_addr, size, EFI_MEMORY_UC))
+		return 1;
 
-		if (phys_addr - md->phys_addr < (md->num_pages << EFI_PAGE_SHIFT)) {
-			if (!(md->attribute & EFI_MEMORY_WB))
-				return 0;
+	/*
+	 * Some firmware doesn't report MMIO regions in the EFI memory map.
+	 * The Intel BigSur (a.k.a. HP i2000) has this problem.  In this
+	 * case, we can't use the EFI memory map to validate mmap requests.
+	 */
+	if (!efi_memmap_has_mmio())
+		return 1;
 
-			if (*size > md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - phys_addr)
-				*size = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - phys_addr;
-			return 1;
-		}
-	}
 	return 0;
 }
 
@@ -707,32 +795,6 @@
 	return 0;
 }
 
-#define efi_md_size(md)	(md->num_pages << EFI_PAGE_SHIFT)
-
-static inline u64
-kmd_end(kern_memdesc_t *kmd)
-{
-	return (kmd->start + (kmd->num_pages << EFI_PAGE_SHIFT));
-}
-
-static inline u64
-efi_md_end(efi_memory_desc_t *md)
-{
-	return (md->phys_addr + efi_md_size(md));
-}
-
-static inline int
-efi_wb(efi_memory_desc_t *md)
-{
-	return (md->attribute & EFI_MEMORY_WB);
-}
-
-static inline int
-efi_uc(efi_memory_desc_t *md)
-{
-	return (md->attribute & EFI_MEMORY_UC);
-}
-
 /*
  * Look for the first granule aligned memory descriptor memory
  * that is big enough to hold EFI memory map. Make sure this
Index: work6/drivers/char/mem.c
===================================================================
--- work6.orig/drivers/char/mem.c	2005-12-19 12:50:52.000000000 -0700
+++ work6/drivers/char/mem.c	2005-12-20 10:32:49.000000000 -0700
@@ -101,6 +101,11 @@
 
 	return 1;
 }
+
+static inline int valid_mmap_phys_addr_range(unsigned long addr, size_t *size)
+{
+	return 1;
+}
 #endif
 
 /*
@@ -244,15 +249,20 @@
 
 static int mmap_mem(struct file * file, struct vm_area_struct * vma)
 {
+	size_t size = vma->vm_end - vma->vm_start;
+
+	if (!valid_mmap_phys_addr_range(vma->vm_pgoff << PAGE_SHIFT, &size))
+		return -EINVAL;
+
 	vma->vm_page_prot = phys_mem_access_prot(file, vma->vm_pgoff,
-						 vma->vm_end - vma->vm_start,
+						 size,
 						 vma->vm_page_prot);
 
 	/* Remap-pfn-range will mark the range VM_IO and VM_RESERVED */
 	if (remap_pfn_range(vma,
 			    vma->vm_start,
 			    vma->vm_pgoff,
-			    vma->vm_end-vma->vm_start,
+			    size,
 			    vma->vm_page_prot))
 		return -EAGAIN;
 	return 0;
Index: work6/include/asm-ia64/io.h
===================================================================
--- work6.orig/include/asm-ia64/io.h	2005-12-19 12:50:52.000000000 -0700
+++ work6/include/asm-ia64/io.h	2005-12-20 10:32:49.000000000 -0700
@@ -89,6 +89,7 @@
 
 #define ARCH_HAS_VALID_PHYS_ADDR_RANGE
 extern int valid_phys_addr_range (unsigned long addr, size_t *count); /* efi.c */
+extern int valid_mmap_phys_addr_range (unsigned long addr, size_t *count);
 
 /*
  * The following two macros are deprecated and scheduled for removal.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] /dev/mem validate mmap requests
  2005-12-20 20:53     ` Bjorn Helgaas
@ 2005-12-22 21:49       ` Bjorn Helgaas
  0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2005-12-22 21:49 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Luck, Tony, linux-kernel, linux-ia64

Andrew, could you replace dev-mem-validate-mmap-requests.patch
with the patch below?  This addresses Tony's request that
the checking handle regions that span EFI memory descriptors.




Add a hook so architectures can validate /dev/mem mmap requests.

This is analogous to validation we already perform in the read/write
paths.

The identity mapping scheme used on ia64 requires that each 16MB or
64MB granule be accessed with exactly one attribute (write-back or
uncacheable).  This avoids "attribute aliasing", which can cause a
machine check.

Sample problem scenario:
  - Machine supports VGA, so it has uncacheable (UC) MMIO at 640K-768K
  - efi_memmap_init() discards any write-back (WB) memory in the first granule
  - Application (e.g., "hwinfo") mmaps /dev/mem, offset 0
  - hwinfo receives UC mapping (the default, since memmap says "no WB here")
  - Machine check abort (on chipsets that don't support UC access to WB
    memory, e.g., sx1000)

In the scenario above, the only choices are
  - Use WB for hwinfo mmap.  Can't do this because it causes attribute
    aliasing with the UC mapping for the VGA MMIO space.
  - Use UC for hwinfo mmap.  Can't do this because the chipset may not
    support UC for that region.
  - Disallow the hwinfo mmap with -EINVAL.  That's what this patch does.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

 arch/ia64/kernel/efi.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/char/mem.c     |   14 ++++++++++++--
 include/asm-ia64/io.h  |    1 +
 3 files changed, 57 insertions(+), 2 deletions(-)

Index: work6/arch/ia64/kernel/efi.c
===================================================================
--- work6.orig/arch/ia64/kernel/efi.c	2005-12-19 12:50:52.000000000 -0700
+++ work6/arch/ia64/kernel/efi.c	2005-12-20 10:32:49.000000000 -0700
@@ -247,6 +247,32 @@
 
 static kern_memdesc_t *kern_memmap;
 
+#define efi_md_size(md)	(md->num_pages << EFI_PAGE_SHIFT)
+
+static inline u64
+kmd_end(kern_memdesc_t *kmd)
+{
+	return (kmd->start + (kmd->num_pages << EFI_PAGE_SHIFT));
+}
+
+static inline u64
+efi_md_end(efi_memory_desc_t *md)
+{
+	return (md->phys_addr + efi_md_size(md));
+}
+
+static inline int
+efi_wb(efi_memory_desc_t *md)
+{
+	return (md->attribute & EFI_MEMORY_WB);
+}
+
+static inline int
+efi_uc(efi_memory_desc_t *md)
+{
+	return (md->attribute & EFI_MEMORY_UC);
+}
+
 static void
 walk (efi_freemem_callback_t callback, void *arg, u64 attr)
 {
@@ -595,8 +621,8 @@
 	return 0;
 }
 
-u32
-efi_mem_type (unsigned long phys_addr)
+static efi_memory_desc_t *
+efi_memory_descriptor (unsigned long phys_addr)
 {
 	void *efi_map_start, *efi_map_end, *p;
 	efi_memory_desc_t *md;
@@ -610,13 +636,13 @@
 		md = p;
 
 		if (phys_addr - md->phys_addr < (md->num_pages << EFI_PAGE_SHIFT))
-			 return md->type;
+			 return md;
 	}
 	return 0;
 }
 
-u64
-efi_mem_attributes (unsigned long phys_addr)
+static int
+efi_memmap_has_mmio (void)
 {
 	void *efi_map_start, *efi_map_end, *p;
 	efi_memory_desc_t *md;
@@ -629,36 +655,98 @@
 	for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
 		md = p;
 
-		if (phys_addr - md->phys_addr < (md->num_pages << EFI_PAGE_SHIFT))
-			return md->attribute;
+		if (md->type == EFI_MEMORY_MAPPED_IO)
+			return 1;
 	}
 	return 0;
 }
+
+u32
+efi_mem_type (unsigned long phys_addr)
+{
+	efi_memory_desc_t *md = efi_memory_descriptor(phys_addr);
+
+	if (md)
+		return md->type;
+	return 0;
+}
+
+u64
+efi_mem_attributes (unsigned long phys_addr)
+{
+	efi_memory_desc_t *md = efi_memory_descriptor(phys_addr);
+
+	if (md)
+		return md->attribute;
+	return 0;
+}
 EXPORT_SYMBOL(efi_mem_attributes);
 
+/*
+ * Determines whether the memory at phys_addr supports the desired
+ * attribute (WB, UC, etc).  If this returns 1, the caller can safely
+ * access *size bytes at phys_addr with the specified attribute.
+ */
+static int
+efi_mem_attribute_range (unsigned long phys_addr, unsigned long *size, u64 attr)
+{
+	efi_memory_desc_t *md = efi_memory_descriptor(phys_addr);
+	unsigned long md_end;
+
+	if (!md || (md->attribute & attr) != attr)
+		return 0;
+
+	do {
+		md_end = efi_md_end(md);
+		if (phys_addr + *size <= md_end)
+			return 1;
+
+		md = efi_memory_descriptor(md_end);
+		if (!md || (md->attribute & attr) != attr) {
+			*size = md_end - phys_addr;
+			return 1;
+		}
+	} while (md);
+	return 0;
+}
+
+/*
+ * For /dev/mem, we only allow read & write system calls to access
+ * write-back memory, because read & write don't allow the user to
+ * control access size.
+ */
 int
 valid_phys_addr_range (unsigned long phys_addr, unsigned long *size)
 {
-	void *efi_map_start, *efi_map_end, *p;
-	efi_memory_desc_t *md;
-	u64 efi_desc_size;
+	return efi_mem_attribute_range(phys_addr, size, EFI_MEMORY_WB);
+}
 
-	efi_map_start = __va(ia64_boot_param->efi_memmap);
-	efi_map_end   = efi_map_start + ia64_boot_param->efi_memmap_size;
-	efi_desc_size = ia64_boot_param->efi_memdesc_size;
+/*
+ * We allow mmap of anything in the EFI memory map that supports
+ * either write-back or uncacheable access.  For uncacheable regions,
+ * the supported access sizes are system-dependent, and the user is
+ * responsible for using the correct size.
+ *
+ * Note that this doesn't currently allow access to hot-added memory,
+ * because that doesn't appear in the boot-time EFI memory map.
+ */
+int
+valid_mmap_phys_addr_range (unsigned long phys_addr, unsigned long *size)
+{
+	if (efi_mem_attribute_range(phys_addr, size, EFI_MEMORY_WB))
+		return 1;
 
-	for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
-		md = p;
+	if (efi_mem_attribute_range(phys_addr, size, EFI_MEMORY_UC))
+		return 1;
 
-		if (phys_addr - md->phys_addr < (md->num_pages << EFI_PAGE_SHIFT)) {
-			if (!(md->attribute & EFI_MEMORY_WB))
-				return 0;
+	/*
+	 * Some firmware doesn't report MMIO regions in the EFI memory map.
+	 * The Intel BigSur (a.k.a. HP i2000) has this problem.  In this
+	 * case, we can't use the EFI memory map to validate mmap requests.
+	 */
+	if (!efi_memmap_has_mmio())
+		return 1;
 
-			if (*size > md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - phys_addr)
-				*size = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - phys_addr;
-			return 1;
-		}
-	}
 	return 0;
 }
 
@@ -707,32 +795,6 @@
 	return 0;
 }
 
-#define efi_md_size(md)	(md->num_pages << EFI_PAGE_SHIFT)
-
-static inline u64
-kmd_end(kern_memdesc_t *kmd)
-{
-	return (kmd->start + (kmd->num_pages << EFI_PAGE_SHIFT));
-}
-
-static inline u64
-efi_md_end(efi_memory_desc_t *md)
-{
-	return (md->phys_addr + efi_md_size(md));
-}
-
-static inline int
-efi_wb(efi_memory_desc_t *md)
-{
-	return (md->attribute & EFI_MEMORY_WB);
-}
-
-static inline int
-efi_uc(efi_memory_desc_t *md)
-{
-	return (md->attribute & EFI_MEMORY_UC);
-}
-
 /*
  * Look for the first granule aligned memory descriptor memory
  * that is big enough to hold EFI memory map. Make sure this
Index: work6/drivers/char/mem.c
===================================================================
--- work6.orig/drivers/char/mem.c	2005-12-19 12:50:52.000000000 -0700
+++ work6/drivers/char/mem.c	2005-12-20 10:32:49.000000000 -0700
@@ -101,6 +101,11 @@
 
 	return 1;
 }
+
+static inline int valid_mmap_phys_addr_range(unsigned long addr, size_t *size)
+{
+	return 1;
+}
 #endif
 
 /*
@@ -244,15 +249,20 @@
 
 static int mmap_mem(struct file * file, struct vm_area_struct * vma)
 {
+	size_t size = vma->vm_end - vma->vm_start;
+
+	if (!valid_mmap_phys_addr_range(vma->vm_pgoff << PAGE_SHIFT, &size))
+		return -EINVAL;
+
 	vma->vm_page_prot = phys_mem_access_prot(file, vma->vm_pgoff,
-						 vma->vm_end - vma->vm_start,
+						 size,
 						 vma->vm_page_prot);
 
 	/* Remap-pfn-range will mark the range VM_IO and VM_RESERVED */
 	if (remap_pfn_range(vma,
 			    vma->vm_start,
 			    vma->vm_pgoff,
-			    vma->vm_end-vma->vm_start,
+			    size,
 			    vma->vm_page_prot))
 		return -EAGAIN;
 	return 0;
Index: work6/include/asm-ia64/io.h
===================================================================
--- work6.orig/include/asm-ia64/io.h	2005-12-19 12:50:52.000000000 -0700
+++ work6/include/asm-ia64/io.h	2005-12-20 10:32:49.000000000 -0700
@@ -89,6 +89,7 @@
 
 #define ARCH_HAS_VALID_PHYS_ADDR_RANGE
 extern int valid_phys_addr_range (unsigned long addr, size_t *count); /* efi.c */
+extern int valid_mmap_phys_addr_range (unsigned long addr, size_t *count);
 
 /*
  * The following two macros are deprecated and scheduled for removal.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 2/2] /dev/mem validate mmap requests
@ 2005-12-13 23:56 Bjorn Helgaas
  0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2005-12-13 23:56 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton, Tony Luck; +Cc: linux-ia64

Add a hook so architectures can validate /dev/mem mmap requests.

This is analogous to validation we already perform in the read/write
paths.

The identity mapping scheme used on ia64 requires that each 16MB or
64MB granule be accessed with exactly one attribute (write-back or
uncacheable).  This avoids "attribute aliasing", which can cause a
machine check.

Sample problem scenario:
  - Machine supports VGA, so it has uncacheable (UC) MMIO at 640K-768K
  - efi_memmap_init() discards any write-back (WB) memory in the first granule
  - Application (e.g., "hwinfo") mmaps /dev/mem, offset 0
  - hwinfo receives UC mapping (the default, since memmap says "no WB here")
  - Machine check abort (on chipsets that don't support UC access to WB
    memory, e.g., sx1000)

In the scenario above, the only choices are
  - Use WB for hwinfo mmap.  Can't do this because it causes attribute
    aliasing with the UC mapping for the VGA MMIO space.
  - Use UC for hwinfo mmap.  Can't do this because the chipset may not
    support UC for that region.
  - Disallow the hwinfo mmap with -EINVAL.  That's what this patch does.

Tony, can you ack/nak this please?  It touches both ia64 and generic
code.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

 arch/ia64/kernel/efi.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/char/mem.c     |   14 ++++++++++++--
 include/asm-ia64/io.h  |    1 +
 3 files changed, 57 insertions(+), 2 deletions(-)

Index: work5/arch/ia64/kernel/efi.c
===================================================================
--- work5.orig/arch/ia64/kernel/efi.c	2005-12-09 16:17:50.000000000 -0700
+++ work5/arch/ia64/kernel/efi.c	2005-12-09 16:28:58.000000000 -0700
@@ -636,6 +636,10 @@
 }
 EXPORT_SYMBOL(efi_mem_attributes);
 
+/*
+ * We only allow /dev/mem read & write system calls to write-back memory,
+ * because read & write don't allow the user to control access size.
+ */
 int
 valid_phys_addr_range (unsigned long phys_addr, unsigned long *size)
 {
@@ -662,6 +666,46 @@
 	return 0;
 }
 
+/*
+ * Anything in the EFI memory map can be accessed via /dev/mem mmap.
+ * This may have to be extended eventually for memory hot-plug.
+ */
+int
+valid_mmap_phys_addr_range (unsigned long phys_addr, unsigned long *size)
+{
+	void *efi_map_start, *efi_map_end, *p;
+	efi_memory_desc_t *md;
+	u64 efi_desc_size;
+	int mmio_found = 0;
+
+	efi_map_start = __va(ia64_boot_param->efi_memmap);
+	efi_map_end   = efi_map_start + ia64_boot_param->efi_memmap_size;
+	efi_desc_size = ia64_boot_param->efi_memdesc_size;
+
+	for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
+		md = p;
+
+		if (md->type == EFI_MEMORY_MAPPED_IO)
+			mmio_found = 1;
+
+		if (phys_addr - md->phys_addr < (md->num_pages << EFI_PAGE_SHIFT)) {
+			if (*size > md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - phys_addr)
+				*size = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - phys_addr;
+			return 1;
+		}
+	}
+
+	/*
+	 * Some firmware doesn't report MMIO regions in the EFI memory map.
+	 * The Intel BigSur (a.k.a. HP i2000) has this problem.  In this
+	 * case, we can't rely on the map to validate mmap requests.
+	 */
+	if (!mmio_found)
+		return 1;
+
+	return 0;
+}
+
 int __init
 efi_uart_console_only(void)
 {
Index: work5/drivers/char/mem.c
===================================================================
--- work5.orig/drivers/char/mem.c	2005-12-09 16:24:07.000000000 -0700
+++ work5/drivers/char/mem.c	2005-12-09 16:33:10.000000000 -0700
@@ -101,6 +101,11 @@
 
 	return 1;
 }
+
+static inline int valid_mmap_phys_addr_range(unsigned long addr, size_t *size)
+{
+	return 1;
+}
 #endif
 
 /*
@@ -244,15 +249,20 @@
 
 static int mmap_mem(struct file * file, struct vm_area_struct * vma)
 {
+	size_t size = vma->vm_end - vma->vm_start;
+
+	if (!valid_mmap_phys_addr_range(vma->vm_pgoff << PAGE_SHIFT, &size))
+		return -EINVAL;
+
 	vma->vm_page_prot = phys_mem_access_prot(file, vma->vm_pgoff,
-						 vma->vm_end - vma->vm_start,
+						 size,
 						 vma->vm_page_prot);
 
 	/* Remap-pfn-range will mark the range VM_IO and VM_RESERVED */
 	if (remap_pfn_range(vma,
 			    vma->vm_start,
 			    vma->vm_pgoff,
-			    vma->vm_end-vma->vm_start,
+			    size,
 			    vma->vm_page_prot))
 		return -EAGAIN;
 	return 0;
Index: work5/include/asm-ia64/io.h
===================================================================
--- work5.orig/include/asm-ia64/io.h	2005-12-09 16:17:50.000000000 -0700
+++ work5/include/asm-ia64/io.h	2005-12-09 16:28:58.000000000 -0700
@@ -89,6 +89,7 @@
 
 #define ARCH_HAS_VALID_PHYS_ADDR_RANGE
 extern int valid_phys_addr_range (unsigned long addr, size_t *count); /* efi.c */
+extern int valid_mmap_phys_addr_range (unsigned long addr, size_t *count);
 
 /*
  * The following two macros are deprecated and scheduled for removal.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2005-12-22 21:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-12-14  0:25 [patch 2/2] /dev/mem validate mmap requests Luck, Tony
2005-12-14 15:48 ` Bjorn Helgaas
2005-12-15 22:06   ` Luck, Tony
2005-12-20 20:53     ` Bjorn Helgaas
2005-12-22 21:49       ` Bjorn Helgaas
  -- strict thread matches above, loose matches on Subject: below --
2005-12-13 23:56 Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).