Re: [PATCH v2] nommu: add page_align to mmap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Greg

On Fri, Jun 10, 2011 at 11:51 AM, Greg Ungerer <gerg@xxxxxxxxxxxx> wrote:
> Hi Bob,
>
> On 09/06/11 20:30, Bob Liu wrote:
>>
>> On Wed, Jun 8, 2011 at 6:19 PM, Greg Ungerer<gerg@xxxxxxxxxxxx> Âwrote:
>>>>>>>
>>>>>>> When booting on a ColdFire (m68knommu) target the init process (or
>>>>>>> there abouts at least) fails. Last console messages are:
>>>>>>>
>>>>>>> ...
>>>>>>> VFS: Mounted root (romfs filesystem) readonly on device 31:0.
>>>>>>> Freeing unused kernel memory: 52k freed (0x401aa000 - 0x401b6000)
>>>>>>> Unable to mmap process text, errno 22
>>>>>>>
>>>>>>
>>>>>> Oh, bad news. I will try to reproduce it on my board.
>>>>>> If you are free please enable debug in nommu.c and then we can see
>>>>>> what
>>>>>> caused the problem.
>>>>>
>>>>> Yep, with debug on:
>>>>>
>>>>> Â...
>>>>> VFS: Mounted root (romfs filesystem) readonly on device 31:0.
>>>>> Freeing unused kernel memory: 52k freed (0x4018c000 - 0x40198000)
>>>>> ==> ÂÃÂdo_mmap_pgoff(,0,6780,5,1002,0)
>>>>> <== do_mmap_pgoff() = -22
>>>>> Unable to mmap process text, errno 22
>>>>>
>>>>
>>>> Since I can't reproduce this problem, could you please attach the
>>>> whole dmesg log with nommu debug on or
>>>> you can step into to see why errno 22 is returned, is it returned by
>>>> do_mmap_private()?
>>>
>>> There was no other debug messages with debug turned on in nommu.c.
>>> (I can give you the boot msgs before this if you want, but there
>>> was no nommu.c debug in it).
>>>
>>> But I did trace it into do_mmap_pgoff() to see what was failing.
>>> It fails based on the return value from:
>>>
>>> addr = file->f_op->get_unmapped_area(file, addr, len,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âpgoff, flags);
>>>
>>
>> Thanks for this information.
>> But it's a callback function. I still can't know what's the problem maybe.
>> Would you do me a favor to do more trace to see where it callback to,
>> fs or some driver etc..?
>
> Its calling to romfs_get_unmapped_area() [fs/romfs/mmap-nommu.c]. It is
> being called with:
>
> Âromfs_get_unmapped_area(addr=0,len=7000,pgoff=0,flags=1002)
>
> This is failing the first size check because isize comes back
> as 0x6ca8, and this is smaller then len (0x7000). Thus returning
> -EINVAL.
>

I look into file fs/romfs/mmap-nommu.c based on your trace.
In my opinion, romfs_get_unmapped_area() in mmap-nommu.c is buggy.
Would you please try below commit.
Thanks a lot.

>From 786add5286ffb476807cb198d7b2c5455e9fb533 Mon Sep 17 00:00:00 2001
From: Bob Liu <lliubbo@xxxxxxxxx>
Date: Fri, 10 Jun 2011 13:34:48 +0800
Subject: [PATCH] romfs: fix romfs_get_unmapped_area() param check

romfs_get_unmapped_area() check len param without considering PAGE_ALIGN which
will cause do_mmap_pgoff() return -EINVAL error after commit f67d9b1576c.

This patch fix the param check by changing it to the same way as function
ramfs_nommu_get_unmapped_area() did in ramfs/file-nommu.c.

Signed-off-by: Bob Liu <lliubbo@xxxxxxxxx>
---
 fs/romfs/mmap-nommu.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/romfs/mmap-nommu.c b/fs/romfs/mmap-nommu.c
index f0511e8..eed9942 100644
--- a/fs/romfs/mmap-nommu.c
+++ b/fs/romfs/mmap-nommu.c
@@ -27,14 +27,18 @@ static unsigned long
romfs_get_unmapped_area(struct file *file,
 {
        struct inode *inode = file->f_mapping->host;
        struct mtd_info *mtd = inode->i_sb->s_mtd;
-       unsigned long isize, offset;
+       unsigned long isize, offset, maxpages, lpages;

        if (!mtd)
                goto cant_map_directly;

+       /* the mapping mustn't extend beyond the EOF */
+       lpages = (len + PAGE_SIZE - 1) >> PAGE_SHIFT;
        isize = i_size_read(inode);
        offset = pgoff << PAGE_SHIFT;
-       if (offset > isize || len > isize || offset > isize - len)
+
+       maxpages = (isize + PAGE_SIZE - 1) >> PAGE_SHIFT;
+       if ((pgoff >= maxpages) || (maxpages - pgoff < lpages))
                return (unsigned long) -EINVAL;

        /* we need to call down to the MTD layer to do the actual mapping */
--
1.6.3.3

> That code is trying to map the contents of the file /bin/init
> directly from the romfs filesystem (which is in RAM). The init
> binary is 0x6ca8 bytes in size (that is the isize above).
>

-- 
Regards,
--Bob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]