On 2018年02月07日 22:35, David Sterba wrote: > On Wed, Feb 07, 2018 at 05:18:25PM +0800, Qu Wenruo wrote: >> Strangely, we have level check in btrfs_print_tree() while we don't have >> the same check in read_node_slot(). >> >> That's to say, for the following corruption, btrfs_search_slot() or >> btrfs_next_leaf() can return invalid leaf: >> >> Parent eb: >> node XXXXXX level 1 >> ^^^^^^^ >> Child should be leaf (level 0) >> ... >> key (XXX XXX XXX) block YYYYYY >> >> Child eb: >> leaf YYYYYY level 1 >> ^^^^^^^ >> Something went wrong now >> >> And for the corrupted leaf returned, later caller can be screwed up >> easily. >> >> Although the root cause (powerloss, but still something wrong breaking >> metadata CoW of btrfs) is still unknown, at least enhance btrfs-progs to >> avoid SEGV. >> >> Reported-by: Ralph Gauges <ralphgau...@googlemail.com> >> Signed-off-by: Qu Wenruo <w...@suse.com> >> --- >> ctree.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/ctree.c b/ctree.c >> index 4fc33b14000a..ddb1e9cc6d37 100644 >> --- a/ctree.c >> +++ b/ctree.c >> @@ -22,6 +22,7 @@ >> #include "repair.h" >> #include "internal.h" >> #include "sizes.h" >> +#include "messages.h" >> >> static int split_node(struct btrfs_trans_handle *trans, struct btrfs_root >> *root, struct btrfs_path *path, int level); >> @@ -640,7 +641,9 @@ static int bin_search(struct extent_buffer *eb, struct >> btrfs_key *key, >> struct extent_buffer *read_node_slot(struct btrfs_fs_info *fs_info, >> struct extent_buffer *parent, int slot) >> { >> + struct extent_buffer *ret; >> int level = btrfs_header_level(parent); >> + >> if (slot < 0) >> return NULL; >> if (slot >= btrfs_header_nritems(parent)) >> @@ -649,8 +652,16 @@ struct extent_buffer *read_node_slot(struct >> btrfs_fs_info *fs_info, >> if (level == 0) >> return NULL; >> >> - return read_tree_block(fs_info, btrfs_node_blockptr(parent, slot), >> + ret = read_tree_block(fs_info, btrfs_node_blockptr(parent, slot), > > The result of read_tree_block should be checked before use, by > extent_buffer_uptodate, null check or IS_ERR at least (depending on the > context of use).
Right, just forgot that. Will fix it in next version. Thanks, Qu > >> btrfs_node_ptr_generation(parent, slot)); >> + if (btrfs_header_level(ret) != level - 1) { >> + error("child eb corrupted: parent bytenr=%llu item=%d parent >> level=%d child level=%d", >> + btrfs_header_bytenr(parent), slot, >> + btrfs_header_level(parent), btrfs_header_level(ret)); >> + free_extent_buffer(ret); >> + return ERR_PTR(-EIO); >> + } >> + return ret; >> } >> >> static int balance_level(struct btrfs_trans_handle *trans, >> -- >> 2.16.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
signature.asc
Description: OpenPGP digital signature