lkml.org 
[lkml]   [2019]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 131/170] Btrfs: send, fix race with transaction commits that create snapshots
    Date
    4.19-stable review patch.  If anyone has any objections, please let me know.

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

    From: Filipe Manana <fdmanana@suse.com>

    commit be6821f82c3cc36e026f5afd10249988852b35ea upstream.

    If we create a snapshot of a snapshot currently being used by a send
    operation, we can end up with send failing unexpectedly (returning
    -ENOENT error to user space for example). The following diagram shows
    how this happens.

    CPU 1 CPU2 CPU3

    btrfs_ioctl_send()
    (...)
    create_snapshot()
    -> creates snapshot of a
    root used by the send
    task
    btrfs_commit_transaction()
    create_pending_snapshot()
    __get_inode_info()
    btrfs_search_slot()
    btrfs_search_slot_get_root()
    down_read commit_root_sem

    get reference on eb of the
    commit root
    -> eb with bytenr == X

    up_read commit_root_sem

    btrfs_cow_block(root node)
    btrfs_free_tree_block()
    -> creates delayed ref to
    free the extent

    btrfs_run_delayed_refs()
    -> runs the delayed ref,
    adds extent to
    fs_info->pinned_extents

    btrfs_finish_extent_commit()
    unpin_extent_range()
    -> marks extent as free
    in the free space cache

    transaction commit finishes

    btrfs_start_transaction()
    (...)
    btrfs_cow_block()
    btrfs_alloc_tree_block()
    btrfs_reserve_extent()
    -> allocates extent at
    bytenr == X
    btrfs_init_new_buffer(bytenr X)
    btrfs_find_create_tree_block()
    alloc_extent_buffer(bytenr X)
    find_extent_buffer(bytenr X)
    -> returns existing eb,
    which the send task got

    (...)
    -> modifies content of the
    eb with bytenr == X

    -> uses an eb that now
    belongs to some other
    tree and no more matches
    the commit root of the
    snapshot, resuts will be
    unpredictable

    The consequences of this race can be various, and can lead to searches in
    the commit root performed by the send task failing unexpectedly (unable to
    find inode items, returning -ENOENT to user space, for example) or not
    failing because an inode item with the same number was added to the tree
    that reused the metadata extent, in which case send can behave incorrectly
    in the worst case or just fail later for some reason.

    Fix this by performing a copy of the commit root's extent buffer when doing
    a search in the context of a send operation.

    CC: stable@vger.kernel.org # 4.4.x: 1fc28d8e2e9: Btrfs: move get root out of btrfs_search_slot to a helper
    CC: stable@vger.kernel.org # 4.4.x: f9ddfd0592a: Btrfs: remove unused check of skip_locking
    CC: stable@vger.kernel.org # 4.4.x
    Signed-off-by: Filipe Manana <fdmanana@suse.com>
    Signed-off-by: David Sterba <dsterba@suse.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/btrfs/ctree.c | 29 +++++++++++++++++++++++------
    1 file changed, 23 insertions(+), 6 deletions(-)

    --- a/fs/btrfs/ctree.c
    +++ b/fs/btrfs/ctree.c
    @@ -2624,14 +2624,27 @@ static struct extent_buffer *btrfs_searc
    root_lock = BTRFS_READ_LOCK;

    if (p->search_commit_root) {
    - /* The commit roots are read only so we always do read locks */
    - if (p->need_commit_sem)
    + /*
    + * The commit roots are read only so we always do read locks,
    + * and we always must hold the commit_root_sem when doing
    + * searches on them, the only exception is send where we don't
    + * want to block transaction commits for a long time, so
    + * we need to clone the commit root in order to avoid races
    + * with transaction commits that create a snapshot of one of
    + * the roots used by a send operation.
    + */
    + if (p->need_commit_sem) {
    down_read(&fs_info->commit_root_sem);
    - b = root->commit_root;
    - extent_buffer_get(b);
    - level = btrfs_header_level(b);
    - if (p->need_commit_sem)
    + b = btrfs_clone_extent_buffer(root->commit_root);
    up_read(&fs_info->commit_root_sem);
    + if (!b)
    + return ERR_PTR(-ENOMEM);
    +
    + } else {
    + b = root->commit_root;
    + extent_buffer_get(b);
    + }
    + level = btrfs_header_level(b);
    /*
    * Ensure that all callers have set skip_locking when
    * p->search_commit_root = 1.
    @@ -2757,6 +2770,10 @@ int btrfs_search_slot(struct btrfs_trans
    again:
    prev_cmp = -1;
    b = btrfs_search_slot_get_root(root, p, write_lock_level);
    + if (IS_ERR(b)) {
    + ret = PTR_ERR(b);
    + goto done;
    + }

    while (b) {
    level = btrfs_header_level(b);

    \
     
     \ /
      Last update: 2019-01-07 18:25    [W:5.157 / U:0.668 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site