2005-12-06 00:00:26

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH] /dev/mem validate mmap requests

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 an uncacheable
MMIO hole at 640K, which requires that we use UC mappings for the entire
0-16MB granule. An application (e.g., "hwinfo", which grubs around for
BIOS details) that mmaps the write-back memory in that granule gets a UC
mapping. Some chipsets do not support UC access to WB memory and raise
a machine check.

Note: this depends on the previous __HAVE_PHYS_MEM_ACCESS_PROT tidy-up.

Tony, please ack/nak so this can go in via akpm.

Signed-off-by: Bjorn Helgaas <[email protected]>

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

Index: work4/arch/ia64/kernel/efi.c
===================================================================
--- work4.orig/arch/ia64/kernel/efi.c 2005-12-02 15:40:12.000000000 -0700
+++ work4/arch/ia64/kernel/efi.c 2005-12-05 15:50:52.000000000 -0700
@@ -792,6 +792,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)
{
@@ -818,6 +822,33 @@
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;
+
+ 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 (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;
+ }
+ }
+ return 0;
+}
+
int __init
efi_uart_console_only(void)
{
Index: work4/drivers/char/mem.c
===================================================================
--- work4.orig/drivers/char/mem.c 2005-12-02 15:40:35.000000000 -0700
+++ work4/drivers/char/mem.c 2005-12-05 15:49:46.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

/*
@@ -243,16 +248,19 @@
static int mmap_mem(struct file * file, struct vm_area_struct * vma)
{
unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
+ size_t size = vma->vm_end - vma->vm_start;
+
+ if (!valid_mmap_phys_addr_range(offset, &size))
+ return -EINVAL;

- vma->vm_page_prot = phys_mem_access_prot(file, offset,
- vma->vm_end - vma->vm_start,
+ vma->vm_page_prot = phys_mem_access_prot(file, offset, 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: work4/include/asm-ia64/io.h
===================================================================
--- work4.orig/include/asm-ia64/io.h 2005-10-12 14:27:18.000000000 -0600
+++ work4/include/asm-ia64/io.h 2005-12-02 15:42:15.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.


2005-12-06 18:41:37

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH] /dev/mem validate mmap requests

On Mon, 5 Dec 2005, Bjorn Helgaas wrote:
> Add a hook so architectures can validate /dev/mem mmap requests.

Not a comment on your patch at all, just an FYI in case you've missed
it, and in case it makes any difference to your ia64 needs.

A side-effect of 2.6.15-rc's PageReserved changes is that a user with
access to /dev/mem can now mmap any page of it, and see what's there
rather than zeroes, whether or not it has been marked PageReserved.

Hugh

2005-12-06 20:49:20

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] /dev/mem validate mmap requests

On Tuesday 06 December 2005 11:39 am, Hugh Dickins wrote:
> On Mon, 5 Dec 2005, Bjorn Helgaas wrote:
> > Add a hook so architectures can validate /dev/mem mmap requests.
>
> Not a comment on your patch at all, just an FYI in case you've missed
> it, and in case it makes any difference to your ia64 needs.
>
> A side-effect of 2.6.15-rc's PageReserved changes is that a user with
> access to /dev/mem can now mmap any page of it, and see what's there
> rather than zeroes, whether or not it has been marked PageReserved.

Thanks for the pointer.

I think my patch is orthogonal to your PageReserved changes.

Here's a little more detail on the problem scenario. An HP
sx1000 machine with VGA has a memory map like this:

0-640K memory (supports only WB mappings)
640K-768K VGA frame buffer (MMIO, supports only UC mappings)
768K-16M memory (supports only WB mappings)

We use a 16MB "granule". All mappings within a granule have to
be the same type to avoid attribute aliasing. Since we're using
VGA, we have to use UC mappings for the MMIO frame buffer. We
can't use UC mappings for the memory, because the sx1000 chipset
doesn't support that. We can't use WB mappings, because that would
cause attribute aliasing.

Since we can't use WB mappings, the kernel ignores all that memory
between 0-16MB. There aren't even any page structures for it. But
without my patch, /dev/mem allows users to mmap it. And there's no
safe way to allow access of any kind to that memory.

Bjorn