[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200810021523.45884.jbarnes@virtuousgeek.org>
Date: Thu, 2 Oct 2008 15:23:43 -0700
From: Jesse Barnes <jbarnes@...tuousgeek.org>
To: Jesse Brandeburg <jesse.brandeburg@...el.com>
Cc: linux-kernel@...r.kernel.org, linux-netdev@...r.kernel.org,
kkeil@...e.de, agospoda@...hat.com, arjan@...ux.intel.com,
david.graham@...el.com, bruce.w.allan@...el.com, jkosina@...e.cz,
john.ronciak@...el.com, tglx@...utronix.de,
chris.jones@...onical.com, tim.gardner@...el.com, airlied@...il.com
Subject: Re: [RFC PATCH 02/12] On Tue, 23 Sep 2008, David Miller wrote:
Ping DaveM. Does this look ok? What else would we need for you to remove
your range checking from sparc?
Thanks,
Jesse
On Monday, September 29, 2008 8:19 pm Jesse Brandeburg wrote:
> From: Jesse Barnes <jbarnes@...tuousgeek.org>
>
> > I did some snooping around, and while doing so I noticed that the PCI
> > mmap code for x86 doesn't do one bit of range checking on the size, or
> > any other aspect of the request, wrt. the MMIO regions actually mapped
> > in the BARs of the PCI device.
>
> Here's a patch that adds range checking to the sysfs mappings at
> least. This patch should catch the case where X (or some other
> process) tries to map beyond the specific BAR it's (supposedly)
> trying to access, making things safer in general. FWIW both my
> F9 and development versions of X start up fine with this patch
> applied.
>
> DaveM, will this work for you on sparc? It looked like your code
> was allowing bridge window mappings, but that behavior should be
> preserved as long as your bridge devices reflect their window
> sizes correctly in their pdev->resources?
>
> If we add similar code to the procfs stuff we wouldn't need to do
> any checking in the arches.
>
> Signed-off-by: Jesse Barnes <jbarnes@...tuousgeek.org>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
> ---
>
> drivers/pci/pci-sysfs.c | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 9c71858..4d1aa6e 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -16,6 +16,7 @@
>
>
> #include <linux/kernel.h>
> +#include <linux/sched.h>
> #include <linux/pci.h>
> #include <linux/stat.h>
> #include <linux/topology.h>
> @@ -502,6 +503,8 @@ pci_mmap_resource(struct kobject *kobj, struct
> bin_attribute *attr, struct resource *res = (struct resource
> *)attr->private;
> enum pci_mmap_state mmap_type;
> resource_size_t start, end;
> + unsigned long map_len = vma->vm_end - vma->vm_start;
> + unsigned long map_offset = vma->vm_pgoff << PAGE_SHIFT;
> int i;
>
> for (i = 0; i < PCI_ROM_RESOURCE; i++)
> @@ -510,6 +513,17 @@ pci_mmap_resource(struct kobject *kobj, struct
> bin_attribute *attr, if (i >= PCI_ROM_RESOURCE)
> return -ENODEV;
>
> + /*
> + * Make sure the range the user is trying to map falls within
> + * the resource
> + */
> + if (map_offset + map_len > pci_resource_len(pdev, i)) {
> + WARN(1, "process \"%s\" tried to map 0x%08lx-0x%08lx on BAR %d (size
> 0x%08lx)\n", + current->comm, map_offset, map_offset + map_len, i,
> + (unsigned long)pci_resource_len(pdev, i));
> + return -EINVAL;
> + }
> +
> /* pci_mmap_page_range() expects the same kind of entry as coming
> * from /proc/bus/pci/ which is a "user visible" value. If this is
> * different from the resource itself, arch will do necessary fixup.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Jesse Barnes, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists