btrfs: Handle owner mismatch gracefully when walking up tree
PATCH 1/1 btrfs: Handle owner mismatch gracefully when walking up tree
[BUG]
When mounting certain crafted image, btrfs will trigger kernel BUG_ON() when try to recover balance:
------
------------[ cut here ]------------ kernel BUG at fs/btrfs/extent-tree.c:8956! invalid opcode: 0000 [#1] PREEMPT SMP NOPTI CPU: 1 PID: 662 Comm: mount Not tainted 4.18.0-rc1-custom+ #10 RIP: 0010:walk_up_proc+0x336/0x480 [btrfs] RSP: 0018:ffffb53540c9b890 EFLAGS: 00010202 Call Trace: walk_up_tree+0x172/0x1f0 [btrfs] btrfs_drop_snapshot+0x3a4/0x830 [btrfs] merge_reloc_roots+0xe1/0x1d0 [btrfs] btrfs_recover_relocation+0x3ea/0x420 [btrfs] open_ctree+0x1af3/0x1dd0 [btrfs] btrfs_mount_root+0x66b/0x740 [btrfs] mount_fs+0x3b/0x16a vfs_kern_mount.part.9+0x54/0x140 btrfs_mount+0x16d/0x890 [btrfs] mount_fs+0x3b/0x16a vfs_kern_mount.part.9+0x54/0x140 do_mount+0x1fd/0xda0 ksys_mount+0xba/0xd0 __x64_sys_mount+0x21/0x30 do_syscall_64+0x60/0x210 entry_SYSCALL_64_after_hwframe+0x49/0xbe ---[ end trace d4344e4deee03435 ]---
------
[CAUSE]
Another extent tree corruption.
In this particular case, tree reloc root's owner is DATA_RELOC_TREE (should be TREE_RELOC_TREE), thus its backref is corrupted and we failed the owner check in walk_up_tree().
[FIX]
It's pretty hard to take care of every extent tree corruption, but at least we can remove such BUG_ON() and exit more gracefully.
And since in this particular image, DATA_RELOC_TREE and TREE_RELOC_TREE shares the same root (which is obviously invalid), we needs to make __del_reloc_root() more robust to detect such invalid share to avoid possible NULL dereference as root->node can be NULL in this case.
---
changelog:
v2:
Merge the same error message output code into one block and add label for the goto usage.
v2.1:
Fix compile error.
--- fs/btrfs/extent-tree.c | 18 ++++++++++++------ fs/btrfs/relocation.c | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-)
fs/btrfs/extent-tree.c +12 -6
@@ -8939,15 +8939,14 @@ static noinline int walk_up_proc(struct btrfs_trans_handle *trans,if (eb == root->node) {if (wc->flags[level] &BTRFS_BLOCK_FLAG_FULL_BACKREF ) parent = eb->start; -else -BUG_ON (root->root_key.objectid != -btrfs_header_owner (eb)); +else if (root->root_key.objectid !=btrfs_header_owner (eb)) + goto owner_mismatch; }else {if (wc->flags[level +1 ] &BTRFS_BLOCK_FLAG_FULL_BACKREF ) parent = path->nodes[level +1 ]->start; -else -BUG_ON (root->root_key.objectid != -btrfs_header_owner (path->nodes[level +1 ])); +else if (root->root_key.objectid != +btrfs_header_owner (path->nodes[level +1 ])) + goto owner_mismatch; }btrfs_free_tree_block (trans, root, eb, parent, wc->refs[level] ==1 );
@@ -8955,6 +8954,11 @@ static noinline int walk_up_proc(struct btrfs_trans_handle *trans, wc->refs[level] =0 ; wc->flags[level] =0 ;return 0 ; + +owner_mismatch: +btrfs_err_rl (fs_info,"unexpected tree owner, have %llu expect %llu" , +btrfs_header_owner (eb), root->root_key.objectid); +return -EUCLEAN ; } static noinline int walk_down_tree(struct btrfs_trans_handle *trans,
@@ -9008,6 +9012,8 @@ static noinline int walk_up_tree(struct btrfs_trans_handle *trans, ret =walk_up_proc (trans, root, path, wc);if (ret >0 )return 0 ; +if (ret <0 ) +return ret;if (path->locks[level]) { btrfs_tree_unlock_rw(path->nodes[level],
fs/btrfs/relocation.c +1 -1
@@ -1321,7 +1321,7 @@ static void __del_reloc_root(struct btrfs_root *root)struct mapping_node *node =NULL ;struct reloc_control *rc = fs_info->reloc_ctl; -if (rc) { +if (rc && root->node) {spin_lock (&rc->reloc_root_tree.lock); rb_node =tree_search (&rc->reloc_root_tree.rb_root, root->node->start);
Signature · 1 line
2.18.0
Quoted 41 lines · Qu Wenruo
[BUG]
When mounting certain crafted image, btrfs will trigger kernelBUG_ON()when try to recover balance:
------------------[ cut here ]------------ kernel BUG at fs/btrfs/extent-tree.c:8956! invalid opcode: 0000 [#1] PREEMPT SMP NOPTI CPU: 1 PID: 662 Comm: mount Not tainted 4.18.0-rc1-custom+ #10 RIP: 0010:walk_up_proc+0x336/0x480 [btrfs] RSP: 0018:ffffb53540c9b890 EFLAGS: 00010202 Call Trace: walk_up_tree+0x172/0x1f0 [btrfs] btrfs_drop_snapshot+0x3a4/0x830 [btrfs] merge_reloc_roots+0xe1/0x1d0 [btrfs] btrfs_recover_relocation+0x3ea/0x420 [btrfs] open_ctree+0x1af3/0x1dd0 [btrfs] btrfs_mount_root+0x66b/0x740 [btrfs] mount_fs+0x3b/0x16a vfs_kern_mount.part.9+0x54/0x140 btrfs_mount+0x16d/0x890 [btrfs] mount_fs+0x3b/0x16a vfs_kern_mount.part.9+0x54/0x140 do_mount+0x1fd/0xda0 ksys_mount+0xba/0xd0 __x64_sys_mount+0x21/0x30 do_syscall_64+0x60/0x210 entry_SYSCALL_64_after_hwframe+0x49/0xbe ---[ end trace d4344e4deee03435 ]---------
[CAUSE]
Another extent tree corruption.In this particular case, tree reloc root's owner is
DATA_RELOC_TREE(should beTREE_RELOC_TREE), thus its backref is corrupted and we failed the owner check inwalk_up_tree().[FIX]
It's pretty hard to take care of every extent tree corruption, but at least we can remove suchBUG_ON()and exit more gracefully.
Quoted 8 lines
And since in this particular image,
DATA_RELOC_TREEandTREE_RELOC_TREEshares the same root (which is obviously invalid), we needs to make__del_reloc_root()more robust to detect such invalid share to avoid possible NULL dereference asroot->nodecan be NULL in this case.Reported-by Xu Wen <[email protected]>Signed-off-by Qu Wenruo <[email protected]>
Quoted 5 lines · Qu Wenruo
[BUG]
When mounting certain crafted image, btrfs will trigger kernelBUG_ON()when try to recover balance:
------------------[ cut here ]------------
Please don't use lines starting with ---, an empty line is fine, or you can also indent the trace dump by a few spaces for clarity. I'll update the patches that have been merged so you don't need to resend.
Quoted 8 lines · David Sterba
Quoted 5 lines · Qu Wenruo
[BUG]
When mounting certain crafted image, btrfs will trigger kernelBUG_ON()when try to recover balance:
------------------[ cut here ]------------Please don't use lines starting with ---,
I though "------" won't trigger the "---" of git thus OK to use.
Quoted 3 lines
an empty line is fine, or you
can also indent the trace dump by a few spaces for clarity. I'll update the patches that have been merged so you don't need to resend.
I'll go an empty line from now on.
Thanks,
Qu