public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly
@ 2009-09-24 14:13 David Howells
  2009-09-24 19:59 ` Pekka Enberg
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: David Howells @ 2009-09-24 14:13 UTC (permalink / raw)
  To: torvalds, akpm, graff.yang
  Cc: linux-kernel, David Howells, Pekka Enberg, Paul Mundt, Mel Gorman,
	Greg Ungerer

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@redhat.com>
	Date:   Sat Sep 5 11:17:07 2009 -0700
	nommu: fix error handling in do_mmap_pgoff()

Reported-by: Graff Yang <graff.yang@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Greg Ungerer <gerg@snapgear.com>
---

 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) {
 		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:


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data  can be mapped directly
  2009-09-24 14:13 [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly David Howells
@ 2009-09-24 19:59 ` Pekka Enberg
  2009-09-24 22:09 ` Andrew Morton
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Pekka Enberg @ 2009-09-24 19:59 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, akpm, graff.yang, linux-kernel, Paul Mundt, Mel Gorman,
	Greg Ungerer

Hi David,

On Thu, Sep 24, 2009 at 5:13 PM, David Howells <dhowells@redhat.com> 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

I have no idea about the BDI_CAP_MAP_DIRECT bits but...

> Note that this fix does the following commit differently:
>
>        commit a190887b58c32d19c2eee007c5eb8faa970a69ba
>        Author: David Howells <dhowells@redhat.com>
>        Date:   Sat Sep 5 11:17:07 2009 -0700
>        nommu: fix error handling in do_mmap_pgoff()

...changes to this part of the code look like nice cleanup to me.

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly
  2009-09-24 14:13 [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly David Howells
  2009-09-24 19:59 ` Pekka Enberg
@ 2009-09-24 22:09 ` Andrew Morton
  2009-09-25  0:39   ` David Howells
  2009-09-25  0:43 ` Paul Mundt
  2009-09-25  4:39 ` graff yang
  3 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2009-09-24 22:09 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, graff.yang, linux-kernel, dhowells, penberg, lethal,
	mel, gerg, stable

On Thu, 24 Sep 2009 15:13:10 +0100
David Howells <dhowells@redhat.com> 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.
> 

The patch seems -stable-worthy but I see no cc:stable here?

> Note that this fix does the following commit differently:
> 
> 	commit a190887b58c32d19c2eee007c5eb8faa970a69ba
> 	Author: David Howells <dhowells@redhat.com>
> 	Date:   Sat Sep 5 11:17:07 2009 -0700
> 	nommu: fix error handling in do_mmap_pgoff()

I don't understand what you're saying here.

> Reported-by: Graff Yang <graff.yang@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Cc: Pekka Enberg <penberg@cs.helsinki.fi>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Greg Ungerer <gerg@snapgear.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly
  2009-09-24 22:09 ` Andrew Morton
@ 2009-09-25  0:39   ` David Howells
  2009-09-25  1:01     ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: David Howells @ 2009-09-25  0:39 UTC (permalink / raw)
  To: Andrew Morton
  Cc: dhowells, torvalds, graff.yang, linux-kernel, penberg, lethal,
	mel, gerg, stable

Andrew Morton <akpm@linux-foundation.org> wrote:

> > Note that this fix does the following commit differently:
> > 
> > 	commit a190887b58c32d19c2eee007c5eb8faa970a69ba
> > 	Author: David Howells <dhowells@redhat.com>
> > 	Date:   Sat Sep 5 11:17:07 2009 -0700
> > 	nommu: fix error handling in do_mmap_pgoff()
> 
> I don't understand what you're saying here.

Sorry, I was attempting to point out that this appears to revert some of that
patch - whilst this is true, it then implements the fix differently.

David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly
  2009-09-24 14:13 [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly David Howells
  2009-09-24 19:59 ` Pekka Enberg
  2009-09-24 22:09 ` Andrew Morton
@ 2009-09-25  0:43 ` Paul Mundt
  2009-09-25  4:39 ` graff yang
  3 siblings, 0 replies; 10+ messages in thread
From: Paul Mundt @ 2009-09-25  0:43 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, akpm, graff.yang, linux-kernel, Pekka Enberg,
	Mel Gorman, Greg Ungerer

On Thu, Sep 24, 2009 at 03:13:10PM +0100, David Howells 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@redhat.com>
> 	Date:   Sat Sep 5 11:17:07 2009 -0700
> 	nommu: fix error handling in do_mmap_pgoff()
> 
> Reported-by: Graff Yang <graff.yang@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Cc: Pekka Enberg <penberg@cs.helsinki.fi>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Greg Ungerer <gerg@snapgear.com>

Acked-by: Paul Mundt <lethal@linux-sh.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly
  2009-09-25  0:39   ` David Howells
@ 2009-09-25  1:01     ` Andrew Morton
  2009-09-25  1:17       ` David Howells
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2009-09-25  1:01 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, graff.yang, linux-kernel, penberg, lethal, mel, gerg,
	stable

On Fri, 25 Sep 2009 01:39:17 +0100 David Howells <dhowells@redhat.com> wrote:

> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > > Note that this fix does the following commit differently:
> > > 
> > > 	commit a190887b58c32d19c2eee007c5eb8faa970a69ba
> > > 	Author: David Howells <dhowells@redhat.com>
> > > 	Date:   Sat Sep 5 11:17:07 2009 -0700
> > > 	nommu: fix error handling in do_mmap_pgoff()
> > 
> > I don't understand what you're saying here.
> 
> Sorry, I was attempting to point out that this appears to revert some of that
> patch - whilst this is true, it then implements the fix differently.
> 

OK.

And what are your opinions on the -stable desirability?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly
  2009-09-25  1:01     ` Andrew Morton
@ 2009-09-25  1:17       ` David Howells
  0 siblings, 0 replies; 10+ messages in thread
From: David Howells @ 2009-09-25  1:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: dhowells, torvalds, graff.yang, linux-kernel, penberg, lethal,
	mel, gerg, stable

Andrew Morton <akpm@linux-foundation.org> wrote:

> And what are your opinions on the -stable desirability?

I think that it would be a good idea to add it to stable.

David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data  can be mapped directly
  2009-09-24 14:13 [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly David Howells
                   ` (2 preceding siblings ...)
  2009-09-25  0:43 ` Paul Mundt
@ 2009-09-25  4:39 ` graff yang
  2009-09-25  8:24   ` David Howells
  3 siblings, 1 reply; 10+ messages in thread
From: graff yang @ 2009-09-25  4:39 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, akpm, linux-kernel, Pekka Enberg, Paul Mundt,
	Mel Gorman, Greg Ungerer

On Thu, Sep 24, 2009 at 10:13 PM, David Howells <dhowells@redhat.com> 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@redhat.com>
>        Date:   Sat Sep 5 11:17:07 2009 -0700
>        nommu: fix error handling in do_mmap_pgoff()
>
> Reported-by: Graff Yang <graff.yang@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Cc: Pekka Enberg <penberg@cs.helsinki.fi>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Greg Ungerer <gerg@snapgear.com>
> ---
>
>  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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly
  2009-09-25  4:39 ` graff yang
@ 2009-09-25  8:24   ` David Howells
  2009-09-25 11:05     ` graff yang
  0 siblings, 1 reply; 10+ messages in thread
From: David Howells @ 2009-09-25  8:24 UTC (permalink / raw)
  To: graff yang
  Cc: dhowells, torvalds, akpm, linux-kernel, Pekka Enberg, Paul Mundt,
	Mel Gorman, Greg Ungerer

graff yang <graff.yang@gmail.com> wrote:

> > +       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.

Then they won't work on NOMMU.  Read Documentation/nommu-mmap.txt:

	============================================
	PROVIDING SHAREABLE CHARACTER DEVICE SUPPORT
	============================================

	To provide shareable character device support, a driver must provide a
	file->f_op->get_unmapped_area() operation. The mmap() routines will
	call this to get a proposed address for the mapping. This may return an
	error if it doesn't wish to honour the mapping because it's too long,
	at a weird offset, under some unsupported combination of flags or
	whatever.

The chardev driver doesn't provide the virtual address through its mmap() fop
in MMU-mode either - that's provided by do_mmap_pgoff().

David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data  can be mapped directly
  2009-09-25  8:24   ` David Howells
@ 2009-09-25 11:05     ` graff yang
  0 siblings, 0 replies; 10+ messages in thread
From: graff yang @ 2009-09-25 11:05 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, akpm, linux-kernel, Pekka Enberg, Paul Mundt,
	Mel Gorman, Greg Ungerer

On Fri, Sep 25, 2009 at 4:24 PM, David Howells <dhowells@redhat.com> wrote:
> graff yang <graff.yang@gmail.com> wrote:
>
>> > +       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.
>
> Then they won't work on NOMMU.  Read Documentation/nommu-mmap.txt:
>
>        ============================================
>        PROVIDING SHAREABLE CHARACTER DEVICE SUPPORT
>        ============================================
>
>        To provide shareable character device support, a driver must provide a
>        file->f_op->get_unmapped_area() operation. The mmap() routines will
>        call this to get a proposed address for the mapping. This may return an
>        error if it doesn't wish to honour the mapping because it's too long,
>        at a weird offset, under some unsupported combination of flags or
>        whatever.
>
> The chardev driver doesn't provide the virtual address through its mmap() fop
> in MMU-mode either - that's provided by do_mmap_pgoff().
>
> David
>
OK, I think do_mmap_pgoff() now looks nice with your patch.   Merely,
at least the alsa/fb driver
will crash due to the vm_region changes.
The alsa guys are fixing this issue:
http://mailman.alsa-project.org/pipermail/alsa-devel/2009-March/015802.html

Is there any other things be affected?

-- 
-Graff

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-09-25 11:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-24 14:13 [PATCH] NOMMU: Fix MAP_PRIVATE mmap() of objects where the data can be mapped directly David Howells
2009-09-24 19:59 ` Pekka Enberg
2009-09-24 22:09 ` Andrew Morton
2009-09-25  0:39   ` David Howells
2009-09-25  1:01     ` Andrew Morton
2009-09-25  1:17       ` David Howells
2009-09-25  0:43 ` Paul Mundt
2009-09-25  4:39 ` graff yang
2009-09-25  8:24   ` David Howells
2009-09-25 11:05     ` graff yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox