diff mbox series

[1/3] btrfs: abort transaction when sibling keys check fails for leaves

Message ID df0094379bdfae431142657c27dd00a854a4b402.1682505780.git.fdmanana@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: improve sibling keys check failure report | expand

Commit Message

Filipe Manana April 26, 2023, 10:51 a.m. UTC 
From: Filipe Manana <fdmanana@suse.com>

If the sibling keys check fails before we move keys from one sibling
leaf to another, we are not aborting the transaction - we leave that to
some higher level caller of btrfs_search_slot() (or anything else that
uses it to insert items into a b+tree).

This means that the transaction abort will provide a stack trace that
omits the b+tree modification call chain. So change this to immediately
abort the transaction and therefore get a more useful stack trace that
shows us the call chain in the bt+tree modification code.

It's also important to immediately abort the transaction just in case
some higher level caller is not doing it, as this indicates a very
serious corruption and we should stop the possibility of doing further
damage.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ctree.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Qu Wenruo April 26, 2023, 11:18 a.m. UTC | #1 
On 2023/4/26 18:51, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If the sibling keys check fails before we move keys from one sibling
> leaf to another, we are not aborting the transaction - we leave that to
> some higher level caller of btrfs_search_slot() (or anything else that
> uses it to insert items into a b+tree).
> 
> This means that the transaction abort will provide a stack trace that
> omits the b+tree modification call chain. So change this to immediately
> abort the transaction and therefore get a more useful stack trace that
> shows us the call chain in the bt+tree modification code.
> 
> It's also important to immediately abort the transaction just in case
> some higher level caller is not doing it, as this indicates a very
> serious corruption and we should stop the possibility of doing further
> damage.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/ctree.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index d94429e0f16a..a0b97a6d075a 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -3296,6 +3296,7 @@ static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root
>   
>   	if (check_sibling_keys(left, right)) {
>   		ret = -EUCLEAN;
> +		btrfs_abort_transaction(trans, ret);
>   		btrfs_tree_unlock(right);
>   		free_extent_buffer(right);
>   		return ret;
> @@ -3514,6 +3515,7 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root
>   
>   	if (check_sibling_keys(left, right)) {
>   		ret = -EUCLEAN;
> +		btrfs_abort_transaction(trans, ret);
>   		goto out;
>   	}
>   	return __push_leaf_left(trans, path, min_data_size, empty, left,
diff mbox series

Patch

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index d94429e0f16a..a0b97a6d075a 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -3296,6 +3296,7 @@  static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root
 
 	if (check_sibling_keys(left, right)) {
 		ret = -EUCLEAN;
+		btrfs_abort_transaction(trans, ret);
 		btrfs_tree_unlock(right);
 		free_extent_buffer(right);
 		return ret;
@@ -3514,6 +3515,7 @@  static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root
 
 	if (check_sibling_keys(left, right)) {
 		ret = -EUCLEAN;
+		btrfs_abort_transaction(trans, ret);
 		goto out;
 	}
 	return __push_leaf_left(trans, path, min_data_size, empty, left,