Message ID | 1348935073.2036.147.camel@shinybook.infradead.org |
---|---|
State | New, archived |
Headers | show |
On Sat, 2012-09-29 at 17:11 +0100, David Woodhouse wrote: > > That check seems to have been missing from David's commit 402d3265 in > which he introduced the mtd_mmap() operation, and wasn't fixed in commit > dd02b67d5 where Anatolij fixed things to actually *work* in the MMU code > path. This should fix it: > + if (map->phys == NO_XIP) > + return -EINVAL; Hm, but there's another problem. That 'map' variable is pulled from mtd->priv but there's a clue in the name 'priv'.... it isn't guaranteed to *be* a device that goes through the map abstraction. Anatolij? Your patch dd02b67d5 might have worked on your test case, but it fails disgracefully in any of the cases where it *isn't* expected to work. I think it needs to use mtd_unmapped_area() on the device in question just like the !CONFIG_MMU code path does, and avoid grubbing around in things that it shouldn't be looking at directly. David, you made mtd_unmapped_area() return the *virtual* address... but there's no reason that couldn't have been the physical address, right? You were only using it in the !CONFIG_MMU case anyway, where they're equal. In the meantime, I think the quick fix is just to disable mtdchar_mmap in the CONFIG_MMU case. It was broken from the moment David introduced it, and Anatolij's fix was insufficient. I'll do that.
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c index f2f482b..27c2f57 100644 --- a/drivers/mtd/mtdchar.c +++ b/drivers/mtd/mtdchar.c @@ -1137,8 +1137,10 @@ static int mtdchar_mmap(struct file *file, struct vm_area_struct *vma) u32 len; if (mtd->type == MTD_RAM || mtd->type == MTD_ROM) { - off = vma->vm_pgoff << PAGE_SHIFT; start = map->phys; + if (map->phys == NO_XIP) + return -EINVAL; + off = vma->vm_pgoff << PAGE_SHIFT; len = PAGE_ALIGN((start & ~PAGE_MASK) + map->size); start &= PAGE_MASK;