Linux PARISC architecture development
 help / color / mirror / Atom feed
* Fwd: [PATCH] fix mapping_writably_mapped()
@ 2008-12-10 21:46 Kyle McMartin
  2008-12-10 21:47 ` Matthew Wilcox
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Kyle McMartin @ 2008-12-10 21:46 UTC (permalink / raw)
  To: linux-parisc

This may explain some of the userspace issues we've been seeing.

----- Forwarded message from Hugh Dickins <hugh@veritas.com> -----

Sender: linux-arch-owner@vger.kernel.org
From: Hugh Dickins <hugh@veritas.com>
Subject: [PATCH] fix mapping_writably_mapped()
To: Linus Torvalds <torvalds@linux-foundation.org>
cc: Andrew Morton <akpm@linux-foundation.org>,
	Lee Schermerhorn <Lee.Schermerhorn@hp.com>, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	stable@kernel.org
Date: Wed, 10 Dec 2008 20:48:52 +0000 (GMT)
Message-ID: <Pine.LNX.4.64.0812102043060.25282@blonde.anvils>

Lee Schermerhorn noticed yesterday that I broke the mapping_writably_mapped
test in 2.6.7!  Bad bad bug, good good find.

The i_mmap_writable count must be incremented for VM_SHARED (just as
i_writecount is for VM_DENYWRITE, but while holding the i_mmap_lock)
when dup_mmap() copies the vma for fork: it has its own more optimal
version of __vma_link_file(), and I missed this out.  So the count
was later going down to 0 (dangerous) when one end unmapped, then
wrapping negative (inefficient) when the other end unmapped.

The only impact on x86 would have been that setting a mandatory lock on
a file which has at some time been opened O_RDWR and mapped MAP_SHARED
(but not necessarily PROT_WRITE) across a fork, might fail with -EAGAIN
when it should succeed, or succeed when it should fail.

But those architectures which rely on flush_dcache_page() to flush
userspace modifications back into the page before the kernel reads it,
may in some cases have skipped the flush after such a fork - though any
repetitive test will soon wrap the count negative, in which case it will
flush_dcache_page() unnecessarily.

Fix would be a two-liner, but mapping variable added, and comment moved.

Reported-by: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

 kernel/fork.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

--- 2.6.28-rc7/kernel/fork.c	2008-11-15 23:09:30.000000000 +0000
+++ linux/kernel/fork.c	2008-12-10 12:49:13.000000000 +0000
@@ -315,17 +315,20 @@ static int dup_mmap(struct mm_struct *mm
 		file = tmp->vm_file;
 		if (file) {
 			struct inode *inode = file->f_path.dentry->d_inode;
+			struct address_space *mapping = file->f_mapping;
+
 			get_file(file);
 			if (tmp->vm_flags & VM_DENYWRITE)
 				atomic_dec(&inode->i_writecount);

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

end of thread, other threads:[~2008-12-13 16:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-10 21:46 Fwd: [PATCH] fix mapping_writably_mapped() Kyle McMartin
2008-12-10 21:47 ` Matthew Wilcox
2008-12-11  2:26 ` [RFC] Fix warnings from various parisc and hp specific files John David Anglin
2008-12-11  2:34   ` John David Anglin
2008-12-11  8:29 ` Fwd: [PATCH] fix mapping_writably_mapped() Grant Grundler
2008-12-11 12:52   ` John David Anglin
2008-12-11 21:31 ` Helge Deller
2008-12-11 22:03   ` James Bottomley
2008-12-13 16:36 ` John David Anglin

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