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: 1879 bytes --]



On 2018年03月22日 22:00, David Sterba wrote:
> On Thu, Mar 22, 2018 at 09:53:46PM +0800, Qu Wenruo wrote:
>>>> 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.
> 
> I saw half of the new parameters were just 0, NULL, so this looks like a
> lot of code churn and I haven't looked closer if there's a chance to
> fill the parameters in all callsites. So if it's a matter of adding them
> incrementally then fine.
> 
I'm afraid some of the call sites (ones I left with NULL, 0) are unable
to pass the new parameters by its nature.

Such callers include:
1) Tree root
   Just @bytenr and @gen from ROOT_ITEM. No @first_key.

2) Backref walker for FULL_BACKREF
   Only parent bytenr, no extra info on @first_key.

But despite of such call sites, every top-down reader should grab first
key and level. (And so I did in the patch).

BTW, about half of the read_tree_block() callers are using the new
parameters.
So a new function seems a little embarrassing here.

Thanks,
Qu


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