Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2023/8/9 19:20, David Sterba wrote:
> On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
>> [PROBLEM]
>> Inside function update_inline_extent_backref(), we have several
>> BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
>> filesystem.
>>
>> [ANAYLYSE]
>> Most of those BUG_ON()s and ASSERT()s are just a way of handling
>> unexpected on-disk data.
>>
>> Although we have tree-checker to rule out obviously incorrect extent
>> tree blocks, it's not enough for those ones.
>>
>> Thus we need proper error handling for them.
>>
>> [FIX]
>> Thankfully all the callers of update_inline_extent_backref() would
>> eventually handle the errror by aborting the current transaction.
>>
>> So this patch would do the proper error handling by:
>>
>> - Make update_inline_extent_backref() to return int
>>    The return value would be either 0 or -EUCLEAN.
>>
>> - Replace BUG_ON()s and ASSERT()s with proper error handling
>>    This includes:
>>    * Dump the bad extent tree leaf
>>    * Output an error message for the cause
>>      This would include the extent bytenr, num_bytes (if needed),
>>      the bad values and expected good values.
>>    * Return -EUCLEAN
>>
>>    Note here we remove all the WARN_ON()s, as eventually the transaction
>>    would be aborted, thus a backtrace would be triggered anyway.
>>
>> - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
>>    tree blocks
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> Is this fix for syzbot report
> https://lore.kernel.org/linux-btrfs/000000000000287928060275b914@google.com/
> ?

Unfortunately, I don't think this is the proper fix.

Since there is no reproducer, I'm not sure if it's really corrupted fs
causing the bug.

If it's something else, then the patch won't help at all, except a
little better debug output.

Thanks,
Qu