From 9cf14029d5fb42126b332aea708b787be9a5079e Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Tue, 7 Feb 2023 11:57:21 -0500 Subject: [PATCH] btrfs: handle errors from btrfs_read_node_slot in split While investigating a problem with error injection I tripped over curious behavior in the node/leaf splitting code. If we get an EIO when trying to read either the left or right leaf/node for splitting we'll simply treat the node as if it were full and continue on. The end result of this isn't too bad, we simply end up allocating a block when we may have pushed items into the adjacent blocks. However this does essentially allow us to continue to modify a file system that we've gotten errors on, either from a bad disk or csum mismatch or other corruption. This isn't particularly safe, so instead handle these btrfs_read_node_slot() usages differently. We allow you to pass in any slot, the idea being that we save some code if the slot number is outside of the range of the parent. This means we treat all errors the same, when in reality we only want to ignore -ENOENT. Fix this by changing how we call btrfs_read_node_slot(), which is to only call it for slots we know are valid. This way if we get an error back from reading the block we can properly pass the error up the chain. This was validated with the error injection testing I was doing. Signed-off-by: Josef Bacik Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/ctree.c | 53 ++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index a1c109d7956a..e1045e6d5e84 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -1064,11 +1064,14 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, BTRFS_NODEPTRS_PER_BLOCK(fs_info) / 4) return 0; - left = btrfs_read_node_slot(parent, pslot - 1); - if (IS_ERR(left)) - left = NULL; + if (pslot) { + left = btrfs_read_node_slot(parent, pslot - 1); + if (IS_ERR(left)) { + ret = PTR_ERR(left); + left = NULL; + goto enospc; + } - if (left) { __btrfs_tree_lock(left, BTRFS_NESTING_LEFT); wret = btrfs_cow_block(trans, root, left, parent, pslot - 1, &left, @@ -1079,11 +1082,14 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, } } - right = btrfs_read_node_slot(parent, pslot + 1); - if (IS_ERR(right)) - right = NULL; + if (pslot + 1 < btrfs_header_nritems(parent)) { + right = btrfs_read_node_slot(parent, pslot + 1); + if (IS_ERR(right)) { + ret = PTR_ERR(right); + right = NULL; + goto enospc; + } - if (right) { __btrfs_tree_lock(right, BTRFS_NESTING_RIGHT); wret = btrfs_cow_block(trans, root, right, parent, pslot + 1, &right, @@ -1240,14 +1246,14 @@ static noinline int push_nodes_for_insert(struct btrfs_trans_handle *trans, if (!parent) return 1; - left = btrfs_read_node_slot(parent, pslot - 1); - if (IS_ERR(left)) - left = NULL; - /* first, try to make some room in the middle buffer */ - if (left) { + if (pslot) { u32 left_nr; + left = btrfs_read_node_slot(parent, pslot - 1); + if (IS_ERR(left)) + return PTR_ERR(left); + __btrfs_tree_lock(left, BTRFS_NESTING_LEFT); left_nr = btrfs_header_nritems(left); @@ -1292,16 +1298,17 @@ static noinline int push_nodes_for_insert(struct btrfs_trans_handle *trans, btrfs_tree_unlock(left); free_extent_buffer(left); } - right = btrfs_read_node_slot(parent, pslot + 1); - if (IS_ERR(right)) - right = NULL; /* * then try to empty the right most buffer into the middle */ - if (right) { + if (pslot + 1 < btrfs_header_nritems(parent)) { u32 right_nr; + right = btrfs_read_node_slot(parent, pslot + 1); + if (IS_ERR(right)) + return PTR_ERR(right); + __btrfs_tree_lock(right, BTRFS_NESTING_RIGHT); right_nr = btrfs_header_nritems(right); @@ -3198,12 +3205,8 @@ static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root btrfs_assert_tree_write_locked(path->nodes[1]); right = btrfs_read_node_slot(upper, slot + 1); - /* - * slot + 1 is not valid or we fail to read the right node, - * no big deal, just return. - */ if (IS_ERR(right)) - return 1; + return PTR_ERR(right); __btrfs_tree_lock(right, BTRFS_NESTING_RIGHT); @@ -3417,12 +3420,8 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root btrfs_assert_tree_write_locked(path->nodes[1]); left = btrfs_read_node_slot(path->nodes[1], slot - 1); - /* - * slot - 1 is not valid or we fail to read the left node, - * no big deal, just return. - */ if (IS_ERR(left)) - return 1; + return PTR_ERR(left); __btrfs_tree_lock(left, BTRFS_NESTING_LEFT); -- GitLab