[patch 4/5] ia64: fail mmaps that span areas with incompatible attributes

[patch 4/5] ia64: fail mmaps that span areas with incompatible attributes

[Bjorn Helgaas]

Example memory map (from HP sx1000 with VGA enabled):
    0x00000 - 0x9FFFF supports only WB (cacheable) access
    0xA0000 - 0xBFFFF supports only UC (uncacheable) access
    0xC0000 - 0xFFFFF supports only WB (cacheable) access

Some versions of X map the entire 0x00000-0xFFFFF area at once.  With the
example above, this mmap must fail because there's no memory attribute that's
safe for the entire area.

Prior to this patch, we performed the mmap with a UC mapping.  When X
accessed the WB memory at 0xC0000, it caused an MCA.  The crash can happen
when mapping 0xC0000 from either /dev/mem or a /sys/.../legacy_mem file.

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

Index: work-mm10/arch/ia64/kernel/efi.c
=================================--- work-mm10.orig/arch/ia64/kernel/efi.c	2007-03-20 21:07:28.000000000 -0700
+++ work-mm10/arch/ia64/kernel/efi.c	2007-03-20 21:13:05.000000000 -0700
@@ -663,6 +663,29 @@
 	return NULL;
 }
 
+static int
+efi_memmap_intersects (unsigned long phys_addr, unsigned long size)
+{
+	void *efi_map_start, *efi_map_end, *p;
+	efi_memory_desc_t *md;
+	u64 efi_desc_size;
+	unsigned long end;
+
+	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;
+
+	end = phys_addr + size;
+
+	for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
+		md = p;
+
+		if (md->phys_addr < end && efi_md_end(md) > phys_addr)
+			return 1;
+	}
+	return 0;
+}
+
 u32
 efi_mem_type (unsigned long phys_addr)
 {
@@ -769,11 +792,28 @@
 int
 valid_mmap_phys_addr_range (unsigned long pfn, unsigned long size)
 {
+	unsigned long phys_addr = pfn << PAGE_SHIFT;
+	u64 attr;
+
+	attr = efi_mem_attribute(phys_addr, size);
+
 	/*
-	 * MMIO regions are often missing from the EFI memory map.
-	 * We must allow mmap of them for programs like X, so we
-	 * currently can't do any useful validation.
+	 * /dev/mem mmap uses normal user pages, so we don't need the entire
+	 * granule, but the entire region we're mapping must support the same
+	 * attribute.
 	 */
+	if (attr & EFI_MEMORY_WB || attr & EFI_MEMORY_UC)
+		return 1;
+
+	/*
+	 * Intel firmware doesn't tell us about all the MMIO regions, so
+	 * in general we have to allow mmap requests.  But if EFI *does*
+	 * tell us about anything inside this region, we should deny it.
+	 * The user can always map a smaller region to avoid the overlap.
+	 */
+	if (efi_memmap_intersects(phys_addr, size))
+		return 0;
+
 	return 1;
 }
 

[patch 4/5] ia64: fail mmaps that span areas with incompatible attributes

[Bjorn Helgaas]

Example memory map (from HP sx1000 with VGA enabled):
    0x00000 - 0x9FFFF supports only WB (cacheable) access
    0xA0000 - 0xBFFFF supports only UC (uncacheable) access
    0xC0000 - 0xFFFFF supports only WB (cacheable) access

Some versions of X map the entire 0x00000-0xFFFFF area at once.  With the
example above, this mmap must fail because there's no memory attribute that's
safe for the entire area.

Prior to this patch, we performed the mmap with a UC mapping.  When X
accessed the WB memory at 0xC0000, it caused an MCA.  The crash can happen
when mapping 0xC0000 from either /dev/mem or a /sys/.../legacy_mem file.

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

Index: work-mm10/arch/ia64/kernel/efi.c
=================================--- work-mm10.orig/arch/ia64/kernel/efi.c	2007-03-20 21:07:28.000000000 -0700
+++ work-mm10/arch/ia64/kernel/efi.c	2007-03-20 21:13:05.000000000 -0700
@@ -663,6 +663,29 @@
 	return NULL;
 }
 
+static int
+efi_memmap_intersects (unsigned long phys_addr, unsigned long size)
+{
+	void *efi_map_start, *efi_map_end, *p;
+	efi_memory_desc_t *md;
+	u64 efi_desc_size;
+	unsigned long end;
+
+	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;
+
+	end = phys_addr + size;
+
+	for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
+		md = p;
+
+		if (md->phys_addr < end && efi_md_end(md) > phys_addr)
+			return 1;
+	}
+	return 0;
+}
+
 u32
 efi_mem_type (unsigned long phys_addr)
 {
@@ -769,11 +792,28 @@
 int
 valid_mmap_phys_addr_range (unsigned long pfn, unsigned long size)
 {
+	unsigned long phys_addr = pfn << PAGE_SHIFT;
+	u64 attr;
+
+	attr = efi_mem_attribute(phys_addr, size);
+
 	/*
-	 * MMIO regions are often missing from the EFI memory map.
-	 * We must allow mmap of them for programs like X, so we
-	 * currently can't do any useful validation.
+	 * /dev/mem mmap uses normal user pages, so we don't need the entire
+	 * granule, but the entire region we're mapping must support the same
+	 * attribute.
 	 */
+	if (attr & EFI_MEMORY_WB || attr & EFI_MEMORY_UC)
+		return 1;
+
+	/*
+	 * Intel firmware doesn't tell us about all the MMIO regions, so
+	 * in general we have to allow mmap requests.  But if EFI *does*
+	 * tell us about anything inside this region, we should deny it.
+	 * The user can always map a smaller region to avoid the overlap.
+	 */
+	if (efi_memmap_intersects(phys_addr, size))
+		return 0;
+
 	return 1;
 }
 

Re: [patch 4/5] ia64: fail mmaps that span areas with incompatible attributes

[Peter Chubb]

>>>>> "Bjorn" = Bjorn Helgaas <bjorn.helgaas@hp.com> writes:

Bjorn> Example memory map (from HP sx1000 with VGA enabled): 0x00000 -
Bjorn> 0x9FFFF supports only WB (cacheable) access 0xA0000 - 0xBFFFF
Bjorn> supports only UC (uncacheable) access 0xC0000 - 0xFFFFF
Bjorn> supports only WB (cacheable) access

Bjorn> Some versions of X map the entire 0x00000-0xFFFFF area at once.
Bjorn> With the example above, this mmap must fail because there's no
Bjorn> memory attribute that's safe for the entire area.

This patch breaks X on a ZX2000.

lspci -v gives:
00:00.0 VGA compatible controller: ATI Technologies Inc Radeon RV100 QY [Radeon 
7000/VE] (prog-if 00 [VGA])
        Subsystem: ATI Technologies Inc Unknown device 010a
        Flags: bus master, stepping, 66MHz, medium devsel, latency 192, IRQ 51
        Memory at 80000000 (32-bit, prefetchable) [size\x128M]
        I/O ports at 0d00 [size%6]
        Memory at 88020000 (32-bit, non-prefetchable) [sizedK]
        Expansion ROM at 88000000 [disabled] [size\x128K]
        Capabilities: [58] AGP version 2.0
        Capabilities: [50] Power Management version 2

strace on X gives:
...
open("/sys/class/pci_bus/0000:00/legacy_mem", O_RDWR) = 8
mmap(NULL, 1048576, PROT_READ|PROT_WRITE, MAP_SHARED, 8, 0) = -1 EINVAL (Invalid argument)
close(8)                                = 0
write(2, "mmap failure: Invalid argument\n", 31mmap failure: Invalid argument
) = 31
write(2, "\nFatal server error:\n", 21
Fatal server error:



EFI memory map is:
mem00: type=4, attr=0x8, range=[0x0000000000000000-0x0000000000001000) (0MB)
mem01: type=7, attr=0x8, range=[0x0000000000001000-0x00000000000a0000) (0MB)
mem02: type\x11, attr=0x3, range=[0x00000000000a0000-0x00000000000c0000) (0MB)
mem03: type=5, attr=0x8000000000000001, range=[0x00000000000c0000-0x0000000000100000) (0MB)
...

so the patch prevents the mmap from succeeding.

What does the spec say?  *Should* it be possible to map the entire ISA
legacy space in one fell swoop?  X *works* iff I allow the mapping.



--
Dr Peter Chubb  http://www.gelato.unsw.edu.au  peterc AT gelato.unsw.edu.au
http://www.ertos.nicta.com.au           ERTOS within National ICT Australia

Re: [patch 4/5] ia64: fail mmaps that span areas with incompatible attributes

[Peter Chubb]

>>>>> "Peter" = Peter Chubb <peter@chubb.wattle.id.au> writes:

>>>>> "Bjorn" = Bjorn Helgaas <bjorn.helgaas@hp.com> writes:

Bjorn> Example memory map (from HP sx1000 with VGA enabled): 0x00000 -
Bjorn> 0x9FFFF supports only WB (cacheable) access 0xA0000 - 0xBFFFF
Bjorn> supports only UC (uncacheable) access 0xC0000 - 0xFFFFF
Bjorn> supports only WB (cacheable) access

Bjorn> Some versions of X map the entire 0x00000-0xFFFFF area at once.
Bjorn> With the example above, this mmap must fail because there's no
Bjorn> memory attribute that's safe for the entire area.

Peter> This patch breaks X on a ZX2000.

Except that there is a fix in the latest X source.

Sorry to bother you all.

PeterC
--
Dr Peter Chubb  http://www.gelato.unsw.edu.au  peterc AT gelato.unsw.edu.au
http://www.ertos.nicta.com.au           ERTOS within National ICT Australia

Re: [patch 4/5] ia64: fail mmaps that span areas with incompatible attributes

[Bjorn Helgaas]

On Sunday 27 May 2007 08:55:45 pm Peter Chubb wrote:
> >>>>> "Peter" = Peter Chubb <peter@chubb.wattle.id.au> writes:
> 
> >>>>> "Bjorn" = Bjorn Helgaas <bjorn.helgaas@hp.com> writes:
> 
> Bjorn> Example memory map (from HP sx1000 with VGA enabled): 0x00000 -
> Bjorn> 0x9FFFF supports only WB (cacheable) access 0xA0000 - 0xBFFFF
> Bjorn> supports only UC (uncacheable) access 0xC0000 - 0xFFFFF
> Bjorn> supports only WB (cacheable) access
> 
> Bjorn> Some versions of X map the entire 0x00000-0xFFFFF area at once.
> Bjorn> With the example above, this mmap must fail because there's no
> Bjorn> memory attribute that's safe for the entire area.
> 
> Peter> This patch breaks X on a ZX2000.
> 
> Except that there is a fix in the latest X source.
> 
> Sorry to bother you all.

No problem, I should have mentioned the X server interlock
in the changelog, which would have saved you the trouble of
debugging this.  This whole area has been an incredible
headache, and I recently discovered yet another hole to
be fixed (mmap via /proc/bus/pci needs to be checked, iirc).

Bjorn