From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH 1/1] btrfs: Handle owner mismatch gracefully when walking up tree
Date: Wed, 1 Aug 2018 16:08:01 +0800 [thread overview]
Message-ID: <20180801080801.13024-1-wqu@suse.com> (raw)
[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>
---
As always, the patch is also pushed to my github repo, along with other
fuzzed images related fixes:
https://github.com/adam900710/linux/tree/tree_checker_enhance
(BTW, is it correct to indicate a branch like above?)
---
fs/btrfs/extent-tree.c | 27 +++++++++++++++++++--------
fs/btrfs/relocation.c | 2 +-
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index da615ebc072e..5f4ca61348b5 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -8949,17 +8949,26 @@ static noinline int walk_up_proc(struct btrfs_trans_handle *trans,
}
if (eb == root->node) {
- if (wc->flags[level] & BTRFS_BLOCK_FLAG_FULL_BACKREF)
+ 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)) {
+ btrfs_err_rl(fs_info,
+ "unexpected tree owner, have %llu expect %llu",
+ btrfs_header_owner(eb),
+ root->root_key.objectid);
+ return -EINVAL;
+ }
} else {
- if (wc->flags[level + 1] & BTRFS_BLOCK_FLAG_FULL_BACKREF)
+ 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])) {
+ btrfs_err_rl(fs_info,
+ "unexpected tree owner, have %llu expect %llu",
+ btrfs_header_owner(eb),
+ root->root_key.objectid);
+ return -EINVAL;
+ }
}
btrfs_free_tree_block(trans, root, eb, parent, wc->refs[level] == 1);
@@ -9020,6 +9029,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 a2fc0bd83a40..c64051d33d05 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);
--
2.18.0
next reply other threads:[~2018-08-01 9:52 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-01 8:08 Qu Wenruo [this message]
2018-08-01 10:08 ` [PATCH 1/1] btrfs: Handle owner mismatch gracefully when walking up tree Nikolay Borisov
2018-08-01 11:13 ` Qu Wenruo
2018-08-01 12:12 ` Nikolay Borisov
2018-08-01 12:19 ` Qu Wenruo
2018-08-01 12:36 ` Nikolay Borisov
2018-08-20 13:56 ` David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180801080801.13024-1-wqu@suse.com \
--to=wqu@suse.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).