btrfs: Validate child tree block's level and first key
PATCH 2/2 btrfs: Validate child tree block's level and first key
We have several reports about node pointer points to incorrect child tree blocks, which could have even wrong owner and level but still with valid generation and checksum.
Although btrfs check could handle it and print error message like:
leaf parent key incorrect 60670574592
Kernel doesn't have enough check on this type of corruption correctly. At least add such check to read_tree_block() and btrfs_read_buffer(), where we need two new parameters @level and @first_key to verify the child tree block.
The new @level check is mandatory and all call sites are already modified to extract expected level from its call chain.
While @first_key is optional, the following call sites are skipping such check:
1) Root node/leaf
As ROOT_ITEM doesn't contain the first key, skip @first_key check.
2) Direct backref
Only parent bytenr and level is known and we need to resolve the key all by ourselves, skip @first_key check.
Another note of this verification is, it needs extra info from nodeptr or ROOT_ITEM, so it can't fit into current tree-checker framework, which is limited to node/leaf boundary.
Signed-off-by Qu Wenruo <[email protected]>
---
changelog:
v2:
Make @level check mandatory, suggesed by Jeff and Nikolay.
Change parameter order as @level is now mandatory, put it in front of @first_key.
Change verify_parent_level() to verify_key_level() to avoid confusion on the @level parameter.
Add btrfs_error() output for CONFIG_BTRFS_DEBUG to help debugging.
v2.1
Rebased to misc-next branch, to use parent_level, it needs to revert Nikolay's patch "btrfs: Remove unused parent_level var from btrfs_realloc_node", sorry Nikolay.
--- fs/btrfs/backref.c | 6 ++-- fs/btrfs/ctree.c | 31 ++++++++++++---- fs/btrfs/disk-io.c | 95 +++++++++++++++++++++++++++++++++++++++++++------- fs/btrfs/disk-io.h | 8 +++-- fs/btrfs/extent-tree.c | 6 +++- fs/btrfs/print-tree.c | 10 ++++-- fs/btrfs/qgroup.c | 7 ++-- fs/btrfs/ref-verify.c | 7 +++- fs/btrfs/relocation.c | 21 ++++++++--- fs/btrfs/tree-log.c | 28 +++++++++------ 10 files changed, 173 insertions(+), 46 deletions(-)
fs/btrfs/backref.c +4 -2
@@ -738,7 +738,8 @@ static int add_missing_keys(struct btrfs_fs_info *fs_info,BUG_ON (ref->key_for_search.type);BUG_ON (!ref->wanted_disk_byte); - eb =read_tree_block (fs_info, ref->wanted_disk_byte,0 ); + eb =read_tree_block (fs_info, ref->wanted_disk_byte,0 , + ref->level -1 ,NULL );if (IS_ERR (eb)) {free_pref (ref);return PTR_ERR (eb);
@@ -1288,7 +1289,8 @@ static int find_parent_nodes(struct btrfs_trans_handle *trans, ref->level == 0) {struct extent_buffer *eb; - eb =read_tree_block (fs_info, ref->parent,0 ); + eb =read_tree_block (fs_info, ref->parent,0 , + ref->level,NULL );if (IS_ERR (eb)) { ret =PTR_ERR (eb); goto out;
fs/btrfs/ctree.c +25 -6
@@ -1354,6 +1354,7 @@ get_old_root(struct btrfs_root *root, u64 time_seq)struct tree_mod_root *old_root =NULL ;u64 old_generation =0 ;u64 logical; +int level; eb_root =btrfs_read_lock_root_node (root); tm =__tree_mod_log_oldest_root (eb_root, time_seq);
@@ -1364,15 +1365,17 @@ get_old_root(struct btrfs_root *root, u64 time_seq) old_root = &tm->old_root; old_generation = tm->generation; logical = old_root->logical; + level = old_root->level; }else { logical = eb_root->start; + level =btrfs_header_level (eb_root); } tm =tree_mod_log_search (fs_info, logical, time_seq);if (old_root && tm && tm->op !=MOD_LOG_KEY_REMOVE_WHILE_FREEING ) {btrfs_tree_read_unlock (eb_root);free_extent_buffer (eb_root); - old =read_tree_block (fs_info, logical,0 ); + old =read_tree_block (fs_info, logical,0 , level,NULL );if (WARN_ON (IS_ERR (old) || !extent_buffer_uptodate (old))) {if (!IS_ERR (old))free_extent_buffer (old);
@@ -1571,11 +1574,14 @@ int btrfs_realloc_node(struct btrfs_trans_handle *trans,int end_slot;int i;int err =0 ; +int parent_level;int uptodate;u32 blocksize;int progress_passed =0 ;struct btrfs_disk_key disk_key; + parent_level =btrfs_header_level (parent); +WARN_ON (trans->transaction != fs_info->running_transaction);WARN_ON (trans->transid != fs_info->generation);
@@ -1589,6 +1595,7 @@ int btrfs_realloc_node(struct btrfs_trans_handle *trans,btrfs_set_lock_blocking (parent);for (i = start_slot; i <= end_slot; i++) { +struct btrfs_key first_key;int close =1 ;btrfs_node_key (parent, &disk_key, i);
@@ -1598,6 +1605,7 @@ int btrfs_realloc_node(struct btrfs_trans_handle *trans, progress_passed =1 ; blocknr =btrfs_node_blockptr (parent, i); gen =btrfs_node_ptr_generation (parent, i); +btrfs_node_key_to_cpu (parent, &first_key, i);if (last_block ==0 ) last_block = blocknr;
@@ -1621,7 +1629,9 @@ int btrfs_realloc_node(struct btrfs_trans_handle *trans, uptodate =0 ;if (!cur || !uptodate) {if (!cur) { - cur =read_tree_block (fs_info, blocknr, gen); + cur =read_tree_block (fs_info, blocknr, gen, + parent_level -1 , + &first_key);if (IS_ERR (cur)) {return PTR_ERR (cur); }else if (!extent_buffer_uptodate (cur)) {
@@ -1629,7 +1639,8 @@ int btrfs_realloc_node(struct btrfs_trans_handle *trans,return -EIO ; } }else if (!uptodate) { - err =btrfs_read_buffer (cur, gen); + err =btrfs_read_buffer (cur, gen, + parent_level -1 ,&first_key);if (err) {free_extent_buffer (cur);return err;
@@ -1782,14 +1793,17 @@ read_node_slot(struct btrfs_fs_info *fs_info, struct extent_buffer *parent, {int level =btrfs_header_level (parent);struct extent_buffer *eb; +struct btrfs_key first_key;if (slot <0 || slot >=btrfs_header_nritems (parent))return ERR_PTR (-ENOENT );BUG_ON (level ==0 ); +btrfs_node_key_to_cpu (parent, &first_key, slot); eb =read_tree_block (fs_info,btrfs_node_blockptr (parent, slot), -btrfs_node_ptr_generation (parent, slot)); +btrfs_node_ptr_generation (parent, slot), + level -1 , &first_key);if (!IS_ERR (eb) && !extent_buffer_uptodate (eb)) {free_extent_buffer (eb); eb =ERR_PTR (-EIO );
@@ -2385,10 +2399,14 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p,u64 gen;struct extent_buffer *b = *eb_ret;struct extent_buffer *tmp; +struct btrfs_key first_key;int ret; +int parent_level; blocknr =btrfs_node_blockptr (b, slot); gen =btrfs_node_ptr_generation (b, slot); + parent_level =btrfs_header_level (b); +btrfs_node_key_to_cpu (b, &first_key, slot); tmp =find_extent_buffer (fs_info, blocknr);if (tmp) {
@@ -2407,7 +2425,7 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p,btrfs_set_path_blocking (p);/* now we're allowed to do a blocking uptodate check */ - ret =btrfs_read_buffer (tmp, gen); + ret =btrfs_read_buffer (tmp, gen, parent_level -1 , &first_key);if (!ret) { *eb_ret = tmp;return 0 ;
@@ -2434,7 +2452,8 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p,btrfs_release_path (p); ret = -EAGAIN ; - tmp =read_tree_block (fs_info, blocknr,0 ); + tmp =read_tree_block (fs_info, blocknr,0 , parent_level -1 , + &first_key);if (!IS_ERR (tmp)) {/* * If the read above didn't mark this buffer up to date,
fs/btrfs/disk-io.c +82 -13
196 lines not on this page · raw message
fs/btrfs/disk-io.h +5 -3
@@ -52,8 +52,9 @@ static inline u64 btrfs_sb_offset(int mirror)struct btrfs_device ;struct btrfs_fs_devices ; -struct extent_buffer *read_tree_block (struct btrfs_fs_info *fs_info, -u64 bytenr,u64 parent_transid); +struct extent_buffer *read_tree_block (struct btrfs_fs_info *fs_info,u64 bytenr, +u64 parent_transid,int level, +struct btrfs_key *first_key);void readahead_tree_block (struct btrfs_fs_info *fs_info,u64 bytenr);int reada_tree_block_flagged (struct btrfs_fs_info *fs_info,u64 bytenr,int mirror_num,struct extent_buffer **eb);
@@ -123,7 +124,8 @@ static inline void btrfs_put_fs_root(struct btrfs_root *root)void btrfs_mark_buffer_dirty (struct extent_buffer *buf);int btrfs_buffer_uptodate (struct extent_buffer *buf,u64 parent_transid,int atomic); -int btrfs_read_buffer (struct extent_buffer *buf,u64 parent_transid); +int btrfs_read_buffer (struct extent_buffer *buf,u64 parent_transid,int level, +struct btrfs_key *first_key);u32 btrfs_csum_data (const char *data,u32 seed,size_t len);void btrfs_csum_final (u32 crc,u8 *result); blk_status_t btrfs_bio_wq_end_io(struct btrfs_fs_info *info, struct bio *bio,
fs/btrfs/extent-tree.c +5 -1
@@ -8710,6 +8710,7 @@ static noinline int do_walk_down(struct btrfs_trans_handle *trans,u64 parent;u32 blocksize;struct btrfs_key key; +struct btrfs_key first_key;struct extent_buffer *next;int level = wc->level;int reada =0 ;
@@ -8730,6 +8731,8 @@ static noinline int do_walk_down(struct btrfs_trans_handle *trans, } bytenr =btrfs_node_blockptr (path->nodes[level], path->slots[level]); +btrfs_node_key_to_cpu (path->nodes[level], &first_key, + path->slots[level]); blocksize = fs_info->nodesize; next =find_extent_buffer (fs_info, bytenr);
@@ -8794,7 +8797,8 @@ static noinline int do_walk_down(struct btrfs_trans_handle *trans,if (!next) {if (reada && level ==1 )reada_walk_down (trans, root, wc, path); - next =read_tree_block (fs_info, bytenr, generation); + next =read_tree_block (fs_info, bytenr, generation, level -1 , + &first_key);if (IS_ERR (next)) {return PTR_ERR (next); }else if (!extent_buffer_uptodate (next)) {
fs/btrfs/print-tree.c +7 -3
@@ -365,9 +365,13 @@ void btrfs_print_tree(struct extent_buffer *c) btrfs_node_blockptr(c, i)); }for (i =0 ; i < nr; i++) { -struct extent_buffer *next =read_tree_block (fs_info, -btrfs_node_blockptr (c, i), -btrfs_node_ptr_generation (c, i)); +struct btrfs_key first_key; +struct extent_buffer *next; + +btrfs_node_key_to_cpu (c, &first_key, i); + next =read_tree_block (fs_info,btrfs_node_blockptr (c, i), +btrfs_node_ptr_generation (c, i), + level -1 , &first_key);if (IS_ERR (next)) {continue ; }else if (!extent_buffer_uptodate (next)) {
fs/btrfs/qgroup.c +5 -2
@@ -1684,7 +1684,7 @@ int btrfs_qgroup_trace_subtree(struct btrfs_trans_handle *trans,return 0 ;if (!extent_buffer_uptodate (root_eb)) { - ret =btrfs_read_buffer (root_eb, root_gen); + ret =btrfs_read_buffer (root_eb, root_gen, root_level,NULL );if (ret) goto out; }
@@ -1715,6 +1715,7 @@ int btrfs_qgroup_trace_subtree(struct btrfs_trans_handle *trans, level = root_level;while (level >=0 ) {if (path->nodes[level] ==NULL ) { +struct btrfs_key first_key;int parent_slot;u64 child_gen;u64 child_bytenr;
@@ -1727,8 +1728,10 @@ int btrfs_qgroup_trace_subtree(struct btrfs_trans_handle *trans, parent_slot = path->slots[level +1 ]; child_bytenr =btrfs_node_blockptr (eb, parent_slot); child_gen =btrfs_node_ptr_generation (eb, parent_slot); +btrfs_node_key_to_cpu (eb, &first_key, parent_slot); - eb =read_tree_block (fs_info, child_bytenr, child_gen); + eb =read_tree_block (fs_info, child_bytenr, child_gen, + level, &first_key);if (IS_ERR (eb)) { ret =PTR_ERR (eb); goto out;
fs/btrfs/ref-verify.c +6 -1
@@ -579,11 +579,16 @@ static int walk_down_tree(struct btrfs_root *root, struct btrfs_path *path,while (level >=0 ) {if (level) { +struct btrfs_key first_key; + block_bytenr =btrfs_node_blockptr (path->nodes[level], path->slots[level]); gen =btrfs_node_ptr_generation (path->nodes[level], path->slots[level]); - eb =read_tree_block (fs_info, block_bytenr, gen); +btrfs_node_key_to_cpu (path->nodes[level], &first_key, + path->slots[level]); + eb =read_tree_block (fs_info, block_bytenr, gen, + level -1 , &first_key);if (IS_ERR (eb))return PTR_ERR (eb);if (!extent_buffer_uptodate (eb)) {
fs/btrfs/relocation.c +17 -4
@@ -1839,6 +1839,8 @@ int replace_path(struct btrfs_trans_handle *trans, parent = eb;while (1 ) { +struct btrfs_key first_key; + level =btrfs_header_level (parent);BUG_ON (level < lowest_level);
@@ -1852,6 +1854,7 @@ int replace_path(struct btrfs_trans_handle *trans, old_bytenr =btrfs_node_blockptr (parent, slot); blocksize = fs_info->nodesize; old_ptr_gen =btrfs_node_ptr_generation (parent, slot); +btrfs_node_key_to_cpu (parent, &key, slot);if (level <= max_level) { eb = path->nodes[level];
@@ -1876,7 +1879,8 @@ int replace_path(struct btrfs_trans_handle *trans,break ; } - eb =read_tree_block (fs_info, old_bytenr, old_ptr_gen); + eb =read_tree_block (fs_info, old_bytenr, old_ptr_gen, + level -1 , &first_key);if (IS_ERR (eb)) { ret =PTR_ERR (eb);break ;
@@ -2036,6 +2040,8 @@ int walk_down_reloc_tree(struct btrfs_root *root, struct btrfs_path *path, last_snapshot =btrfs_root_last_snapshot (&root->root_item);for (i = *level; i >0 ; i--) { +struct btrfs_key first_key; + eb = path->nodes[i]; nritems =btrfs_header_nritems (eb);while (path->slots[i] < nritems) {
@@ -2056,7 +2062,9 @@ int walk_down_reloc_tree(struct btrfs_root *root, struct btrfs_path *path, } bytenr =btrfs_node_blockptr (eb, path->slots[i]); - eb =read_tree_block (fs_info, bytenr, ptr_gen); +btrfs_node_key_to_cpu (eb, &first_key, path->slots[i]); + eb =read_tree_block (fs_info, bytenr, ptr_gen, i -1 , + &first_key);if (IS_ERR (eb)) {return PTR_ERR (eb); }else if (!extent_buffer_uptodate (eb)) {
@@ -2714,6 +2722,8 @@ static int do_relocation(struct btrfs_trans_handle *trans, path->lowest_level = node->level +1 ; rc->backref_cache.path[node->level] = node;list_for_each_entry (edge, &node->upper, list[LOWER ]) { +struct btrfs_key first_key; +cond_resched (); upper = edge->node[UPPER ];
@@ -2779,7 +2789,9 @@ static int do_relocation(struct btrfs_trans_handle *trans, blocksize = root->fs_info->nodesize; generation =btrfs_node_ptr_generation (upper->eb, slot); - eb =read_tree_block (fs_info, bytenr, generation); +btrfs_node_key_to_cpu (upper->eb, &first_key, slot); + eb =read_tree_block (fs_info, bytenr, generation, + upper->level -1 , &first_key);if (IS_ERR (eb)) { err =PTR_ERR (eb); goto next;
@@ -2944,7 +2956,8 @@ static int get_tree_block_key(struct btrfs_fs_info *fs_info,struct extent_buffer *eb;BUG_ON (block->key_ready); - eb =read_tree_block (fs_info, block->bytenr, block->key.offset); + eb =read_tree_block (fs_info, block->bytenr, block->key.offset, + block->level,NULL );if (IS_ERR (eb)) {return PTR_ERR (eb); }else if (!extent_buffer_uptodate (eb)) {
fs/btrfs/tree-log.c +17 -11
@@ -286,7 +286,7 @@ struct walk_control {* inside it */ int (*process_func)(struct btrfs_root *log,struct extent_buffer *eb, -struct walk_control *wc,u64 gen); +struct walk_control *wc,u64 gen,int level); };/*
@@ -294,7 +294,7 @@ struct walk_control {*/ static int process_one_buffer (struct btrfs_root *log,struct extent_buffer *eb, -struct walk_control *wc,u64 gen) +struct walk_control *wc,u64 gen,int level) {struct btrfs_fs_info *fs_info = log->fs_info;int ret =0 ;
@@ -304,7 +304,7 @@ static int process_one_buffer(struct btrfs_root *log,* pin down any logged extents, so we have to read the block. */ if (btrfs_fs_incompat (fs_info,MIXED_GROUPS )) { - ret =btrfs_read_buffer (eb, gen); + ret =btrfs_read_buffer (eb, gen, level,NULL );if (ret)return ret; }
@@ -2406,17 +2406,16 @@ static noinline int replay_dir_deletes(struct btrfs_trans_handle *trans,* back refs). */ static int replay_one_buffer (struct btrfs_root *log,struct extent_buffer *eb, -struct walk_control *wc,u64 gen) +struct walk_control *wc,u64 gen,int level) {int nritems;struct btrfs_path *path;struct btrfs_root *root = wc->replay_dest;struct btrfs_key key; -int level;int i;int ret; - ret =btrfs_read_buffer (eb, gen); + ret =btrfs_read_buffer (eb, gen, level,NULL );if (ret)return ret;
@@ -2533,6 +2532,8 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,WARN_ON (*level >=BTRFS_MAX_LEVEL );while (*level >0 ) { +struct btrfs_key first_key; +WARN_ON (*level <0 );WARN_ON (*level >=BTRFS_MAX_LEVEL ); cur = path->nodes[*level];
@@ -2545,6 +2546,7 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans, bytenr =btrfs_node_blockptr (cur, path->slots[*level]); ptr_gen =btrfs_node_ptr_generation (cur, path->slots[*level]); +btrfs_node_key_to_cpu (cur, &first_key, path->slots[*level]); blocksize = fs_info->nodesize; parent = path->nodes[*level];
@@ -2555,7 +2557,8 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,return PTR_ERR (next);if (*level ==1 ) { - ret = wc->process_func (root, next, wc, ptr_gen); + ret = wc->process_func (root, next, wc, ptr_gen, + *level -1 );if (ret) {free_extent_buffer (next);return ret;
@@ -2563,7 +2566,8 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans, path->slots[*level]++;if (wc->free) { - ret =btrfs_read_buffer (next, ptr_gen); + ret =btrfs_read_buffer (next, ptr_gen, + *level -1 , &first_key);if (ret) {free_extent_buffer (next);return ret;
@@ -2593,7 +2597,7 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,free_extent_buffer (next);continue ; } - ret =btrfs_read_buffer (next, ptr_gen); + ret =btrfs_read_buffer (next, ptr_gen, *level -1 , &first_key);if (ret) {free_extent_buffer (next);return ret;
@@ -2643,7 +2647,8 @@ static noinline int walk_up_log_tree(struct btrfs_trans_handle *trans, root_owner =btrfs_header_owner (parent); ret = wc->process_func (root, path->nodes[*level], wc, -btrfs_header_generation (path->nodes[*level])); +btrfs_header_generation (path->nodes[*level]), + *level);if (ret)return ret;
@@ -2725,7 +2730,8 @@ static int walk_log_tree(struct btrfs_trans_handle *trans,/* was the root node processed? if not, catch it here */ if (path->nodes[orig_level]) { ret = wc->process_func (log, path->nodes[orig_level], wc, -btrfs_header_generation (path->nodes[orig_level])); +btrfs_header_generation (path->nodes[orig_level]), + orig_level);if (ret) goto out;if (wc->free) {
Signature · 1 line
2.16.2
Quoted 28 lines · Qu Wenruo
We have several reports about node pointer points to incorrect child tree blocks, which could have even wrong owner and level but still with valid generation and checksum.
Although btrfs check could handle it and print error message like:
leaf parent key incorrect 60670574592Kernel doesn't have enough check on this type of corruption correctly. At least add such check to
read_tree_block()andbtrfs_read_buffer(), where we need two new parameters @level and @first_key to verify the child tree block.The new @level check is mandatory and all call sites are already modified to extract expected level from its call chain.
While @first_key is optional, the following call sites are skipping such check:
1) Root node/leaf
AsROOT_ITEMdoesn't contain the first key, skip @first_key check.
2) Direct backref
Only parent bytenr and level is known and we need to resolve the key all by ourselves, skip @first_key check.Another note of this verification is, it needs extra info from nodeptr or
ROOT_ITEM, so it can't fit into current tree-checker framework, which is limited to node/leaf boundary.Signed-off-by Qu Wenruo <[email protected]>
Quoted 13 lines
---
changelog:
v2:
Make @level check mandatory, suggesed by Jeff and Nikolay.
Change parameter order as @level is now mandatory, put it in front of @first_key.
Changeverify_parent_level()toverify_key_level()to avoid confusion on the @level parameter.
Addbtrfs_error()output forCONFIG_BTRFS_DEBUGto help debugging.
v2.1
Rebased to misc-next branch, to useparent_level, it needs to revert Nikolay's patch "btrfs: Remove unusedparent_levelvar frombtrfs_realloc_node", sorry Nikolay.
Ah ok, I've reverted the patch, thanks.