check mapped ranges on sysfs resource files (for 2.6.27)

I'm sending a patch rather than a pull request since it's just this. Totally up to you (obviously) whether we stuff this into 2.6.27 or hold off on it until 2.6.28.

It's fairly common for applications to map PCI resources through sysfs. However, with the current implementation, it's possible for an application to map far more than the range corresponding to the resourceN file it opened. This patch plugs that hole by checking the range at mmap time, similar to what is done on platforms like sparc64 in their lower level PCI remapping routines.

It was initially put together to help debug the e1000e NVRAM corruption problem, since we initially thought an X driver might be walking past the end of one of its mappings and clobbering the NVRAM. It now looks like that's not the case, but doing the check is still important for obvious reasons.

index 9c71858..4d1aa6e 100644
@@ -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.

Ah yes, exactly what we want to be checking for in fact.

Hm, yeah looks like that's a long standing issue...

Yeah, looks much better. I was using this silly test program to see if the earlier code worked. Just pass in both valid and invalid sizes.

Jesse

#include <errno.h>
#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>

int main(int argc, char *argv[])

{
	size_t len;
	int fd;
	void *ptr;

	if (argc != 3) {
		fprintf(stderr, "usage: %s <file> <mapsize>\n",
			argv[0]);
		return -1;
	}

	len = atoi(argv[2]);

	fd = open(argv[1], O_RDONLY);
	if (fd == -1) {
		fprintf(stderr, "open failed: %s\n", strerror(errno));
		return errno;
	}

	ptr = mmap(NULL, len, PROT_READ, MAP_SHARED, fd, 0);
	if (ptr == MAP_FAILED) {
		fprintf(stderr, "mmap failed: %s\n", strerror(errno));
		return errno;
	}

	printf("mmap of %s with size %zd succeeded\n", argv[1], len);

	munmap(ptr, len); /* ignore any errors, we don't care */
	close(fd);
	return 0;
}

All right, it seems to pass your test program, so I committed it with your ack and quoting your original reason for doing this.

Linus