From: Qu Wenruo <quwenruo.btrfs@gmx.com>
To: Josef Bacik <josef@toxicpanda.com>, Qu Wenruo <wqu@suse.com>,
linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v3 3/5] btrfs: Detect unbalanced tree with empty leaf before crashing btree operations
Date: Wed, 12 Aug 2020 07:04:21 +0800 [thread overview]
Message-ID: <dbe1176e-db46-7ff7-1231-ee69d7c3c5d1@gmx.com> (raw)
In-Reply-To: <8d21ba85-52a5-5419-dc16-ceece8b0c3a8@toxicpanda.com>
On 2020/8/12 上午2:48, Josef Bacik wrote:
> On 8/9/20 8:09 AM, Qu Wenruo wrote:
>> [BUG]
>> With crafted image, btrfs will panic at btree operations:
>> kernel BUG at fs/btrfs/ctree.c:3894!
>> invalid opcode: 0000 [#1] SMP PTI
>> CPU: 0 PID: 1138 Comm: btrfs-transacti Not tainted 5.0.0-rc8+ #9
>> RIP: 0010:__push_leaf_left+0x6b6/0x6e0
>> Code: 00 00 48 98 48 8d 04 80 48 8d 74 80 65 e8 42 5a 04 00 48 8b
>> bd 78 ff ff ff 8b bf 90 d0 00 00 89 7d 98 83 ef 65 e9 06 ff ff ff <0f>
>> 0b 0f 0b 48 8b 85 78 ff ff ff 8b 90 90 d0 00 00 e9 eb fe ff ff
>> RSP: 0018:ffffc0bd4128b990 EFLAGS: 00010246
>> RAX: 0000000000000000 RBX: ffffa0a4ab8f0e38 RCX: 0000000000000000
>> RDX: ffffa0a280000000 RSI: 0000000000000000 RDI: ffffa0a4b3814000
>> RBP: ffffc0bd4128ba38 R08: 0000000000001000 R09: ffffc0bd4128b948
>> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000240
>> R13: ffffa0a4b556fb60 R14: ffffa0a4ab8f0af0 R15: ffffa0a4ab8f0af0
>> FS: 0000000000000000(0000) GS:ffffa0a4b7a00000(0000)
>> knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00007f2461c80020 CR3: 000000022b32a006 CR4: 00000000000206f0
>> Call Trace:
>> ? _cond_resched+0x1a/0x50
>> push_leaf_left+0x179/0x190
>> btrfs_del_items+0x316/0x470
>> btrfs_del_csums+0x215/0x3a0
>> __btrfs_free_extent.isra.72+0x5a7/0xbe0
>> __btrfs_run_delayed_refs+0x539/0x1120
>> btrfs_run_delayed_refs+0xdb/0x1b0
>> btrfs_commit_transaction+0x52/0x950
>> ? start_transaction+0x94/0x450
>> transaction_kthread+0x163/0x190
>> kthread+0x105/0x140
>> ? btrfs_cleanup_transaction+0x560/0x560
>> ? kthread_destroy_worker+0x50/0x50
>> ret_from_fork+0x35/0x40
>> Modules linked in:
>> ---[ end trace c2425e6e89b5558f ]---
>>
>> [CAUSE]
>> The offending csum tree looks like this:
>> checksum tree key (CSUM_TREE ROOT_ITEM 0)
>> node 29741056 level 1 items 14 free 107 generation 19 owner CSUM_TREE
>> ...
>> key (EXTENT_CSUM EXTENT_CSUM 85975040) block 29630464 gen 17
>> key (EXTENT_CSUM EXTENT_CSUM 89911296) block 29642752 gen 17 <<<
>> key (EXTENT_CSUM EXTENT_CSUM 92274688) block 29646848 gen 17
>> ...
>>
>> leaf 29630464 items 6 free space 1 generation 17 owner CSUM_TREE
>> item 0 key (EXTENT_CSUM EXTENT_CSUM 85975040) itemoff 3987
>> itemsize 8
>> range start 85975040 end 85983232 length 8192
>> ...
>> leaf 29642752 items 0 free space 3995 generation 17 owner 0
>> ^ empty leaf invalid owner ^
>>
>> leaf 29646848 items 1 free space 602 generation 17 owner CSUM_TREE
>> item 0 key (EXTENT_CSUM EXTENT_CSUM 92274688) itemoff 627
>> itemsize 3368
>> range start 92274688 end 95723520 length 3448832
>>
>> So we have a corrupted csum tree where one tree leaf is completely
>> empty, causing unbalanced btree, thus leading to unexpected btree
>> balance error.
>>
>> [FIX]
>> For this particular case, we handle it in two directions to catch it:
>> - Check if the tree block is empty through btrfs_verify_level_key()
>> So that invalid tree blocks won't be read out through
>> btrfs_search_slot() and its variants.
>>
>> - Check 0 tree owner in tree checker
>> NO tree is using 0 as its tree owner, detect it and reject at tree
>> block read time.
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202821
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> This test is done further down, just after a
>
> if (btrfs_header_generation(eb) > fs_info->last_trans_committed)
> return 0;
>
> Which I assume is the problem? The generation is 19, is that >
> last_trans_committed? Seems like this check just needs to be moved
> lower, right? Thanks,
Nope, that generation 19 is valid. That fs has a higher generation, so
that's completely valid.
The generation 19 is there because there is another csum leaf whose
generation is 19.
Thanks,
Qu
>
> Josef
next prev parent reply other threads:[~2020-08-11 23:04 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-09 12:09 [PATCH v3 0/5] btrfs: Enhanced runtime defence against fuzzed images Qu Wenruo
2020-08-09 12:09 ` [PATCH v3 1/5] btrfs: extent_io: Do extra check for extent buffer read write functions Qu Wenruo
2020-08-11 18:42 ` Josef Bacik
2020-08-09 12:09 ` [PATCH v3 2/5] btrfs: extent-tree: Kill BUG_ON() in __btrfs_free_extent() and do better comment Qu Wenruo
2020-08-11 18:45 ` Josef Bacik
2020-08-09 12:09 ` [PATCH v3 3/5] btrfs: Detect unbalanced tree with empty leaf before crashing btree operations Qu Wenruo
2020-08-11 18:48 ` Josef Bacik
2020-08-11 23:04 ` Qu Wenruo [this message]
2020-08-12 0:23 ` Josef Bacik
2020-08-12 0:29 ` Qu Wenruo
2020-08-12 1:50 ` Josef Bacik
2020-08-12 1:53 ` Qu Wenruo
2020-08-09 12:09 ` [PATCH v3 4/5] btrfs: extent-tree: Kill the BUG_ON() in insert_inline_extent_backref() Qu Wenruo
2020-08-11 18:50 ` Josef Bacik
2020-08-09 12:09 ` [PATCH v3 5/5] btrfs: ctree: Checking key orders before merged tree blocks Qu Wenruo
2020-08-11 18:53 ` Josef Bacik
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=dbe1176e-db46-7ff7-1231-ee69d7c3c5d1@gmx.com \
--to=quwenruo.btrfs@gmx.com \
--cc=josef@toxicpanda.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=wqu@suse.com \
/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