[lkml]   [2008]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    Subject[patch 76/83] fix mapping_writably_mapped()
    2.6.27-stable review patch.  If anyone has any objections, please let us know.


    From: Hugh Dickins <>

    commit b88ed20594db2c685555b68c52b693b75738b2f5 upstream.

    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 <>
    Signed-off-by: Hugh Dickins <>
    Signed-off-by: Linus Torvalds <>
    Signed-off-by: Greg Kroah-Hartman <>

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

    --- a/kernel/fork.c
    +++ b/kernel/fork.c
    @@ -313,17 +313,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;
    if (tmp->vm_flags & VM_DENYWRITE)
    - /* insert tmp into the share list, just after mpnt */
    - spin_lock(&file->f_mapping->i_mmap_lock);
    + spin_lock(&mapping->i_mmap_lock);
    + if (tmp->vm_flags & VM_SHARED)
    + mapping->i_mmap_writable++;
    tmp->vm_truncate_count = mpnt->vm_truncate_count;
    - flush_dcache_mmap_lock(file->f_mapping);
    + flush_dcache_mmap_lock(mapping);
    + /* insert tmp into the share list, just after mpnt */
    vma_prio_tree_add(tmp, mpnt);
    - flush_dcache_mmap_unlock(file->f_mapping);
    - spin_unlock(&file->f_mapping->i_mmap_lock);
    + flush_dcache_mmap_unlock(mapping);
    + spin_unlock(&mapping->i_mmap_lock);


     \ /
      Last update: 2008-12-11 20:49    [W:0.021 / U:66.920 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site