aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHugh Dickins <hugh@veritas.com>2008-12-10 15:48:52 -0500
committerLinus Torvalds <torvalds@linux-foundation.org>2008-12-10 17:40:45 -0500
commitb88ed20594db2c685555b68c52b693b75738b2f5 (patch)
treea8edcf975fb8437692bf670841859d4ac9fbd2f0
parentf4fd2c5b6f691ff71614acbc28461b3c1488c81b (diff)
fix mapping_writably_mapped()
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> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--kernel/fork.c15
1 files changed, 9 insertions, 6 deletions
diff --git a/kernel/fork.c b/kernel/fork.c
index 8d6a7dd9282b..495da2e9a8b4 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -315,17 +315,20 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
315 file = tmp->vm_file; 315 file = tmp->vm_file;
316 if (file) { 316 if (file) {
317 struct inode *inode = file->f_path.dentry->d_inode; 317 struct inode *inode = file->f_path.dentry->d_inode;
318 struct address_space *mapping = file->f_mapping;
319
318 get_file(file); 320 get_file(file);
319 if (tmp->vm_flags & VM_DENYWRITE) 321 if (tmp->vm_flags & VM_DENYWRITE)
320 atomic_dec(&inode->i_writecount); 322 atomic_dec(&inode->i_writecount);
321 323 spin_lock(&mapping->i_mmap_lock);
322 /* insert tmp into the share list, just after mpnt */ 324 if (tmp->vm_flags & VM_SHARED)
323 spin_lock(&file->f_mapping->i_mmap_lock); 325 mapping->i_mmap_writable++;
324 tmp->vm_truncate_count = mpnt->vm_truncate_count; 326 tmp->vm_truncate_count = mpnt->vm_truncate_count;
325 flush_dcache_mmap_lock(file->f_mapping); 327 flush_dcache_mmap_lock(mapping);
328 /* insert tmp into the share list, just after mpnt */
326 vma_prio_tree_add(tmp, mpnt); 329 vma_prio_tree_add(tmp, mpnt);
327 flush_dcache_mmap_unlock(file->f_mapping); 330 flush_dcache_mmap_unlock(mapping);
328 spin_unlock(&file->f_mapping->i_mmap_lock); 331 spin_unlock(&mapping->i_mmap_lock);
329 } 332 }
330 333
331 /* 334 /*