| Message ID | 20180821014203.5443-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2.1] btrfs: Handle owner mismatch gracefully when walking up tree | expand |
On Tue, Aug 21, 2018 at 09:42:03AM +0800, Qu Wenruo wrote: > [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. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=200411 > Reported-by: Xu Wen <wen.xu@gatech.edu> > Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: David Sterba <dsterba@suse.com>
On Tue, Aug 21, 2018 at 09:42:03AM +0800, Qu Wenruo wrote: > [BUG] > When mounting certain crafted image, btrfs will trigger kernel BUG_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.
On 2018/8/23 下午9:24, David Sterba wrote: > On Tue, Aug 21, 2018 at 09:42:03AM +0800, Qu Wenruo wrote: >> [BUG] >> When mounting certain crafted image, btrfs will trigger kernel BUG_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. > 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
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 3578fa5b30ef..697dffc9d598 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -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], diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index be94c65bb4d2..5ee49b796815 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -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);
[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. Link: https://bugzilla.kernel.org/show_bug.cgi?id=200411 Reported-by: Xu Wen <wen.xu@gatech.edu> Signed-off-by: Qu Wenruo <wqu@suse.com> --- 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(-)