Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly

[graff yang]

On Thu, Sep 24, 2009 at 10:13 PM, David Howells <dhowells@xxxxxxxxxx> wrote:
> Fix MAP_PRIVATE mmap() of files and devices where the data in the backing store
> might be mapped directly. ÂUse the BDI_CAP_MAP_DIRECT capability flag to govern
> whether or not we should be trying to map a file directly. ÂThis can be used to
> determine whether or not a region has been filled in at the point where we call
> do_mmap_shared() or do_mmap_private().
>
> The BDI_CAP_MAP_DIRECT capability flag is cleared by validate_mmap_request() if
> there's any reason we can't use it. ÂIt's also cleared in do_mmap_pgoff() if
> f_op->get_unmapped_area() fails.
>
>
> Without this fix, attempting to run a program from a RomFS image on a
> non-mappable MTD partition results in a BUG as the kernel attempts XIP, and
> this can be caught in gdb:
>
> Program received signal SIGABRT, Aborted.
> 0xc005dce8 in add_nommu_region (region=<value optimized out>) at mm/nommu.c:547
> (gdb) bt
> #0 Â0xc005dce8 in add_nommu_region (region=<value optimized out>) at mm/nommu.c:547
> #1 Â0xc005f168 in do_mmap_pgoff (file=0xc31a6620, addr=<value optimized out>, len=3808, prot=3, flags=6146, pgoff=0) at mm/nommu.c:1373
> #2 Â0xc00a96b8 in elf_fdpic_map_file (params=0xc33fbbec, file=0xc31a6620, mm=0xc31bef60, what=0xc0213144 "executable") at mm.h:1145
> #3 Â0xc00aa8b4 in load_elf_fdpic_binary (bprm=0xc316cb00, regs=<value optimized out>) at fs/binfmt_elf_fdpic.c:343
> #4 Â0xc006b588 in search_binary_handler (bprm=0x6, regs=0xc33fbce0) at fs/exec.c:1234
> #5 Â0xc006c648 in do_execve (filename=<value optimized out>, argv=0xc3ad14cc, envp=0xc3ad1460, regs=0xc33fbce0) at fs/exec.c:1356
> #6 Â0xc0008cf0 in sys_execve (name=<value optimized out>, argv=0xc3ad14cc, envp=0xc3ad1460) at arch/frv/kernel/process.c:263
> #7 Â0xc00075dc in __syscall_call () at arch/frv/kernel/entry.S:897
>
>
> Note that this fix does the following commit differently:
>
> Â Â Â Âcommit a190887b58c32d19c2eee007c5eb8faa970a69ba
> Â Â Â ÂAuthor: David Howells <dhowells@xxxxxxxxxx>
> Â Â Â ÂDate: Â Sat Sep 5 11:17:07 2009 -0700
> Â Â Â Ânommu: fix error handling in do_mmap_pgoff()
>
> Reported-by: Graff Yang <graff.yang@xxxxxxxxx>
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> Cc: Pekka Enberg <penberg@xxxxxxxxxxxxxx>
> Cc: Paul Mundt <lethal@xxxxxxxxxxxx>
> Cc: Mel Gorman <mel@xxxxxxxxx>
> Cc: Greg Ungerer <gerg@xxxxxxxxxxxx>
> ---
>
> Âmm/nommu.c | Â 34 ++++++++++++----------------------
> Â1 files changed, 12 insertions(+), 22 deletions(-)
>
>
> diff --git a/mm/nommu.c b/mm/nommu.c
> index c459aec..cc24d9f 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1074,7 +1074,7 @@ static int do_mmap_shared_file(struct vm_area_struct *vma)
> Â Â Â Âret = vma->vm_file->f_op->mmap(vma->vm_file, vma);
> Â Â Â Âif (ret == 0) {
> Â Â Â Â Â Â Â Âvma->vm_region->vm_top = vma->vm_region->vm_end;
> - Â Â Â Â Â Â Â return ret;
> + Â Â Â Â Â Â Â return 0;
> Â Â Â Â}
> Â Â Â Âif (ret != -ENOSYS)
> Â Â Â Â Â Â Â Âreturn ret;
> @@ -1091,7 +1091,8 @@ static int do_mmap_shared_file(struct vm_area_struct *vma)
> Â*/
> Âstatic int do_mmap_private(struct vm_area_struct *vma,
> Â Â Â Â Â Â Â Â Â Â Â Â Â struct vm_region *region,
> - Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long len)
> + Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long len,
> + Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long capabilities)
> Â{
> Â Â Â Âstruct page *pages;
> Â Â Â Âunsigned long total, point, n, rlen;
> @@ -1102,13 +1103,13 @@ static int do_mmap_private(struct vm_area_struct *vma,
> Â Â Â Â * shared mappings on devices or memory
> Â Â Â Â * - VM_MAYSHARE will be set if it may attempt to share
> Â Â Â Â */
> - Â Â Â if (vma->vm_file) {
> + Â Â Â if (capabilities & BDI_CAP_MAP_DIRECT) {

This will breaks many drivers, e.g. some frame-buffer drivers, on NOMMU system.
Because they don't have get_unmapped_area().
These drivers depend on it's mmap() to return the frame-buffer base address.

-Graff


> Â Â Â Â Â Â Â Âret = vma->vm_file->f_op->mmap(vma->vm_file, vma);
> Â Â Â Â Â Â Â Âif (ret == 0) {
> Â Â Â Â Â Â Â Â Â Â Â Â/* shouldn't return success if we're not sharing */
> Â Â Â Â Â Â Â Â Â Â Â ÂBUG_ON(!(vma->vm_flags & VM_MAYSHARE));
> Â Â Â Â Â Â Â Â Â Â Â Âvma->vm_region->vm_top = vma->vm_region->vm_end;
> - Â Â Â Â Â Â Â Â Â Â Â return ret;
> + Â Â Â Â Â Â Â Â Â Â Â return 0;
> Â Â Â Â Â Â Â Â}
> Â Â Â Â Â Â Â Âif (ret != -ENOSYS)
> Â Â Â Â Â Â Â Â Â Â Â Âreturn ret;
> @@ -1346,7 +1347,7 @@ unsigned long do_mmap_pgoff(struct file *file,
> Â Â Â Â Â Â Â Â * - this is the hook for quasi-memory character devices to
> Â Â Â Â Â Â Â Â * Â tell us the location of a shared mapping
> Â Â Â Â Â Â Â Â */
> - Â Â Â Â Â Â Â if (file && file->f_op->get_unmapped_area) {
> + Â Â Â Â Â Â Â if (capabilities & BDI_CAP_MAP_DIRECT) {
> Â Â Â Â Â Â Â Â Â Â Â Âaddr = file->f_op->get_unmapped_area(file, addr, len,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â pgoff, flags);
> Â Â Â Â Â Â Â Â Â Â Â Âif (IS_ERR((void *) addr)) {
> @@ -1370,15 +1371,17 @@ unsigned long do_mmap_pgoff(struct file *file,
> Â Â Â Â}
>
> Â Â Â Âvma->vm_region = region;
> - Â Â Â add_nommu_region(region);
>
> - Â Â Â /* set up the mapping */
> + Â Â Â /* set up the mapping
> + Â Â Â Â* - the region is filled in if BDI_CAP_MAP_DIRECT is still set
> + Â Â Â Â*/
> Â Â Â Âif (file && vma->vm_flags & VM_SHARED)
> Â Â Â Â Â Â Â Âret = do_mmap_shared_file(vma);
> Â Â Â Âelse
> - Â Â Â Â Â Â Â ret = do_mmap_private(vma, region, len);
> + Â Â Â Â Â Â Â ret = do_mmap_private(vma, region, len, capabilities);
> Â Â Â Âif (ret < 0)
> - Â Â Â Â Â Â Â goto error_put_region;
> + Â Â Â Â Â Â Â goto error_just_free;
> + Â Â Â add_nommu_region(region);
>
> Â Â Â Â/* okay... we have a mapping; now we have to register it */
> Â Â Â Âresult = vma->vm_start;
> @@ -1396,19 +1399,6 @@ share:
> Â Â Â Âkleave(" = %lx", result);
> Â Â Â Âreturn result;
>
> -error_put_region:
> - Â Â Â __put_nommu_region(region);
> - Â Â Â if (vma) {
> - Â Â Â Â Â Â Â if (vma->vm_file) {
> - Â Â Â Â Â Â Â Â Â Â Â fput(vma->vm_file);
> - Â Â Â Â Â Â Â Â Â Â Â if (vma->vm_flags & VM_EXECUTABLE)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â removed_exe_file_vma(vma->vm_mm);
> - Â Â Â Â Â Â Â }
> - Â Â Â Â Â Â Â kmem_cache_free(vm_area_cachep, vma);
> - Â Â Â }
> - Â Â Â kleave(" = %d [pr]", ret);
> - Â Â Â return ret;
> -
> Âerror_just_free:
> Â Â Â Âup_write(&nommu_region_sem);
> Âerror:
>
>



-- 
-Graff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/