On 16.04.19 г. 10:15 ч., Qu Wenruo wrote:
> [BUG]
> When running fuzz-tests/003 and fuzz-tests/009, btrfs-progs will crash
> due to BUG_ON().
>
> [CAUSE]
> We abused BUG_ON() in btrfs_commit_transaction(), which is one of the
> most error prone function for fuzzed images.
>
> Currently to cleanup the aborted transaction, we only need to clean up
> the only per-transaction data: delayed refs.
>
> This patch will introduce a new function, btrfs_destroy_delayed_refs()
> to cleanup delayed refs when we failed to commit transaction.
>
> With that function, we will gently destroy per-trans delayed ref, and
> remove the BUG_ON()s in btrfs_commit_transaction().
>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>
Though see below for a suggestion of further cleanup
> ---
> delayed-ref.c | 24 ++++++++++++++++++++++++
> delayed-ref.h | 5 +++++
> extent-tree.c | 6 +++---
> transaction.c | 20 ++++++++++++++------
> 4 files changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/delayed-ref.c b/delayed-ref.c
> index 9974dbbd4c6b..69f8be34fe18 100644
> --- a/delayed-ref.c
> +++ b/delayed-ref.c
> @@ -605,3 +605,27 @@ free_ref:
>
> return -ENOMEM;
> }
> +
> +void btrfs_destroy_delayed_refs(struct btrfs_trans_handle *trans)
> +{
> + struct btrfs_fs_info *fs_info = trans->fs_info;
> + struct rb_node *node;
> + struct btrfs_delayed_ref_root *delayed_refs;
> +
> + delayed_refs = &trans->delayed_refs;
> + if (RB_EMPTY_ROOT(&delayed_refs->href_root))
> + return;
> + while ((node = rb_first(&delayed_refs->href_root)) != NULL) {
> + struct btrfs_delayed_ref_head *head;
> + struct btrfs_delayed_ref_node *ref;
> + struct rb_node *n;
> +
> + head = rb_entry(node, struct btrfs_delayed_ref_head, href_node);
> + while ((n = rb_first(&head->ref_tree)) != NULL) {
> + ref = rb_entry(n, struct btrfs_delayed_ref_node,
> + ref_node);
> + drop_delayed_ref(trans, delayed_refs, head, ref);
> + }
> + ASSERT(cleanup_ref_head(trans, fs_info, head) == 0);
> + }
> +}
> diff --git a/delayed-ref.h b/delayed-ref.h
> index 30a68b2a278c..4ff3eaa929ba 100644
> --- a/delayed-ref.h
> +++ b/delayed-ref.h
> @@ -205,4 +205,9 @@ btrfs_delayed_node_to_tree_ref(struct btrfs_delayed_ref_node *node)
> {
> return container_of(node, struct btrfs_delayed_tree_ref, node);
> }
> +
> +int cleanup_ref_head(struct btrfs_trans_handle *trans,
> + struct btrfs_fs_info *fs_info,
> + struct btrfs_delayed_ref_head *head);
> +void btrfs_destroy_delayed_refs(struct btrfs_trans_handle *trans);
> #endif
> diff --git a/extent-tree.c b/extent-tree.c
> index 1140ebfc61ff..e62ee8c2ba13 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -4085,9 +4085,9 @@ static void unselect_delayed_ref_head(struct btrfs_delayed_ref_root *delayed_ref
> delayed_refs->num_heads_ready++;
> }
>
> -static int cleanup_ref_head(struct btrfs_trans_handle *trans,
> - struct btrfs_fs_info *fs_info,
> - struct btrfs_delayed_ref_head *head)
> +int cleanup_ref_head(struct btrfs_trans_handle *trans,
> + struct btrfs_fs_info *fs_info,
> + struct btrfs_delayed_ref_head *head)
You can create a followup patch that removes the fs_info argument from
cleanup_ref_head to make it identical to the kernel version.
> {
> struct btrfs_delayed_ref_root *delayed_refs;
>
> diff --git a/transaction.c b/transaction.c
> index 8d15eb11c7b5..138e10f0d6cc 100644
> --- a/transaction.c
> +++ b/transaction.c
> @@ -17,6 +17,7 @@
> #include "kerncompat.h"
> #include "disk-io.h"
> #include "transaction.h"
> +#include "delayed-ref.h"
>
> #include "messages.h"
>
> @@ -165,7 +166,8 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
> * consistent
> */
> ret = btrfs_run_delayed_refs(trans, -1);
> - BUG_ON(ret);
> + if (ret < 0)
> + goto error;
>
> if (root->commit_root == root->node)
> goto commit_tree;
> @@ -182,21 +184,24 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
> root->root_item.level = btrfs_header_level(root->node);
> ret = btrfs_update_root(trans, root->fs_info->tree_root,
> &root->root_key, &root->root_item);
> - BUG_ON(ret);
> + if (ret < 0)
> + goto error;
>
> commit_tree:
> ret = commit_tree_roots(trans, fs_info);
> - BUG_ON(ret);
> + if (ret < 0)
> + goto error;
> /*
> * Ensure that all committed roots are properly accounted in the
> * extent tree
> */
> ret = btrfs_run_delayed_refs(trans, -1);
> - BUG_ON(ret);
> + if (ret < 0)
> + goto error;
> btrfs_write_dirty_block_groups(trans);
> __commit_transaction(trans, root);
> if (ret < 0)
> - goto out;
> + goto error;
> ret = write_ctree_super(trans);
> btrfs_finish_extent_commit(trans);
> kfree(trans);
> @@ -204,7 +209,10 @@ commit_tree:
> root->commit_root = NULL;
> fs_info->running_transaction = NULL;
> fs_info->last_trans_committed = transid;
> -out:
> + return ret;
> +error:
> + btrfs_destroy_delayed_refs(trans);
> + free(trans);
> return ret;
> }
>
>
