lkml.org 
[lkml]   [2011]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[173/264] ext4: fix race in xattr block allocation path
    3.1-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Eric Sandeen <sandeen@redhat.com>

    commit 6d6a435190bdf2e04c9465cde5bdc3ac68cf11a4 upstream.

    Ceph users reported that when using Ceph on ext4, the filesystem
    would often become corrupted, containing inodes with incorrect
    i_blocks counters.

    I managed to reproduce this with a very hacked-up "streamtest"
    binary from the Ceph tree.

    Ceph is doing a lot of xattr writes, to out-of-inode blocks.
    There is also another thread which does sync_file_range and close,
    of the same files. The problem appears to happen due to this race:

    sync/flush thread xattr-set thread
    ----------------- ----------------

    do_writepages ext4_xattr_set
    ext4_da_writepages ext4_xattr_set_handle
    mpage_da_map_blocks ext4_xattr_block_set
    set DELALLOC_RESERVE
    ext4_new_meta_blocks
    ext4_mb_new_blocks
    if (!i_delalloc_reserved_flag)
    vfs_dq_alloc_block
    ext4_get_blocks
    down_write(i_data_sem)
    set i_delalloc_reserved_flag
    ...
    up_write(i_data_sem)
    if (i_delalloc_reserved_flag)
    vfs_dq_alloc_block_nofail


    In other words, the sync/flush thread pops in and sets
    i_delalloc_reserved_flag on the inode, which makes the xattr thread
    think that it's in a delalloc path in ext4_new_meta_blocks(),
    and add the block for a second time, after already having added
    it once in the !i_delalloc_reserved_flag case in ext4_mb_new_blocks

    The real problem is that we shouldn't be using the DELALLOC_RESERVED
    state flag, and instead we should be passing
    EXT4_GET_BLOCKS_DELALLOC_RESERVE down to ext4_map_blocks() instead of
    using an inode state flag. We'll fix this for now with using
    i_data_sem to prevent this race, but this is really not the right way
    to fix things.

    Signed-off-by: Eric Sandeen <sandeen@redhat.com>
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

    ---
    fs/ext4/xattr.c | 6 ++++++
    1 file changed, 6 insertions(+)

    --- a/fs/ext4/xattr.c
    +++ b/fs/ext4/xattr.c
    @@ -820,8 +820,14 @@ inserted:
    if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
    goal = goal & EXT4_MAX_BLOCK_FILE_PHYS;

    + /*
    + * take i_data_sem because we will test
    + * i_delalloc_reserved_flag in ext4_mb_new_blocks
    + */
    + down_read((&EXT4_I(inode)->i_data_sem));
    block = ext4_new_meta_blocks(handle, inode, goal, 0,
    NULL, &error);
    + up_read((&EXT4_I(inode)->i_data_sem));
    if (error)
    goto cleanup;




    \
     
     \ /
      Last update: 2011-11-10 05:13    [W:2.218 / U:1.808 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site