Re: [PATCH] btrfs: Validate child tree block's level and first key

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

[-- Attachment #1.1: Type: text/plain, Size: 2465 bytes --]



On 2018年03月22日 21:40, David Sterba wrote:
> On Mon, Mar 19, 2018 at 05:18:41PM +0800, Qu Wenruo wrote:
>> We have several reports about node pointer points to incorrect child
>> tree blocks, which could have even wrong owner and level but still with
>> valid generation and checksum.
>>
>> Although btrfs check could handle it and print error message like:
>> leaf parent key incorrect 60670574592
>>
>> Kernel doesn't have enough check on this type of corruption correctly.
>> At least add such check to read_tree_block() and btrfs_read_buffer(),
>> where we need two new parameters @first_key and @level to verify the
>> child tree block.
>>
>> For case where we don't have parent node to get @first_key and @level,
>> just pass @first_key as NULL and kernel will skip such check.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  fs/btrfs/backref.c     |  6 +++--
>>  fs/btrfs/ctree.c       | 25 +++++++++++++-----
>>  fs/btrfs/disk-io.c     | 69 ++++++++++++++++++++++++++++++++++++++++----------
>>  fs/btrfs/disk-io.h     |  8 +++---
>>  fs/btrfs/extent-tree.c |  6 ++++-
>>  fs/btrfs/print-tree.c  | 10 +++++---
>>  fs/btrfs/qgroup.c      |  7 +++--
>>  fs/btrfs/relocation.c  | 21 ++++++++++++---
>>  fs/btrfs/tree-log.c    | 12 ++++++---
>>  9 files changed, 126 insertions(+), 38 deletions(-)
>>
>> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
>> index 26484648d090..3866b8ab20f1 100644
>> --- a/fs/btrfs/backref.c
>> +++ b/fs/btrfs/backref.c
>> @@ -738,7 +738,8 @@ static int add_missing_keys(struct btrfs_fs_info *fs_info,
>>  		BUG_ON(ref->key_for_search.type);
>>  		BUG_ON(!ref->wanted_disk_byte);
>>  
>> -		eb = read_tree_block(fs_info, ref->wanted_disk_byte, 0);
>> +		eb = read_tree_block(fs_info, ref->wanted_disk_byte, 0, NULL,
>> +				     0);
> 
> Please add 2nd function that will take the extended parameters and
> keep read_tree_block as is.

So for any new caller of read_tree_block(), reviewer is the last person
to info the author to use these parameters for safety check?

And in fact, the old function should be avoid if possible, I think the
new parameters act as a pretty good sign to make any caller double think
about this.

Thanks,
Qu

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]