diff options
author | Hugh Dickins <hugh@veritas.com> | 2008-12-10 15:48:52 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2008-12-10 17:40:45 -0500 |
commit | b88ed20594db2c685555b68c52b693b75738b2f5 (patch) | |
tree | a8edcf975fb8437692bf670841859d4ac9fbd2f0 | |
parent | f4fd2c5b6f691ff71614acbc28461b3c1488c81b (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.c | 15 |
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 | /* |