linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: 2.5.55-rmk1: user space lossage
       [not found] <000001c2c287$ffa8eef0$800b040f@bergamot>
@ 2003-01-23  9:48 ` David Woodhouse
  2003-01-23  9:56   ` Andrew Morton
  2003-01-23 10:01   ` David Woodhouse
  0 siblings, 2 replies; 5+ messages in thread
From: David Woodhouse @ 2003-01-23  9:48 UTC (permalink / raw)
  To: Christopher Hoover
  Cc: 'Frank Becker', linux-arm-kernel, 'linux-mtd',
	akpm, linux-kernel


 < Snip long thread about init segfaulting immediately at boot on 2.5.55 >

ch@murgatroid.com said:
> I just dropped jffs2 from 2.5.52 into 2.5.55 and it works, too.

ch@murgatroid.com said:
> Aha!  This is the problem: 
> -       .mmap =         generic_file_mmap,
> +       .mmap =         generic_file_readonly_mmap,
> If you reverese this change, 2.5.55-rmk1 behaves.

Er, yes. generic_file_readonly_mmap() silently removed the VM_MAYWRITE bit 
from vma->vm_flags when init made a _PRIVATE_ writable mapping, apparently 
on the basis that we have no writepage().

Then we return success anyway.

Then init segfaults when it touches something in that mapping.

/me blames akpm. :)

--
dwmw2



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

* Re: 2.5.55-rmk1: user space lossage
  2003-01-23  9:48 ` 2.5.55-rmk1: user space lossage David Woodhouse
@ 2003-01-23  9:56   ` Andrew Morton
  2003-01-23 10:01   ` David Woodhouse
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2003-01-23  9:56 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ch, fbecker, linux-arm-kernel, linux-mtd, linux-kernel

David Woodhouse <dwmw2@infradead.org> wrote:
>
> /me blames akpm. :)

Linus did it!

diff -puN mm/filemap.c~generic_file_readonly_mmap-fix mm/filemap.c
--- 25/mm/filemap.c~generic_file_readonly_mmap-fix	2003-01-23 01:55:41.000000000 -0800
+++ 25-akpm/mm/filemap.c	2003-01-23 01:55:45.000000000 -0800
@@ -1312,7 +1312,6 @@ int generic_file_readonly_mmap(struct fi
 {
 	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
 		return -EINVAL;
-	vma->vm_flags &= ~VM_MAYWRITE;
 	return generic_file_mmap(file, vma);
 }
 #else

_


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

* Re: 2.5.55-rmk1: user space lossage
  2003-01-23  9:48 ` 2.5.55-rmk1: user space lossage David Woodhouse
  2003-01-23  9:56   ` Andrew Morton
@ 2003-01-23 10:01   ` David Woodhouse
  2003-01-23 10:06     ` Andrew Morton
  1 sibling, 1 reply; 5+ messages in thread
From: David Woodhouse @ 2003-01-23 10:01 UTC (permalink / raw)
  To: Andrew Morton; +Cc: ch, fbecker, linux-arm-kernel, linux-mtd, linux-kernel


akpm@digeo.com said:
> Linus did it!

> diff -puN mm/filemap.c~generic_file_readonly_mmap-fix mm/filemap.c
> --- 25/mm/filemap.c~generic_file_readonly_mmap-fix	2003-01-23 01:55:41 -0800
> +++ 25-akpm/mm/filemap.c	2003-01-23 01:55:45 -0800
> @@ -1312,7 +1312,6 @@ int generic_file_readonly_mmap(struct fi
>  {
>  	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
> 		return -EINVAL;
> - 	vma->vm_flags &= ~VM_MAYWRITE;
>  	return generic_file_mmap(file, vma);
>  }
>  #else


-  	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
+-  	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
++ 	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))



...?

--
dwmw2



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

* Re: 2.5.55-rmk1: user space lossage
  2003-01-23 10:01   ` David Woodhouse
@ 2003-01-23 10:06     ` Andrew Morton
  2003-01-23 17:57       ` Hugh Dickins
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2003-01-23 10:06 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ch, fbecker, linux-arm-kernel, linux-mtd, linux-kernel

David Woodhouse <dwmw2@infradead.org> wrote:
>
> 
> 
> -  	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
> +-  	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
> ++ 	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))

Yup.



We cannot clear VM_MAYWRITE in there - it turns writeable MAP_PRIVATE
mappings into readonly ones.

So change it back to the 2.4 form - disallow a writeable MAP_SHARED mapping
against filesystems which do no implement ->writepage().


 filemap.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff -puN mm/filemap.c~generic_file_readonly_mmap-fix mm/filemap.c
--- 25/mm/filemap.c~generic_file_readonly_mmap-fix	2003-01-23 01:55:41.000000000 -0800
+++ 25-akpm/mm/filemap.c	2003-01-23 02:04:05.000000000 -0800
@@ -1308,11 +1308,13 @@ int generic_file_mmap(struct file * file
 	return 0;
 }
 
+/*
+ * This is for filesystems which do not implement ->writepage.
+ */
 int generic_file_readonly_mmap(struct file *file, struct vm_area_struct *vma)
 {
-	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
+	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
 		return -EINVAL;
-	vma->vm_flags &= ~VM_MAYWRITE;
 	return generic_file_mmap(file, vma);
 }
 #else

_


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

* Re: 2.5.55-rmk1: user space lossage
  2003-01-23 10:06     ` Andrew Morton
@ 2003-01-23 17:57       ` Hugh Dickins
  0 siblings, 0 replies; 5+ messages in thread
From: Hugh Dickins @ 2003-01-23 17:57 UTC (permalink / raw)
  To: Andrew Morton; +Cc: David Woodhouse, linux-kernel

On Thu, 23 Jan 2003, Andrew Morton wrote:
> 
> We cannot clear VM_MAYWRITE in there - it turns writeable MAP_PRIVATE
> mappings into readonly ones.
> 
> So change it back to the 2.4 form - disallow a writeable MAP_SHARED mapping
> against filesystems which do no implement ->writepage().

Sorry for late quibbles, but wouldn't patch below be better?  Linus
did intend that change to the occasionally criticized 2.4 behaviour,
this preserves that change while avoiding the bug which hit David.

You may notice I've sneaked a ~VM_SHARED in there too.  Not because
I can say it's necessary, but the VM_MAYWRITE, VM_SHARED treatment
is already weird, I'd feel safer if we keep to the same combination
as already exists in the !FMODE_WRITE case (see do_mmap_pgoff).

Hugh

--- 2.5.59-mm2/mm/filemap.c	Sat Jan 18 10:37:36 2003
+++ linux/mm/filemap.c	Thu Jan 23 16:50:34 2003
@@ -1310,9 +1310,11 @@
 
 int generic_file_readonly_mmap(struct file *file, struct vm_area_struct *vma)
 {
-	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
-		return -EINVAL;
-	vma->vm_flags &= ~VM_MAYWRITE;
+	if (vma->vm_flags & VM_SHARED) {
+		if (vma->vm_flags & VM_WRITE)
+			return -EINVAL;
+		vma->vm_flags &= ~(VM_MAYWRITE | VM_SHARED);
+	}
 	return generic_file_mmap(file, vma);
 }
 #else


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

end of thread, other threads:[~2003-01-23 17:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <000001c2c287$ffa8eef0$800b040f@bergamot>
2003-01-23  9:48 ` 2.5.55-rmk1: user space lossage David Woodhouse
2003-01-23  9:56   ` Andrew Morton
2003-01-23 10:01   ` David Woodhouse
2003-01-23 10:06     ` Andrew Morton
2003-01-23 17:57       ` Hugh Dickins

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).