On 25.07.19 г. 1:54 ч., Qu Wenruo wrote:
>
>
> On 2019/7/25 上午12:00, David Sterba wrote:
>> On Wed, Jul 10, 2019 at 10:58:40AM +0000, WenRuo Qu wrote:
>>>
>>>
>>> On 2019/7/10 下午6:42, Nikolay Borisov wrote:
>>>>
>>>>
>>> [...]
>>>>>
>>>>> +/*
>>>>> + * Check if the [start, start + len) range is valid before
>>>>> reading/writing
>>>>> + * the eb.
>>>>> + *
>>>>> + * Caller should not touch the dst/src memory if this function returns
>>>>> error.
>>>>> + */
>>>>> +static int check_eb_range(const struct extent_buffer *eb, unsigned long
>>>>> start,
>>>>> + unsigned long len)
>>>>> +{
>>>>> + unsigned long end;
>>>>> +
>>>>> + /* start, start + len should not go beyond eb->len nor overflow */
>>>>> + if (unlikely(start > eb->len || start + len > eb->len ||
>>>>
>>>> I think your check here is wrong, it should be start + len > start +
>>>> eb->len. start is the logical address hence it can be a lot bigger than
>>>> the size of the eb which is 16k by default.
>>>
>>> Definitely NO.
>>>
>>> [start, start + len) must be in the range of [0, nodesize).
>>> So think again.
>>
>> 'start' is the logical address, that's always larger than eb->len (16K),
>> Nikolay is IMO right, the check
>
> No. This @start is not eb->start. It's the offset inside the eb.
Given David is the 2nd person who got confused I think it's clear that
the variable inside check_eb_range is poorly named ... For the next
version rename it to something like offset or eb_offset.
<snip>
>
