[lkml]   [2020]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    Subject[PATCH 3.16 102/148] Btrfs: fix negative subv_writers counter and data space leak after buffered write
    3.16.82-rc1 review patch.  If anyone has any objections, please let me know.


    From: Filipe Manana <>

    commit a0e248bb502d5165b3314ac3819e888fdcdf7d9f upstream.

    When doing a buffered write it's possible to leave the subv_writers
    counter of the root, used for synchronization between buffered nocow
    writers and snapshotting. This happens in an exceptional case like the

    1) We fail to allocate data space for the write, since there's not
    enough available data space nor enough unallocated space for allocating
    a new data block group;

    2) Because of that failure, we try to go to NOCOW mode, which succeeds
    and therefore we set the local variable 'only_release_metadata' to true
    and set the root's sub_writers counter to 1 through the call to
    btrfs_start_write_no_snapshotting() made by check_can_nocow();

    3) The call to btrfs_copy_from_user() returns zero, which is very unlikely
    to happen but not impossible;

    4) No pages are copied because btrfs_copy_from_user() returned zero;

    5) We call btrfs_end_write_no_snapshotting() which decrements the root's
    subv_writers counter to 0;

    6) We don't set 'only_release_metadata' back to 'false' because we do
    it only if 'copied', the value returned by btrfs_copy_from_user(), is
    greater than zero;

    7) On the next iteration of the while loop, which processes the same
    page range, we are now able to allocate data space for the write (we
    got enough data space released in the meanwhile);

    8) After this if we fail at btrfs_delalloc_reserve_metadata(), because
    now there isn't enough free metadata space, or in some other place
    further below (prepare_pages(), lock_and_cleanup_extent_if_need(),
    btrfs_dirty_pages()), we break out of the while loop with
    'only_release_metadata' having a value of 'true';

    9) Because 'only_release_metadata' is 'true' we end up decrementing the
    root's subv_writers counter to -1 (through a call to
    btrfs_end_write_no_snapshotting()), and we also end up not releasing the
    data space previously reserved through btrfs_check_data_free_space().
    As a consequence the mechanism for synchronizing NOCOW buffered writes
    with snapshotting gets broken.

    Fix this by always setting 'only_release_metadata' to false at the start
    of each iteration.

    Fixes: 8257b2dc3c1a ("Btrfs: introduce btrfs_{start, end}_nocow_write() for each subvolume")
    Fixes: 7ee9e4405f26 ("Btrfs: check if we can nocow if we don't have data space")
    Reviewed-by: Josef Bacik <>
    Signed-off-by: Filipe Manana <>
    Reviewed-by: David Sterba <>
    Signed-off-by: David Sterba <>
    [bwh: Backported to 3.16: adjust context]
    Signed-off-by: Ben Hutchings <>
    fs/btrfs/file.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    --- a/fs/btrfs/file.c
    +++ b/fs/btrfs/file.c
    @@ -1540,6 +1540,7 @@ static noinline ssize_t __btrfs_buffered

    + only_release_metadata = false;
    reserve_bytes = num_pages << PAGE_CACHE_SHIFT;
    ret = btrfs_check_data_free_space(inode, reserve_bytes);
    if (ret == -ENOSPC &&
    @@ -1671,7 +1672,6 @@ again:
    set_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
    - only_release_metadata = false;

    btrfs_drop_pages(pages, num_pages);
     \ /
      Last update: 2020-02-08 19:36    [W:2.434 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site