Re: [PATCH v5 1/4] btrfs: extent_io: do extra check for extent buffer read write functions

Qu Wenruo
2020-08-20T09:59:15Z

On 2020/8/20 下午5:50, David Sterba wrote:
> On Thu, Aug 20, 2020 at 07:14:13AM +0800, Qu Wenruo wrote:
>>>> +static inline int check_eb_range(const struct extent_buffer *eb,
>>>> + unsigned long start, unsigned long len)
>>>> +{
>>>> + /* start, start + len should not go beyond eb->len nor overflow */
>>>> + if (unlikely(start > eb->len || start + len > eb->len ||
>>>> + len > eb->len)) {
>>>
>>> Can the number of condition be reduced? If 'start + len' overflows, then
>>> we don't need to check 'start > eb->len', and for the case where
>>> start = 1024 and len = -1024 the 'len > eb-len' would be enough.
>>
>> I'm afraid not.
>> Although 'start > eb->len || len > eb->len' is enough to detect overflow
>> case, it no longer detects cases like 'start = 2k, len = 3k' while
>> eb->len == 4K case.
>>
>> So we still need all 3 checks.
>
> I was suggesting 'start + len > eb->len', not 'start > eb-len'.
>
> "start > eb->len" is implied by "start + len > eb->len".
>

start > eb->len is not implied if (start + len) over flows.

E.g. start = -2K, len = 2k, eb->len = 4K. We can still pass !(start +
len > eb->len || len > eb->len).

In short, if we want overflow check along with each one checked, we
really need 3 checks.

Thanks,
Qu