Re: [PATCH v3] btrfs: Don't submit any btree write bio after transaction is aborted

Josef Bacik
2020-02-07T00:37:18Z

On 2/6/20 7:24 PM, Qu Wenruo wrote:
>
>
> On 2020/2/7 上午12:00, Josef Bacik wrote:
>> On 2/5/20 2:10 AM, Qu Wenruo wrote:
>>> [BUG]
>>> There is a fuzzed image which could cause KASAN report at unmount time.
>>>
>>>    ==================================================================
>>>    BUG: KASAN: use-after-free in btrfs_queue_work+0x2c1/0x390
>>>    Read of size 8 at addr ffff888067cf6848 by task umount/1922
>>>
>>>    CPU: 0 PID: 1922 Comm: umount Tainted: G        W         5.0.21 #1
>>>    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
>>> 1.10.2-1ubuntu1 04/01/2014
>>>    Call Trace:
>>>     dump_stack+0x5b/0x8b
>>>     print_address_description+0x70/0x280
>>>     kasan_report+0x13a/0x19b
>>>     btrfs_queue_work+0x2c1/0x390
>>>     btrfs_wq_submit_bio+0x1cd/0x240
>>>     btree_submit_bio_hook+0x18c/0x2a0
>>>     submit_one_bio+0x1be/0x320
>>>     flush_write_bio.isra.41+0x2c/0x70
>>>     btree_write_cache_pages+0x3bb/0x7f0
>>>     do_writepages+0x5c/0x130
>>>     __writeback_single_inode+0xa3/0x9a0
>>>     writeback_single_inode+0x23d/0x390
>>>     write_inode_now+0x1b5/0x280
>>>     iput+0x2ef/0x600
>>>     close_ctree+0x341/0x750
>>>     generic_shutdown_super+0x126/0x370
>>>     kill_anon_super+0x31/0x50
>>>     btrfs_kill_super+0x36/0x2b0
>>>     deactivate_locked_super+0x80/0xc0
>>>     deactivate_super+0x13c/0x150
>>>     cleanup_mnt+0x9a/0x130
>>>     task_work_run+0x11a/0x1b0
>>>     exit_to_usermode_loop+0x107/0x130
>>>     do_syscall_64+0x1e5/0x280
>>>     entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>>
>>> [CAUSE]
>>> The fuzzed image has a completely screwd up extent tree:
>>>    leaf 29421568 gen 8 total ptrs 6 free space 3587 owner EXTENT_TREE
>>>    refs 2 lock (w:0 r:0 bw:0 br:0 sw:0 sr:0) lock_owner 0 current 5938
>>>            item 0 key (12587008 168 4096) itemoff 3942 itemsize 53
>>>                    extent refs 1 gen 9 flags 1
>>>                    ref#0: extent data backref root 5 objectid 259
>>> offset 0 count 1
>>>            item 1 key (12591104 168 8192) itemoff 3889 itemsize 53
>>>                    extent refs 1 gen 9 flags 1
>>>                    ref#0: extent data backref root 5 objectid 271
>>> offset 0 count 1
>>>            item 2 key (12599296 168 4096) itemoff 3836 itemsize 53
>>>                    extent refs 1 gen 9 flags 1
>>>                    ref#0: extent data backref root 5 objectid 259
>>> offset 4096 count 1
>>>            item 3 key (29360128 169 0) itemoff 3803 itemsize 33
>>>                    extent refs 1 gen 9 flags 2
>>>                    ref#0: tree block backref root 5
>>>            item 4 key (29368320 169 1) itemoff 3770 itemsize 33
>>>                    extent refs 1 gen 9 flags 2
>>>                    ref#0: tree block backref root 5
>>>            item 5 key (29372416 169 0) itemoff 3737 itemsize 33
>>>                    extent refs 1 gen 9 flags 2
>>>                    ref#0: tree block backref root 5
>>>
>>> Note that, leaf 29421568 doesn't has its backref in extent tree.
>>> Thus extent allocator can re-allocate leaf 29421568 for other trees.
>>>
>>> Short version for the corruption:
>>> - Extent tree corruption
>>>    Existing tree block X can be allocated as new tree block.
>>>
>>> - Tree block X allocated to log tree
>>>    The tree block X generation get bumped, and is traced by
>>>    log_root->dirty_log_pages now.
>>>
>>> - Log tree writes tree blocks
>>>    log_root->dirty_log_pages is cleaned.
>>>
>>> - The original owner of tree block X wants to modify its content
>>>    Instead of COW tree block X to a new eb, due to the bumped
>>>    generation, tree block X is reused as is.
>>>
>>>    Btrfs believes tree block X is already dirtied due to its transid,
>>>    but it is not tranced by transaction->dirty_pages.
>>>
>>
>> But at the write part we should have gotten BTRFS_HEADER_FLAG_WRITTEN,
>> so we should have cow'ed this block.  So this isn't what's happening,
>> right?
>
> From my debugging, it's not the case. By somehow, after log tree writes
> back, the tree block just got reused.
>
>>   Or is something else clearing the BTRFS_HEADER_FLAG_WRITTEN in
>> between the writeout and this part?  Thanks,
>
> It didn't occur to me at the time of writing, is it possible that log
> tree get freed, thus that tree block X is considered free, and get
> re-allocated to extent tree again?
>

Yeah, but then they'd go onto the dirty pages radix tree properly, because it
would be the correct root, and we wouldn't have this problem.

> The problem is really killing me to digging.
> Can't we use this last-resort method anyway? The corrupted extent tree
> is really causing all kinds of issues we didn't expect...

Which is why I want the real root cause and a real fix, not something that's
papering over the problem. Thanks,

Josef